fix: push_path and pull_path include empty directories

ops/model.py

@@ -2275,6 +2275,9 @@ def local_list(source_path: Path) -> List[pebble.FileInfo]:
             try:
                 for info in Container._list_recursive(local_list, source_path):
                     dstpath = self._build_destpath(info.path, source_path, dest_dir)
+                    if info.type is pebble.FileType.DIRECTORY:
+                        self.make_dir(dstpath, make_parents=True)
+                        continue
                     with open(info.path) as src:
                         self.push(
                             dstpath,
@@ -2352,6 +2355,9 @@ def pull_path(self,
             try:
                 for info in Container._list_recursive(self.list_files, source_path):
                     dstpath = self._build_destpath(info.path, source_path, dest_dir)
+                    if info.type is pebble.FileType.DIRECTORY:
+                        dstpath.mkdir(parents=True, exist_ok=True)
+                        continue
                     dstpath.parent.mkdir(parents=True, exist_ok=True)
                     with self.pull(info.path, encoding=None) as src:
                         with dstpath.open(mode='wb') as dst:
@@ -2406,6 +2412,9 @@ def _list_recursive(list_func: Callable[[Path], Iterable[pebble.FileInfo]],
 
         for info in list_func(path):
             if info.type is pebble.FileType.DIRECTORY:
+                # Yield the directory to ensure empty directories are created, then
+                # all of the contained files.
+                yield info
                 yield from Container._list_recursive(list_func, Path(info.path))
             elif info.type in (pebble.FileType.FILE, pebble.FileType.SYMLINK):
                 yield info

test/test_model.py

@@ -981,28 +981,32 @@ def __init__(self,
                  files: typing.List[str],
                  want: typing.Optional[typing.Set[str]] = None,
                  dst: typing.Optional[str] = None,
-                 errors: typing.Optional[typing.Set[str]] = None):
+                 errors: typing.Optional[typing.Set[str]] = None,
+                 dirs: typing.Optional[typing.Set[str]] = None,
+                 want_dirs: typing.Optional[typing.Set[str]] = None):
         self.pattern = None
         self.dst = dst
         self.errors = errors or set()
         self.name = name
         self.path = path
         self.files = files
+        self.dirs = dirs or set()
         self.want = want or set()
+        self.want_dirs = want_dirs or set()
 
 
 recursive_list_cases = [
     PushPullCase(
         name='basic recursive list',
         path='/',
         files=['/foo/bar.txt', '/baz.txt'],
-        want={'/foo/bar.txt', '/baz.txt'},
+        want={'/foo', '/foo/bar.txt', '/baz.txt'},
     ),
     PushPullCase(
         name='basic recursive list reverse',
         path='/',
         files=['/baz.txt', '/foo/bar.txt'],
-        want={'/foo/bar.txt', '/baz.txt'},
+        want={'/foo', '/foo/bar.txt', '/baz.txt'},
     ),
     PushPullCase(
         name='directly list a (non-directory) file',
@@ -1156,6 +1160,14 @@ def inner(path: pathlib.Path):
         files=['/foo/bar/baz.txt', '/foo/foobar.txt', '/quux.txt'],
         want={'/baz/foobar.txt', '/baz/bar/baz.txt'},
     ),
+    PushPullCase(
+        name='push/pull an empty directory',
+        path='/foo',
+        dst='/foobar',
+        files=[],
+        dirs={'/foo/baz'},
+        want_dirs={'/foobar/foo/baz'},
+    ),
 ]
 
 
@@ -1179,6 +1191,10 @@ def test_recursive_push_and_pull(case: PushPullCase):
         os.makedirs(os.path.dirname(fpath), exist_ok=True)
         with open(fpath, 'w') as f:
             f.write('hello')
+    if case.dirs:
+        for directory in case.dirs:
+            fpath = os.path.join(push_src.name, directory[1:])
+            os.makedirs(fpath, exist_ok=True)
 
     # test push
     if isinstance(case.path, list):
@@ -1204,11 +1220,16 @@ def test_recursive_push_and_pull(case: PushPullCase):
         f'push_path gave wrong expected errors: want {case.errors}, got {errors}'
     for fpath in case.want:
         assert c.exists(fpath), f'push_path failed: file {fpath} missing at destination'
+    for fdir in case.want_dirs:
+        assert c.isdir(fdir), f'push_path failed: dir {fdir} missing at destination'
 
     # create pull test case filesystem structure
     pull_dst = tempfile.TemporaryDirectory()
     for fpath in case.files:
         c.push(fpath, 'hello', make_dirs=True)
+    if case.dirs:
+        for directory in case.dirs:
+            c.make_dir(directory, make_parents=True)
 
     # test pull
     errors: typing.Set[str] = set()
@@ -1223,6 +1244,8 @@ def test_recursive_push_and_pull(case: PushPullCase):
         f'pull_path gave wrong expected errors: want {case.errors}, got {errors}'
     for fpath in case.want:
         assert c.exists(fpath), f'pull_path failed: file {fpath} missing at destination'
+    for fdir in case.want_dirs:
+        assert c.isdir(fdir), f'pull_path failed: dir {fdir} missing at destination'
 
 
 @pytest.mark.parametrize('case', [

test/test_testing.py

@@ -4574,11 +4574,13 @@ def test_push_path(self):
             (tempdir / "foo/bar").mkdir(parents=True)
             (tempdir / "foo/test").write_text("test")
             (tempdir / "foo/bar/foobar").write_text("foobar")
+            (tempdir / "foo/baz").mkdir(parents=True)
             self.container.push_path(tempdir / "foo", "/tmp")
 
             self.assertTrue((self.root / "tmp").is_dir())
             self.assertTrue((self.root / "tmp/foo").is_dir())
             self.assertTrue((self.root / "tmp/foo/bar").is_dir())
+            self.assertTrue((self.root / "tmp/foo/baz").is_dir())
             self.assertEqual((self.root / "tmp/foo/test").read_text(), "test")
             self.assertEqual((self.root / "tmp/foo/bar/foobar").read_text(), "foobar")
 
@@ -4595,16 +4597,14 @@ def test_pull(self):
     def test_pull_path(self):
         (self.root / "foo").mkdir()
         (self.root / "foo/bar").write_text("bar")
-        # TODO: pull_path doesn't pull empty directories
-        # https://github.com/canonical/operator/issues/968
-        # (self.root / "foobar").mkdir()
+        (self.root / "foobar").mkdir()
         (self.root / "test").write_text("test")
         with tempfile.TemporaryDirectory() as temp:
             tempdir = pathlib.Path(temp)
             self.container.pull_path("/", tempdir)
             self.assertTrue((tempdir / "foo").is_dir())
             self.assertEqual((tempdir / "foo/bar").read_text(), "bar")
-            # self.assertTrue((tempdir / "foobar").is_dir())
+            self.assertTrue((tempdir / "foobar").is_dir())
             self.assertEqual((tempdir / "test").read_text(), "test")
 
     def test_list_files(self):