Skip to content

cp -P hardlink-to-symlink hardlink-to-same-symlink should no-op #7753

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

Conversation

dan-hipschman
Copy link
Contributor

@dan-hipschman dan-hipschman commented Apr 13, 2025

Follow up to #7729 (comment)

Recap:

# Test setup
$ ln -s nonexistent bad
$ cp -P --link bad bad2
$ ls -1i bad bad2
20145169 bad
20145169 bad2

# GNU
$ cp -P bad bad2
$ ls -1i bad bad2
20145169 bad
20145169 bad2

# This PR (7677be2075a0401f05fdf9e138d821a5673f4404)
$ cargo run -q cp -P bad bad2
$ ls -1i bad bad2
20145169 bad
20178774 bad2

It looks like GNU detects that the hardlinks are the same and simply no-ops (I did an strace and all it's doing is stat-ing each file and exiting). There's similar logic in uutils, but it's currently conditioned on the backup option. I think this is unnecessary since if source and dest are hardlinks to the same file, then no actual copy should be necessary. Let me know if I may be missing something though; there are an awful lot of obscure edge cases though (luckily there are many unit tests for them).

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

Copy link
Collaborator

@BenWiederhake BenWiederhake left a comment

Choose a reason for hiding this comment

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

Deliciously simple. LGTM!

(test_tail::test_retry9 looks like a flake)

@BenWiederhake BenWiederhake merged commit d37f500 into uutils:main Apr 14, 2025
68 of 69 checks passed
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.

2 participants