cp: refuse to copy symlink over itself #7729
Conversation
| if dest_is_symlink | ||
| && source_is_symlink | ||
| && source.file_name() != dest.file_name() | ||
| && !options.dereference |
There was a problem hiding this comment.
Note, if source and dest are both the same symlink but with different names, then the existing behavior (cp succeeds) is correct. This can occur, for example, if source and dest are both hardlinks to the same symlink. There's actually an existing unit test for this case.
There was a problem hiding this comment.
maybe add this as a comment in the code too ? :)
There was a problem hiding this comment.
"hardlinks to the same symlink" also sounds like something that we should include in the test suite, especially because this PR does NOT handle it correctly, despite your comment:
# 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 bad2As you can see, the second file is no longer a hardlink of the first, even though it should still be.
There was a problem hiding this comment.
Thanks! Interesting test case. I looked into it and it seems related but separate from this PR (it occurs on main too BTW), so I made a separate PR for it: #7753
There was a problem hiding this comment.
Good call! I thought it's the same issue, sorry :D
610ad10 to
a43a2ff
Compare
|
GNU testsuite comparison: |
a43a2ff to
7677be2
Compare
|
GNU testsuite comparison: |
Fixes #6589
Here's a quick recap of the issue:
This doesn't affect the GNU compatibility tests.
Note that the other cases described in #6589 have the correct behavior and exit statuses, just different error messages. So I didn't bother to change those since I don't think matching error messages is a goal of this project (correct me if I'm wrong).