cp: fix "delete before copy" logic. Add tests.

[andrewliebenow]

Also: fix "delete before copy" verbose mode output.

[sylvestre]

Could you please be more explicit about the issue you are fixing ?
Fix "delete before copy" logic isn't super clear

thanks

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/mv/update. tests/mv/update is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/timeout/timeout is no longer failing!

[andrewliebenow]

Sorry. The issue is kind of explained in this deleted comment:

coreutils/src/uu/cp/src/cp.rs

Lines 1775 to 1779 in b8150f5

	// TODO
	// Using `readonly` here to check if `dest` needs to be deleted is not correct:
	// "On Unix-based platforms this checks if any of the owner, group or others write permission bits are set. It does not check if the current user is in the file's assigned group. It also does not check ACLs. Therefore the return value of this function cannot be relied upon to predict whether attempts to read or write the file will actually succeed."
	// This results in some copy operations failing, because this necessary deletion is being skipped.
	is_symlink_loop(dest) || fs::metadata(dest)?.permissions().readonly()

If -f is used, or -i (and the user accepts the prompt), a non-writable file (a file with the user write bit set to 0) will be deleted by cp before the copy operation begins. This check was using the readonly function from the Rust Standard Library (https://doc.rust-lang.org/std/fs/struct.Permissions.html#method.readonly), but the documentation for readonly clearly states that it returns false if any write bit is set. On "target_family" = "unix" platforms, we can use the access API to check if the current user is actually able to write to the file.

This incorrect check was causing some necessary pre-copy deletions to be skipped, which then caused the copy operation to fail due to lack of write permission. I added tests for this edge case (the ones where the destination file mode is 077, as opposed to 000 in the other "delete before copy" tests).

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/mv/update. tests/mv/update is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/timeout/timeout is no longer failing!

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/timeout/timeout is no longer failing!

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/timeout/timeout. tests/timeout/timeout is passing on 'main'. Maybe you have to rebase?

[github-actions]

GNU testsuite comparison:

Congrats! The gnu test tests/timeout/timeout is no longer failing!

[sylvestre]

could you please add a link to the issue ?

[andrewliebenow]

I need to make one. I remember that this is the part of the test causing problems, but I'll have to refresh my memory on this:

https://github.com/coreutils/coreutils/blob/8083944484f2cdf6c9b737642567bcdb54db784d/tests/mv/update.sh#L72

[sylvestre]

please let me know when both comments are addressed :)

[sylvestre]

why this todo ?

[andrewliebenow]

That was an uncommon edge case, but I'll have to refresh my memory on the details. We don't have tests covering it.