Fix copying of single glob to directory

docs/source/copying.rst

@@ -196,11 +196,6 @@ Forward slashes are used for directory separators throughout.
 
 .. dropdown:: 1g. Glob to existing directory
 
-    .. warning::
-
-        This does not currently work correctly, it creates a extra directory
-        (`issue 1233 <https://github.com/fsspec/filesystem_spec/issues/1233>`_).
-
     Nonrecursive
 
     .. code-block:: python
@@ -234,11 +229,6 @@ Forward slashes are used for directory separators throughout.
 
 .. dropdown:: 1h. Glob to new directory
 
-    .. warning::
-
-        This does not currently work correctly, it creates a extra directory
-        (`issue 1233 <https://github.com/fsspec/filesystem_spec/issues/1233>`_).
-
     Nonrecursive
 
     .. code-block:: python

fsspec/spec.py

@@ -886,15 +886,13 @@ 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)
+        rpaths = self.expand_path(rpath, recursive=recursive)
+        if source_is_str and not recursive:
+            # Non-recursive glob does not copy directories
+            rpaths = [p for p in rpaths if not (trailing_sep(p) or self.isdir(p))]
+            if not rpaths:
+                return
 
         if isinstance(lpath, str):
             lpath = make_path_posix(lpath)
@@ -953,15 +951,13 @@ def put(self, lpath, rpath, recursive=False, callback=_DEFAULT_CALLBACK, **kwarg
         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)
+        lpaths = fs.expand_path(lpath, recursive=recursive)
+        if source_is_str and not recursive:
+            # Non-recursive glob does not copy directories
+            lpaths = [p for p in lpaths if not (trailing_sep(p) or self.isdir(p))]
+            if not lpaths:
+                return
 
         rpath = (
             self._strip_protocol(rpath)
@@ -1011,15 +1007,13 @@ def copy(self, path1, path2, recursive=False, on_error=None, **kwargs):
         elif on_error is None:
             on_error = "raise"
 
-        paths = self.expand_path(path1, recursive=recursive)
-        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)
+        paths = self.expand_path(path1, recursive=recursive)
+        if source_is_str and not recursive:
+            # Non-recursive glob does not copy directories
+            paths = [p for p in paths if not (trailing_sep(p) or self.isdir(p))]
+            if not paths:
+                return
 
         isdir = isinstance(path2, str) and (trailing_sep(path2) or self.isdir(path2))
         path2 = other_paths(

fsspec/tests/abstract/copy.py

@@ -168,20 +168,33 @@ def test_copy_glob_to_existing_directory(
         target = fs_join(fs_path, "target")
         fs.mkdir(target)
 
-        # for target_slash in [False, True]:
-        for target_slash in [False]:
+        for target_slash in [False, True]:
             t = target + "/" if target_slash else target
 
             # Without recursive
             fs.cp(fs_join(source, "subdir", "*"), t)
             assert fs.isfile(fs_join(target, "subfile1"))
             assert fs.isfile(fs_join(target, "subfile2"))
-            # assert not fs.isdir(fs_join(target, "nesteddir"))  # ERROR
-            assert not fs.isdir(fs_join(target, "subdir"))
+            assert not fs.isdir(fs_join(target, "nesteddir"))
+            assert not fs.exists(fs_join(target, "nesteddir", "nestedfile"))
+            assert not fs.exists(fs_join(target, "subdir"))
+
+            fs.rm(fs.ls(target, detail=False), recursive=True)
+            assert fs.ls(target) == []
 
             # With recursive
+            fs.cp(fs_join(source, "subdir", "*"), t, recursive=True)
+            assert fs.isfile(fs_join(target, "subfile1"))
+            assert fs.isfile(fs_join(target, "subfile2"))
+            assert fs.isdir(fs_join(target, "nesteddir"))
+            assert fs.isfile(fs_join(target, "nesteddir", "nestedfile"))
+            assert not fs.exists(fs_join(target, "subdir"))
+
+            fs.rm(fs.ls(target, detail=False), recursive=True)
+            assert fs.ls(target) == []
 
             # Limit by maxdepth
+            # ERROR: maxdepth ignored here
 
     def test_copy_glob_to_new_directory(self, fs, fs_join, fs_path, fs_scenario_cp):
         # Copy scenario 1h
@@ -200,8 +213,10 @@ def test_copy_glob_to_new_directory(self, fs, fs_join, fs_path, fs_scenario_cp):
             assert fs.isdir(fs_join(target, "newdir"))
             assert fs.isfile(fs_join(target, "newdir", "subfile1"))
             assert fs.isfile(fs_join(target, "newdir", "subfile2"))
-            # ERROR - do not copy empty directory
-            # assert not fs.exists(fs_join(target, "newdir", "nesteddir"))
+            assert not fs.exists(fs_join(target, "newdir", "nesteddir"))
+            assert not fs.exists(fs_join(target, "newdir", "nesteddir", "nestedfile"))
+            assert not fs.exists(fs_join(target, "subdir"))
+            assert not fs.exists(fs_join(target, "newdir", "subdir"))
 
             fs.rm(fs_join(target, "newdir"), recursive=True)
             assert fs.ls(target) == []
@@ -213,6 +228,8 @@ def test_copy_glob_to_new_directory(self, fs, fs_join, fs_path, fs_scenario_cp):
             assert fs.isfile(fs_join(target, "newdir", "subfile2"))
             assert fs.isdir(fs_join(target, "newdir", "nesteddir"))
             assert fs.isfile(fs_join(target, "newdir", "nesteddir", "nestedfile"))
+            assert not fs.exists(fs_join(target, "subdir"))
+            assert not fs.exists(fs_join(target, "newdir", "subdir"))
 
             fs.rm(fs_join(target, "newdir"), recursive=True)
             assert fs.ls(target) == []