[Fix] Match Unix cp -r nesting semantics in copy_files#4081
Merged
Conversation
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) <[email protected]>
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2696ea3. Configure here.
Wauplin
changed the title
cp -r nesting semantics in copy_filescp -r nesting semantics in copy_files
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
julien-c
approved these changes
Apr 10, 2026
Member
julien-c
left a comment
julien-c
left a comment
There was a problem hiding this comment.
Sorry about sending you into such a rabbit hole. =)
Contributor
|
This PR has been shipped as part of the v1.11.0 release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why? Let's make
hf buckets cpas consistent as possible with unixcp. It's already almost the case (see table), this PR solves the few remaining issues + adds tests.Summary
When copying a folder to a destination that already exists as a directory, Unix
cp -rnests the source folder inside it. Ourcopy_filesimplementation was instead always copying the contents directly (merge/rename semantics), which is closer torsync src/ dst/thancp -r.This PR introduces a
destination_exists_as_directoryflag (separate fromdestination_is_directory) so the folder-copy path can distinguish "destination confirmed to exist via API" from "user hinted directory with trailing/". Nesting only applies when the destination actually exists as a prefix.cpvshf buckets cp— full comparisoncpresulthf buckets cpresultdst.txtdst.txtcp f.txt new.txtnew.txtnew.txtcp f.txt existing.txtexisting.txt(overwritten)existing.txt(overwritten)cp f.txt newnamenewnamenewname/)cp f.txt dirdir/f.txtdir/f.txt/cp f.txt dir/dir/f.txtdir/f.txtcp f.txt //f.txtf.txt(at root)cp -r src newdstnewdst/contents...newdst/contents.../)cp -r src dstdst/src/contents...dst/src/contents.../)cp -r src dst/dst/src/contents...dst/src/contents.../cp -r src newdst/newdst/contents...cp -r src //src/contents...src/contents...(at root)/on sourcecp -r src/ dstcp -r src dst-r)Test plan
test_copy_files_folder_to_existing_folder_dest— now expects nested pathstest_copy_files_folder_to_existing_folder_with_trailing_slash— existing dir + trailing/test_copy_files_folder_to_nonexistent_dest_with_trailing_slash— rename semantics preservedtest_copy_files_folder_to_bucket_root— nesting at bucket root🤖 Generated with Claude Code
Note
Medium Risk
Changes destination path resolution for folder copies, which can alter where files land for existing bucket prefixes (including root) and may break callers relying on the previous merge behavior.
Overview
HfApi.copy_filesnow distinguishes between destination hinted as a directory (trailing/) and destination confirmed to exist as a directory prefix via API checks. When copying a folder into an existing destination directory (including bucket root), it now nests the source folder under the destination to match Unixcp -rsemantics instead of merging contents directly.Tests were updated/added to cover nesting for existing destinations (with and without trailing
/), preserved rename semantics for non-existent destinations, and the bucket-root folder copy case.Reviewed by Cursor Bugbot for commit c4f0413. Bugbot is set up for automated code reviews on this repo. Configure here.