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

Realpath symlinks handling, solves issue #3669 #3703

Merged
merged 22 commits into from
Jul 10, 2022

Conversation

niyaznigmatullin
Copy link
Contributor

Fixes issue #3669

  • Made better component-wise symlink handling, if any part of the path inside symlink is non-existent it fails now

  • Fixed test for dangling symlink which was mistakenly changed to failing, try:

      $ ln -s nonexisting link
      $ realpath link
    

    It should print path to nonexisting as GNU version does

  • Fixed test for looping symlink, GNU version does make an error

      $ ln -s 1 2
      $ ln -s 2 3
      $ ln -s 3 1
      $ realpath 1
    

    It should fail with Too many levels of symbolic links

@niyaznigmatullin niyaznigmatullin marked this pull request as draft July 6, 2022 21:01
@niyaznigmatullin niyaznigmatullin force-pushed the realpath_symlinks_handling branch 2 times, most recently from f7b77f5 to b8c93a1 Compare July 7, 2022 16:22
@niyaznigmatullin
Copy link
Contributor Author

niyaznigmatullin commented Jul 8, 2022

After changing canonicalize function, some cp util usages began to fail, it incorrectly spotted whether it required to copy symlink itself or copy the contents, when checking for errors. So,

  • Fixed how to check if source and destination are the same file, using FileInformation
  • When copying symlink over symlink with no-dereference option, symlink not always removed (i.e. it was dangling or was a symlink to directory), fixed it
  • When destination is symlink, no-dereference is used and source is a regular file it didn't check that destination is dangling (it thought symlink copy is going to take place, but since source is a regular file, contents are copied)
  • Added more tests for symlinks, and no-dereference options

Also all tests that contained relative symlinks with / inside the symlink didn't work on Windows, I think that's because Windows itself doesn't support it (mklink ../t a doesn't work with strange message, but mklink ..\t a works correctly), it creates it, but then it can't open the file through symlink. So I changed that in tests when creating relative symlinks all slashes are substituted to a correct separator.

The part of cp with preserving hardlinks used unsafe syscalls, I found out it fails silently in Windows sometimes, I substituted it to using FileInformation, since it does the same.

@niyaznigmatullin niyaznigmatullin marked this pull request as ready for review July 8, 2022 07:37
@sylvestre
Copy link
Contributor

wahou:
bravo!


Warning: Congrats! The gnu test tests/cp/no-deref-link1 is no longer failing!
Warning: Congrats! The gnu test tests/cp/no-deref-link2 is no longer failing!
Warning: Congrats! The gnu test tests/cp/slink-2-slink is no longer failing!
Warning: Congrats! The gnu test tests/ln/relative is no longer failing!
Warning: Congrats! The gnu test tests/misc/readlink-root is no longer failing!

symlinks resolved to 256 for windows due to directory rights issue
can't compare just by equals, checking only last part
@niyaznigmatullin niyaznigmatullin force-pushed the realpath_symlinks_handling branch from 603e194 to 6283227 Compare July 8, 2022 20:43
@sylvestre sylvestre merged commit 9d285e9 into uutils:main Jul 10, 2022
@niyaznigmatullin niyaznigmatullin deleted the realpath_symlinks_handling branch July 18, 2022 06:57
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