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: gnu test case preserve-mode fix #6432

Merged
merged 5 commits into from
May 30, 2024
Merged

Conversation

matrixhead
Copy link
Contributor

@matrixhead matrixhead commented May 25, 2024

This PR was initially intended to fix GNU test preserve-mode, but as it turns out, there was nothing wrong with how uu-cp preserved mode. The test was failing because overriding wasn't working properly. The test involved copying a FIFO with options -a --preserve=mode. The -a was supposed to disable copying contents, but it was getting overridden by --preserve wrongly. So, this PR now tries to fix the overriding of attributes.

changes in behavior

  • when a flag like -a which expands to -dR --preserve=all is given and a --preserve is given after that, cp would only override the --preserve part.
  • cp could handle --preserve and --no-preserve together in a single command, for example cp --preserve=mode,ownership --no-preserve=ownership --preserve=link f f2 would preserve mode and link but not the ownership

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/preserve-mode is no longer failing!

@sylvestre
Copy link
Sponsor Contributor

well done :)

Some comments:

  • please add an integration test
  • some comments in the code would be appreciate
  • the code coverage isn't happy for some lines

@matrixhead
Copy link
Contributor Author

thank you, i will do that

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/preserve-mode is no longer failing!

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/preserve-mode is no longer failing!
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

1 similar comment
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/preserve-mode is no longer failing!
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/preserve-mode is no longer failing!
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@matrixhead matrixhead marked this pull request as ready for review May 29, 2024 19:56
@matrixhead
Copy link
Contributor Author

changes in force push

  • changed the way how overriding was handled.

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/preserve-mode is no longer failing!

src/uu/cp/src/cp.rs Outdated Show resolved Hide resolved
matrixhead and others added 2 commits May 30, 2024 14:08
Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/preserve-mode is no longer failing!
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@matrixhead matrixhead requested a review from sylvestre May 30, 2024 14:33
@sylvestre
Copy link
Sponsor Contributor

excellent, thanks!

@sylvestre sylvestre merged commit 8cac375 into uutils:main May 30, 2024
62 of 68 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