cp: force copying file to itself with --backup

[jfinkels]

Fix the behavior of cp when both --backup and --force are specified and the source and destination are the same file. Before this commit, cp terminated without copying and without making a backup. After this commit, the copy is made and the backup file is made. For example,

$ touch f
$ cp --force --backup f f

results in a backup file f~ being created.

This should fix the GNU test suite file tests/cp/backup-1.sh.

[sylvestre]

Indeed: Warning: Congrats! The gnu test tests/cp/backup-1 is no longer failing! well done

[sylvestre]

But fails on Windows with:

---- test_cp::test_same_file_force_backup stdout ----
current_directory_resolved: 
touch: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpHGDeZD\f
run: D:\a\coreutils\coreutils\target\debug\coreutils.exe cp --force --backup f f
thread 'test_cp::test_same_file_force_backup' panicked at 'Command was expected to succeed.
stdout = 
 stderr = cp: Option 'force' not yet implemented.
', tests\common\util.rs:176:9

[jfinkels]

I marked the test as not(windows) and opened issue #3969.

[jfinkels]

It seems that there is still an issue on macos. I'm not really able to diagnose it:

---- test_cp::test_same_file_force_backup stdout ----
current_directory_resolved: 
touch: /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/.tmpLeUePk/f
run: /Users/runner/work/coreutils/coreutils/target/debug/coreutils cp --force --backup f f
thread 'test_cp::test_same_file_force_backup' panicked at 'Command was expected to succeed.
stdout = 
 stderr = cp: 'f' -> 'f': No such file or directory (os error 2)
', tests/common/util.rs:176:9

[jfinkels]

Oh, this is accomplishing the same goal as pull request #3427.

[tertsdiepraam]

It is, but it is doing it differently. @sylvestre focused on the --backup and you focused on --force, but I think the full solution requires checking both. Here's some GNU output:

❯ cp --force source.txt source.txt
cp: 'source.txt' and 'source.txt' are the same file

❯ cp --backup source.txt source.txt
cp: 'source.txt' and 'source.txt' are the same file

❯ cp --backup --force source.txt source.txt
[succeeds: source.txt~ is created]

On this branch, the first command succeeds as well as the third. On #3427, the second and third succeed. If you want you could try to get all cases correctly in this PR.

[jfinkels]

Interesting, thanks for the test cases. I'll update this branch to get all three to work.

[github-actions[bot]]

Congrats! The gnu test tests/misc/tee is no longer failing! Congrats! The gnu test tests/ls/readdir-mountpoint-inode is no longer failing! Congrats! The gnu test tests/misc/sync is no longer failing! GNU test failed: tests/misc/tee. tests/misc/tee is passing on 'main'. Maybe you have to rebase? Congrats! The gnu test tests/misc/tee is no longer failing! Congrats! The gnu test tests/cp/backup-1 is no longer failing!

[sylvestre]

Still fails on mac:

---- test_cp::test_same_file_force_backup stdout ----
current_directory_resolved: 
touch: /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/.tmpmo36hU/f
run: /Users/runner/work/coreutils/coreutils/target/debug/coreutils cp --force --backup f f
---- test_cp::test_same_file_force_backup stderr ----
thread 'main' panicked at 'Command was expected to succeed.
stdout = 
 stderr = cp: 'f' -> 'f': No such file or directory (os error 2)
', tests/common/util.rs:176:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[github-actions[bot]]

GNU testsuite comparison:

Congrats! The gnu test tests/cp/backup-1 is no longer failing!

[sylvestre]

Still the same issue on mac sorry

[jfinkels]

I don't own a Mac so I'm probably not going to be able to get this to pass on the macos environment. I attempted to add the following block to the copy_on_write() function in the platform/macos.rs module, but it didn't seem to help:

        // `std::fs::copy()` seems to fail on Mac OS if `source` and                          
        // `dest` are the same, so instead we check whether the two                           
        // files are the same and do nothing in that case.                                    
        if paths_refer_to_same_file(source, dest, true) {                                     
            return Ok(());                                                                    
        }          

Hopefully someone comes along with a Mac who can help. An alternative is to just skip the test on macos, but I feel like there should be a simple solution out there...

[sylvestre]

Hopefully someone comes along with a Mac who can help. An alternative is to just skip the test on macos, but I feel like there should be a simple solution out there...

Works for me (if an issue is opened to keep track of this)

thanks

S

[github-actions[bot]]

GNU testsuite comparison:

Congrats! The gnu test tests/cp/backup-1 is no longer failing!

[sylvestre]

Wrong syntax for the mac ignore:

error: operating system used in target family position
    --> tests/by-util/test_cp.rs:2254:1
     |
2254 | #[cfg(all(not(windows), not(macos)))]
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----^^^^
     |                             |
     |                             help: try: `target_os = "macos"`
     |
     = note: `#[deny(clippy::mismatched_target_os)]` on by default

[github-actions[bot]]

GNU testsuite comparison:

Congrats! The gnu test tests/cp/backup-1 is no longer failing!
GNU test failed: tests/misc/timeout. tests/misc/timeout is passing on 'main'. Maybe you have to rebase?

[github-actions[bot]]

GNU testsuite comparison:

Congrats! The gnu test tests/cp/backup-1 is no longer failing!
Congrats! The gnu test tests/cp/existing-perm-race is no longer failing!