Do nothing for non-recursive copy of empty directory

docs/source/copying.rst

@@ -121,9 +121,6 @@ Forward slashes are used for directory separators throughout.
 
     .. warning::
 
-       ``recursive=False`` is not correct
-       (`issue 1232 <https://github.com/fsspec/filesystem_spec/issues/1232>`_).
-
        ``maxdepth`` is not yet implemented for copying functions
        (`issue 1231 <https://github.com/fsspec/filesystem_spec/issues/1231>`_).
 
@@ -175,9 +172,6 @@ Forward slashes are used for directory separators throughout.
 
     .. warning::
 
-       ``recursive=False`` is not correct
-       (`issue 1232 <https://github.com/fsspec/filesystem_spec/issues/1232>`_).
-
        ``maxdepth`` is not yet implemented for copying functions
        (`issue 1231 <https://github.com/fsspec/filesystem_spec/issues/1231>`_).
 

fsspec/implementations/local.py

@@ -281,7 +281,7 @@ def trailing_sep(path):
     # TODO: if all incoming paths were posix-compliant then separator would
     # always be a forward slash, simplifying this function.
     # See https://github.com/fsspec/filesystem_spec/pull/1250
-    return path.endswith(os.sep) or os.altsep and path.endswith(os.altsep)
+    return path.endswith(os.sep) or (os.altsep is not None and path.endswith(os.altsep))
 
 
 def trailing_sep_maybe_asterisk(path):
@@ -294,10 +294,8 @@ def trailing_sep_maybe_asterisk(path):
     # TODO: if all incoming paths were posix-compliant then separator would
     # always be a forward slash, simplifying this function.
     # See https://github.com/fsspec/filesystem_spec/pull/1250
