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: fix for gnu test case i-2 compatibility for mv #6659

Closed
wants to merge 14 commits into from

Conversation

matrixhead
Copy link
Contributor

fix for #6658 contains #6599

Behaviors changed:

  • When the destination file is read-only and the -i flag is given, instead of prompting with something like cp: overwrite 'f'? cp will prompt with unwritable 'f' (mode 0000, ---------); try anyway?
  • When the destination file is read-only and the -i and -f flags are given, instead of prompting with something like cp: overwrite 'f'? cp will prompt with cp: replace 'f', overriding mode 0000 (---------)?. Additionally, this operation will no longer fail.

@matrixhead matrixhead marked this pull request as draft August 21, 2024 11:43
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/mv/i-2 is no longer failing!
Congrats! The gnu test tests/mv/mv-n is no longer failing!
Congrats! The gnu test tests/mv/update is no longer failing!

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/mv/i-2 is no longer failing!
Congrats! The gnu test tests/mv/mv-n is no longer failing!
Congrats! The gnu test tests/mv/update is no longer failing!

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/cp-i is no longer failing!
Congrats! The gnu test tests/mv/i-2 is no longer failing!
Congrats! The gnu test tests/mv/mv-n is no longer failing!
Congrats! The gnu test tests/mv/update is no longer failing!

Copy link

github-actions bot commented Sep 8, 2024

GNU testsuite comparison:

Congrats! The gnu test tests/cp/cp-i is no longer failing!
Congrats! The gnu test tests/mv/i-2 is no longer failing!
Congrats! The gnu test tests/mv/mv-n is no longer failing!
Congrats! The gnu test tests/mv/update is no longer failing!

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/cp-i is no longer failing!
Congrats! The gnu test tests/mv/i-2 is no longer failing!
Congrats! The gnu test tests/mv/mv-n is no longer failing!
Congrats! The gnu test tests/mv/update is no longer failing!

@sylvestre
Copy link
Contributor

please let us know when it is ready to be reviewed

@sylvestre
Copy link
Contributor

Congrats! The gnu test tests/cp/cp-i is no longer failing!
Congrats! The gnu test tests/mv/i-2 is no longer failing!
Congrats! The gnu test tests/mv/mv-n is no longer failing!
Congrats! The gnu test tests/mv/update is no longer failing!

Impressed by this btw
I don't remember the last time we fixed that many tests at the same time :)

@matrixhead
Copy link
Contributor Author

😊 Thanks, that means a lot! Would it be possible to get a review for #6599? I know these PRs are a bit scattered—should I try to clean them up a bit more?

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cp/cp-i is no longer failing!
Congrats! The gnu test tests/mv/i-2 is no longer failing!

@sylvestre
Copy link
Contributor

@matrixhead i am sorry, it is conflicting, could you please have a look ? sorry

@matrixhead
Copy link
Contributor Author

@sylvestre Please don't be! It looks like #6700 offers a more refined solution to this problem. Thanks for the heads-up—I'll take a look and resolve the conflicts. 🙂

@matrixhead matrixhead marked this pull request as draft October 7, 2024 06:08
@matrixhead
Copy link
Contributor Author

It looks like #6712 (which is a continuation of #6700) would fix the same issue this PR was intended to solve. Since #6700 has been merged, I think it would be better to close this PR in favor of #6712.

@matrixhead matrixhead closed this Oct 11, 2024
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