-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cp: copy attributes after making subdir #6884
base: main
Are you sure you want to change the base?
Conversation
898a027
to
6676cde
Compare
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
fn build_dir( | ||
path: &PathBuf, | ||
recursive: bool, | ||
options: &Options, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why changing the order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the usual convention, no?
The "primary" argument is first, all options and etc come afterwards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe but if you do, it should be in a sep commit :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done. I've reorganized commits & force-pushed.
GNU testsuite comparison:
|
GNU testsuite comparison:
|
82de5b9
to
62c33d4
Compare
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
This pr should be ready to be merged. |
Sorry, i looked if skip_last wasn't implemented elsewhere and got distracted |
// (Note that there can be more than one! We might step out of | ||
// `./a/b/c` into `./a/`, in which case we'll need to fix the | ||
// permissions of both `./a/b/c` and `./a/b`, in that order.) | ||
if direntry.file_type().is_dir() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense to move this into a function?
it is starting to be a bit long
GNU testsuite comparison:
|
Closes #6875.
TODO:
test_dir_perm_race_with_preserve_mode_and_ownership
.