-    return (
-        path.endswith((os.sep, os.sep + "*"))
-        or os.altsep
-        and path.endswith((os.altsep, os.altsep + "*"))
+    return path.endswith((os.sep, os.sep + "*")) or (
+        os.altsep is not None and path.endswith((os.altsep, os.altsep + "*"))
     )
 
 

fsspec/implementations/tests/test_local.py

@@ -939,32 +939,24 @@ def test_cp_get_put_empty_directory(tmpdir, funcname):
     # cp/get/put without slash, target directory exists
     assert fs.isdir(target)
     func(empty, target)
-    assert fs.find(target, withdirs=True) == [
-        make_path_posix(os.path.join(target, "empty"))
-    ]
-
-    fs.rm(target + "/empty", recursive=True)
+    assert fs.find(target, withdirs=True) == []
 
     # cp/get/put with slash, target directory exists
     assert fs.isdir(target)
     func(empty + "/", target)
     assert fs.find(target, withdirs=True) == []
 
-    fs.rm(target, recursive=True)
+    fs.rmdir(target)
 
     # cp/get/put without slash, target directory doesn't exist
     assert not fs.isdir(target)
     func(empty, target)
-    assert fs.isdir(target)
-    assert fs.find(target, withdirs=True) == []
-
-    fs.rm(target, recursive=True)
+    assert not fs.isdir(target)
 
     # cp/get/put with slash, target directory doesn't exist
     assert not fs.isdir(target)
     func(empty + "/", target)
-    assert fs.isdir(target)
-    assert fs.find(target, withdirs=True) == []
+    assert not fs.isdir(target)
 
 
 def test_cp_two_files(tmpdir):

fsspec/implementations/tests/test_memory.py

@@ -291,30 +291,24 @@ def test_cp_empty_directory(m):
     # cp without slash, target directory exists
     assert m.isdir(target)
     m.cp(empty, target)
-    assert m.find(target, withdirs=True) == [target + "/empty"]
-
-    m.rm(target + "/empty", recursive=True)
+    assert m.find(target, withdirs=True) == []
 
     # cp with slash, target directory exists
     assert m.isdir(target)
     m.cp(empty + "/", target)
     assert m.find(target, withdirs=True) == []
 
-    m.rm(target, recursive=True)
+    m.rmdir(target)
 
     # cp without slash, target directory doesn't exist
     assert not m.isdir(target)
     m.cp(empty, target)
-    assert m.isdir(target)
-    assert m.find(target, withdirs=True) == []
-
-    m.rm(target, recursive=True)
+    assert not m.isdir(target)
 
     # cp with slash, target directory doesn't exist
     assert not m.isdir(target)
     m.cp(empty + "/", target)
-    assert m.isdir(target)
-    assert m.find(target, withdirs=True) == []
+    assert not m.isdir(target)
 
 
 def test_cp_two_files(m):

fsspec/spec.py

@@ -886,13 +886,21 @@ def get(self, rpath, lpath, recursive=False, callback=_DEFAULT_CALLBACK, **kwarg
             trailing_sep_maybe_asterisk,
         )
 
+        rpaths = self.expand_path(rpath, recursive=recursive)
+        if (
+            len(rpaths) == 1
+            and not recursive
+            and (trailing_sep(rpaths[0]) or self.isdir(rpaths[0]))
+        ):
+            # Non-recursive copy of directory does nothing.
+            return
+        source_is_str = isinstance(rpath, str)
+
         if isinstance(lpath, str):
             lpath = make_path_posix(lpath)
-        rpaths = self.expand_path(rpath, recursive=recursive)
         isdir = isinstance(lpath, str) and (
             trailing_sep(lpath) or LocalFileSystem().isdir(lpath)
         )
-        source_is_str = isinstance(rpath, str)
         lpaths = other_paths(
             rpaths,
             lpath,
@@ -942,17 +950,25 @@ def put(self, lpath, rpath, recursive=False, callback=_DEFAULT_CALLBACK, **kwarg
             trailing_sep_maybe_asterisk,
         )
 
+        if isinstance(lpath, str):
+            lpath = make_path_posix(lpath)
+        fs = LocalFileSystem()
+        lpaths = fs.expand_path(lpath, recursive=recursive)
+        if (
+            len(lpaths) == 1
+            and not recursive
+            and (trailing_sep(lpaths[0]) or self.isdir(lpaths[0]))
+        ):
+            # Non-recursive copy of directory does nothing.
+            return
+        source_is_str = isinstance(lpath, str)
+
         rpath = (
             self._strip_protocol(rpath)
             if isinstance(rpath, str)
             else [self._strip_protocol(p) for p in rpath]
         )
-        if isinstance(lpath, str):
-            lpath = make_path_posix(lpath)
-        fs = LocalFileSystem()
-        lpaths = fs.expand_path(lpath, recursive=recursive)
         isdir = isinstance(rpath, str) and (trailing_sep(rpath) or self.isdir(rpath))
-        source_is_str = isinstance(lpath, str)
         rpaths = other_paths(
             lpaths,
             rpath,
@@ -996,8 +1012,16 @@ def copy(self, path1, path2, recursive=False, on_error=None, **kwargs):
             on_error = "raise"
 
         paths = self.expand_path(path1, recursive=recursive)
-        isdir = isinstance(path2, str) and (trailing_sep(path2) or self.isdir(path2))
+        if (
+            len(paths) == 1
+            and not recursive
+            and (trailing_sep(paths[0]) or self.isdir(paths[0]))
+        ):
+            # Non-recursive copy of directory does nothing.
+            return
         source_is_str = isinstance(path1, str)
+
+        isdir = isinstance(path2, str) and (trailing_sep(path2) or self.isdir(path2))
         path2 = other_paths(
             paths,
             path2,
@@ -1019,6 +1043,7 @@ def expand_path(self, path, recursive=False, maxdepth=None, **kwargs):
 
         kwargs are passed to ``glob`` or ``find``, which may in turn call ``ls``
         """
+
         if maxdepth is not None and maxdepth < 1:
             raise ValueError("maxdepth must be at least 1")
 

fsspec/tests/abstract/copy.py

@@ -92,9 +92,8 @@ def test_copy_directory_to_existing_directory(
             t = target + "/" if target_slash else target
 
             # Without recursive does nothing
-            # ERROR: erroneously creates new directory
-            # fs.cp(s, t)
-            # assert fs.ls(target) == []
+            fs.cp(s, t)
+            assert fs.ls(target) == []
 
             # With recursive
             fs.cp(s, t, recursive=True)
@@ -143,9 +142,8 @@ def test_copy_directory_to_new_directory(
                 t += "/"
 
             # Without recursive does nothing
-            # ERROR: erroneously creates new directory
-            # fs.cp(s, t)
-            # assert fs.ls(target) == []
+            fs.cp(s, t)
+            assert fs.ls(target) == []
 
             # With recursive
             fs.cp(s, t, recursive=True)