Copy files using coreutils cp #595
Merged
Conversation
This file contains 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
Unlike `cat` redirected to a file, `cp` sees both its input and output and can more readily know the details of its arguments. These details can sometimes be exploited to achieve zero-copy transfers when the OS/filesystem can do copy-on-write (e.g. `clonefile` on Mac, `copy_file_range` on Linux). uutils' `cp` will attempt to take advantage of these opportunities where possible (see: https://github.com/uutils/coreutils/blob/35232689366fb102854693b6bd6b6161901d192e/src/uu/cp/src/cp.rs#L2190 ). This is a slight behaviour change with respect to permissions and file modes: `cp` preserves them while `cat` naturally does not. If file metadata erasure is an important property of this code (I don't think it is, but LMK) `cp` could be instructed to use the default file mode/permission using the `--no-preserve` flag. Also convert the one of the remaining references to the system's `cat` (rather than uutils') to also convert it to a call to `cp`.
|
Thanks for the detailed write up. I never got cp working with Bazel permissions, mode. |
Looks like I failed to get it working too. It seems I overlooked the fact that |
|
My nightmares came alive again, bonus points if you could get it working. :) |
We don't want it ever and it seems to be breaking things.
|
OK, despite my best efforts, I couldn't get this latest instance to repro locally, but I did manage to run the workflow in my fork to get a repro. And based on that, it seems that if we pass |
thesayyn
approved these changes
Jun 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Unlike
catredirected to a file,cpsees both its input and output and can more readily know the details of its arguments. These details can sometimes be exploited to achieve zero-copy transfers when the OS and filesystem know how to do copy-on-write (e.g.clonefileon Mac,copy_file_rangeon Linux). uutils'cpwill attempt to take advantage of these opportunities where possible (see: https://github.com/uutils/coreutils/blob/35232689366fb102854693b6bd6b6161901d192e/src/uu/cp/src/cp.rs#L2190 ).We need to pass
--no-preserve=modein order forcpto create the files as writable; which is necessary as some of the copies we perform need to be mutated afterwards. Depending on the nature of the Bazel setup, action input files might be readable but not writable, so the default behaviour ofcpto preserve the permissions from its input to its output in this case would cause our later mutations to fail.Also convert one of the remaining references to the system's
cat(rather than uutils') in order to also convert it to a call tocp.