fix: match Unix cp -r nesting semantics in copy_files

Wauplin · claude · Wauplin · commit 2696ea36fca3 · 2026-04-10T13:58:53.000+02:00
When copying a folder to an existing directory, Unix `cp -r` nests the
source folder inside it (e.g. `cp -r src dst` → `dst/src/...`). The
previous implementation always copied contents directly (→ `dst/...`),
which is rsync-like rather than cp-like.

Track `destination_exists_as_directory` separately from the
`destination_is_directory` flag (which also covers trailing-slash hints)
so that folder copies can distinguish "destination exists" from
"destination was requested as a directory". Nesting only applies when
the destination actually exists as a prefix in the bucket.

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/src/huggingface_hub/hf_api.py b/src/huggingface_hub/hf_api.py
@@ -12621,15 +12621,21 @@ def copy_files(self, source: str, destination: str, *, token: str | bool | None
 
         destination_bucket_id = destination_handle.bucket_id
         destination_path = destination_handle.path
-        if destination_path == "" or destination.endswith("/"):
+        destination_is_directory = False
+        destination_exists_as_directory = False
+
+        if destination_path == "":
+            # Bucket root always exists as a directory
             destination_is_directory = True
+            destination_exists_as_directory = True
         else:
-            # Check if destination path is an existing file or a directory in the bucket
+            # Check if destination matches an existing file
             dest_path_info = list(self.get_bucket_paths_info(destination_bucket_id, [destination_path], token=token))
             if dest_path_info:
                 destination_is_directory = False
             else:
-                destination_is_directory = (
+                # Check if destination is an existing "directory" (prefix with children)
+                destination_exists_as_directory = (
                     next(
                         iter(
                             self.list_bucket_tree(
@@ -12640,6 +12646,8 @@ def copy_files(self, source: str, destination: str, *, token: str | bool | None
                     )
                     is not None
                 )
+                # Treat as directory if it exists as one, or if the user signaled with trailing slash
+                destination_is_directory = destination_exists_as_directory or destination.endswith("/")
 
         all_adds: list[tuple[str, str]] = []
         all_copies: list[_BucketCopyFile] = []
@@ -12665,6 +12673,14 @@ def _resolve_target_path(src_file_path: str, src_root_path: str | None, is_singl
 
             if rel_path == "":
                 raise ValueError("Cannot copy an empty relative path.")
+
+            # Match Unix `cp -r` behavior: when the destination already exists as a
+            # directory, nest the source folder inside it (e.g. cp -r src dst → dst/src/...).
+            # When the destination does not exist, use rename semantics (cp -r src new → new/...).
+            if destination_exists_as_directory and src_root_path is not None:
+                src_dir_basename = src_root_path.rsplit("/", 1)[-1]
+                rel_path = f"{src_dir_basename}/{rel_path}"
+
             if destination_path == "":
                 return rel_path
             return f"{destination_path.rstrip('/')}/{rel_path}"
@@ -12712,7 +12728,6 @@ def _add_repo_file(file: RepoFile, target_path: str) -> None:
                 )
             else:
                 # Source path is a folder (or prefix) — list and copy all matching files
-                destination_is_directory = True
                 for item in self.list_bucket_tree(
                     source_handle.bucket_id, prefix=source_path or None, recursive=True, token=token
                 ):
@@ -12744,7 +12759,6 @@ def _add_repo_file(file: RepoFile, target_path: str) -> None:
                 _add_repo_file(source_repo_path_info[0], target_path)
             else:
                 # Source path is a folder — list and copy all files recursively
-                destination_is_directory = True
                 for repo_item in self.list_repo_tree(
                     repo_id=source_handle.repo_id,
                     path_in_repo=source_path,
diff --git a/tests/test_buckets.py b/tests/test_buckets.py
@@ -388,7 +388,7 @@ def test_copy_files_folder_to_nonexistent_dest(api: HfApi, bucket_write: str, bu
 
 @requires("hf_xet")
 def test_copy_files_folder_to_existing_folder_dest(api: HfApi, bucket_write: str, bucket_write_2: str):
-    """source=folder, dest is an existing folder => files merged under dest path."""
+    """source=folder, dest is an existing folder => source folder nested under dest (like `cp -r`)."""
     api.batch_bucket_files(bucket_write, add=[(b"a", "folder/a.txt"), (b"b", "folder/sub/b.txt")])
     api.batch_bucket_files(bucket_write_2, add=[(b"existing", "target-folder/existing.txt")])
 
@@ -397,10 +397,11 @@ def test_copy_files_folder_to_existing_folder_dest(api: HfApi, bucket_write: str
         f"hf://buckets/{bucket_write_2}/target-folder",
     )
 
+    # Like `cp -r folder target-folder` when target-folder exists: nests as target-folder/folder/...
     destination_files = {entry.path for entry in api.list_bucket_tree(bucket_write_2)}
     assert "target-folder/existing.txt" in destination_files
-    assert "target-folder/a.txt" in destination_files
-    assert "target-folder/sub/b.txt" in destination_files
+    assert "target-folder/folder/a.txt" in destination_files
+    assert "target-folder/folder/sub/b.txt" in destination_files
 
 
 @requires("hf_xet")
@@ -449,6 +450,57 @@ def test_copy_files_file_to_folder_dest(api: HfApi, bucket_write: str, bucket_wr
     assert output_path.read_bytes() == b"content"
 
 
+@requires("hf_xet")
+def test_copy_files_folder_to_existing_folder_with_trailing_slash(api: HfApi, bucket_write: str, bucket_write_2: str):
+    """source=folder, dest is existing folder with trailing '/' => source folder nested (like `cp -r`)."""
+    api.batch_bucket_files(bucket_write, add=[(b"a", "logs/a.txt"), (b"b", "logs/sub/b.txt")])
+    api.batch_bucket_files(bucket_write_2, add=[(b"existing", "backup/existing.txt")])
+
+    api.copy_files(
+        f"hf://buckets/{bucket_write}/logs",
+        f"hf://buckets/{bucket_write_2}/backup/",
+    )
+
+    # Like `cp -r logs backup/` when backup/ exists: nests as backup/logs/...
+    destination_files = {entry.path for entry in api.list_bucket_tree(bucket_write_2)}
+    assert "backup/existing.txt" in destination_files
+    assert "backup/logs/a.txt" in destination_files
+    assert "backup/logs/sub/b.txt" in destination_files
+
+
+@requires("hf_xet")
+def test_copy_files_folder_to_nonexistent_dest_with_trailing_slash(api: HfApi, bucket_write: str, bucket_write_2: str):
+    """source=folder, dest doesn't exist but has trailing '/' => rename semantics (no nesting)."""
+    api.batch_bucket_files(bucket_write, add=[(b"a", "logs/a.txt"), (b"b", "logs/sub/b.txt")])
+
+    api.copy_files(
+        f"hf://buckets/{bucket_write}/logs",
+        f"hf://buckets/{bucket_write_2}/new-backup/",
+    )
+
+    # Like `cp -r logs new-backup/` when new-backup/ doesn't exist:
+    # in Unix this errors, but in object storage we create it with rename semantics.
+    destination_files = {entry.path for entry in api.list_bucket_tree(bucket_write_2)}
+    assert "new-backup/a.txt" in destination_files
+    assert "new-backup/sub/b.txt" in destination_files
+
+
+@requires("hf_xet")
+def test_copy_files_folder_to_bucket_root(api: HfApi, bucket_write: str, bucket_write_2: str):
+    """source=folder, dest is bucket root => source folder nested at root (like `cp -r models /`)."""
+    api.batch_bucket_files(bucket_write, add=[(b"a", "models/a.txt"), (b"b", "models/sub/b.txt")])
+
+    api.copy_files(
+        f"hf://buckets/{bucket_write}/models",
+        f"hf://buckets/{bucket_write_2}/",
+    )
+
+    # Bucket root always "exists" as a directory, so nesting applies
+    destination_files = {entry.path for entry in api.list_bucket_tree(bucket_write_2)}
+    assert "models/a.txt" in destination_files
+    assert "models/sub/b.txt" in destination_files
+
+
 @pytest.mark.parametrize(
     "source, destination, expected_content_type",
     [
