Skip to content
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: allow removing symbolic link loop destination #3972

Merged
merged 3 commits into from
Oct 10, 2022

Conversation

jfinkels
Copy link
Collaborator

Allow cp --remove-destination to remove a symbolic link loop (or a symbolic link that initiates a chain of too many symbolic links). Before this commit, if loop were a symbolic link to itself, then

cp --remove-destination file loop

would fail with an error message. After this commit, it succeeds. This matches the behaviotr of GNU cp.

This should make the GNU test suite file tests/cp/dir-rm-dest.sh pass.

@jfinkels jfinkels force-pushed the cp-symbolic-link-loop branch 2 times, most recently from 437e2f1 to ccc3810 Compare September 26, 2022 00:43
@tertsdiepraam
Copy link
Member

Congrats! The gnu test tests/cp/dir-rm-dest is no longer failing!

Nice!

// because if `path` is a symbolic link and there are too many
// levels of symbolic link, then those methods will return false
// or an OS error.
std::fs::metadata(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not functionally the same as just using symlink_metadata? Because if the path is not a link, they are the same and if it is a link, then symlink_metadata should return Ok in a superset of the cases of metadata, right?

fn file_or_link_exists(path: &Path) -> bool {
    path.symlink_metadata().is_ok()
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might be right. I'll update it.

@jfinkels jfinkels force-pushed the cp-symbolic-link-loop branch from ccc3810 to aad3202 Compare September 27, 2022 03:52
Allow `cp --remove-destination` to remove a symbolic link loop (or a
symbolic link that initiates a chain of too many symbolic
links). Before this commit, if `loop` were a symbolic link to itself,
then

    cp --remove-destination file loop

would fail with an error message. After this commit, it succeeds. This
matches the behaviotr of GNU cp.
@sylvestre sylvestre force-pushed the cp-symbolic-link-loop branch from aad3202 to 24630db Compare October 5, 2022 11:32
@github-actions
Copy link

Congrats! The gnu test tests/misc/tee is no longer failing!
Congrats! The gnu test tests/ls/readdir-mountpoint-inode is no longer failing!
Congrats! The gnu test tests/misc/sync is no longer failing!
GNU test failed: tests/misc/tee. tests/misc/tee is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/misc/tee is no longer failing!
Congrats! The gnu test tests/cp/dir-rm-dest is no longer failing!

@sylvestre
Copy link
Contributor

cool:
Congrats! The gnu test tests/cp/dir-rm-dest is no longer failing!

@sylvestre sylvestre merged commit d205823 into uutils:main Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants