-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: correctly copy attributes of a dangling symbolic link #3692
Conversation
Add helper method for deciding whether a symbolic link exists in the test directory.
Refactor common code used in several places into a convenience function `is_symlink()` that behaves like `Path::is_symlink()` added in Rust 1.58.0. (We support earlier versions of Rust so we cannot use the standard library version of this function.)
Fix a bug in which `cp` incorrectly exited with an error when attempting to copy the attributes of a dangling symbolic link (that is, when running `cp -P -p`). Fixes uutils#3531.
91cd563
to
4780690
Compare
src/uu/cp/src/cp.rs
Outdated
// TODO Replace this convenience function with `Path::is_symlink()` | ||
// when the minimum supported version of Rust is 1.58 or greater. | ||
match fs::symlink_metadata(path) { | ||
Err(_) => false, |
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.
Because we need to call file_type().is_symlink()
on the value stored in Ok()
. Could you give an example of what you mean and why it would work better?
I believe this is a case for a map. fs::symlink_metadata(path)
.map(|m| m.file_type().is_symlink())
.unwrap_or(false) |
I broke it with a bad rebase, could you please have a look? thanks |
@sylvestre you should remove the is symlink function from line 987-990 in cp.rs It's already in uucore as of now |
// don't dereference the link | ||
// | copy permissions, too | ||
// | | from the link | ||
// | | | to new file d2 | ||
// | | | | | ||
// V V V V | ||
ucmd.args(&["-P", "-p", "dangle", "d2"]) |
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.
excellent doc, bravo
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.
I was having a hard time remembering the meaning of each argument, so I needed to write it down :)
Sorry, I didn't get back to this in time! Thank you for updating it. |
Fix a bug in which
cp
incorrectly exited with an error whenattempting to copy the attributes of a dangling symbolic link (that
is, when running
cp -P -p
).Fixes #3531.
This causes the
tests/cp/preserve-slink-time.sh
script in the GNU test suite to change from ERROR to PASS: