Make mv command fallback to copy only if the src and dst are on different device

[hamflx]

trafficstars

fixes #6029

[tertsdiepraam]

Thanks for putting this up! I have two questions reading this that I think we need to answer before merging this.

1. Can we think of any other cases where the rename call fails that we need to take into account?

2. What does GNU do? We can't look at the source code, but we can use strace on GNU mv to see what syscalls it performs. I'd like to know whether they also give up after a failed rename.

[github-actions[bot]]

GNU testsuite comparison:

GNU test failed: tests/mv/backup-dir. tests/mv/backup-dir is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

[hamflx]

@tertsdiepraam

1. maybe EIO should be considered (such as the file system does not support move operation i gussed)?
2. yes, the gnu mv give up copying files after EACCES fired:

   renameat2(AT_FDCWD, "docs", AT_FDCWD, "../some-docs", RENAME_NOREPLACE) = -1 EACCES (Permission denied)
   newfstatat(AT_FDCWD, "../some-docs", 0x7ffd6d9a2cd0, 0) = -1 ENOENT (No such file or directory)
   newfstatat(AT_FDCWD, "docs", {st_mode=S_IFDIR|0755, st_size=132, ...}, AT_SYMLINK_NOFOLLOW) = 0
   newfstatat(AT_FDCWD, "../some-docs", 0x7ffd6d9a2830, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
   ....
   write(2, ": Permission denied", 19: Permission denied)
   

   it fallback to copy if we copy files across device (mv docs /tmp/some-docs):

   renameat2(AT_FDCWD, "docs", AT_FDCWD, "/tmp/some-docs", RENAME_NOREPLACE) = -1 EXDEV (Invalid cross-device link)
   newfstatat(AT_FDCWD, "/tmp/some-docs", 0x7ffc6b18c1b0, 0) = -1 ENOENT (No such file or directory)
   newfstatat(AT_FDCWD, "docs", {st_mode=S_IFDIR|0755, st_size=132, ...}, AT_SYMLINK_NOFOLLOW) = 0
   newfstatat(AT_FDCWD, "/tmp/some-docs", 0x7ffc6b18bd10, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
   rmdir("/tmp/some-docs")                 = -1 ENOENT (No such file or directory)
   lgetxattr("docs", "security.selinux", "unconfined_u:object_r:user_tmp_t"..., 255) = 36
   access("/var/run/setrans/.setrans-unix", F_OK) = -1 ENOENT (No such file or directory)
   futex(0x7fd465c55520, FUTEX_WAKE_PRIVATE, 2147483647) = 0
   futex(0x7fd465c56644, FUTEX_WAKE_PRIVATE, 2147483647) = 0
   openat(AT_FDCWD, "/proc/thread-self/attr/fscreate", O_RDWR|O_CLOEXEC) = 3
   write(3, "unconfined_u:object_r:user_tmp_t"..., 36) = 36
   close(3)                                = 0
   mkdir("/tmp/some-docs", 0700)           = 0
   newfstatat(AT_FDCWD, "/tmp/some-docs", {st_mode=S_IFDIR|0700, st_size=6, ...}, AT_SYMLINK_NOFOLLOW) = 0
   openat(AT_FDCWD, "docs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 3
   fstat(3, {st_mode=S_IFDIR|0755, st_size=132, ...}) = 0
   getdents64(3, 0x561b1e9f5fc0 /* 9 entries */, 32768) = 280
   getdents64(3, 0x561b1e9f5fc0 /* 0 entries */, 32768) = 0
   close(3)                                = 0
   newfstatat(AT_FDCWD, "docs/src", {st_mode=S_IFDIR|0755, st_size=4096, ...}, AT_SYMLINK_NOFOLLOW) = 0
   newfstatat(AT_FDCWD, "/tmp/some-docs/src", 0x7ffc6b18b7c0, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
   rmdir("/tmp/some-docs/src")             = -1 ENOENT (No such file or directory)
   lgetxattr("docs/src", "security.selinux", "unconfined_u:object_r:user_tmp_t"..., 255) = 36
   mkdir("/tmp/some-docs/src", 0700)       = 0
   newfstatat(AT_FDCWD, "/tmp/some-docs/src", {st_mode=S_IFDIR|0700, st_size=6, ...}, AT_SYMLINK_NOFOLLOW) = 0
   openat(AT_FDCWD, "docs/src", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 3
   fstat(3, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
   getdents64(3, 0x561b1e9fe1d0 /* 15 entries */, 32768) = 512
   getdents64(3, 0x561b1e9fe1d0 /* 0 entries */, 32768) = 0
   close(3)                                = 0
   ...
   

[github-actions[bot]]

GNU testsuite comparison:

GNU test failed: tests/mv/backup-dir. tests/mv/backup-dir is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/chown/preserve-root is no longer failing!

[sylvestre]

Two jobs are failing. Minor stuff :)

[github-actions[bot]]

GNU testsuite comparison:

GNU test failed: tests/mv/backup-dir. tests/mv/backup-dir is passing on 'main'. Maybe you have to rebase?

[hamflx]

@sylvestre i have fixed cSpell and Cargo.toml formatting issues.

GNU testsuite comparison:

GNU test failed: tests/mv/backup-dir. tests/mv/backup-dir is passing on 'main'. Maybe you have to rebase?

but it does not seem to be caused my changes.

[github-actions[bot]]

GNU testsuite comparison:

GNU test failed: tests/mv/backup-dir. tests/mv/backup-dir is passing on 'main'. Maybe you have to rebase?

[sylvestre]

GNU testsuite comparison:

GNU test failed: tests/mv/backup-dir. tests/mv/backup-dir is passing on 'main'. Maybe you have to rebase?

sorry but it is very very likely your change here

[hamflx]

@sylvestre Thanks for your patience. I have found the problem, the call to std::fs::rename("E/", "E/.~1~") reported an error: EINVAL. This is not an EXDEV error, so I made it return immediately.

But I don't think it's my fault, because the GNU tests running mv -T --backup=numbered C E/ || fail=1 will move E to E.~1~, but uu_mv will move E to E/.~1~. This is clearly incorrect.

Before my changes, the GNU tests passed, but the result is incorrect. see the outputs below:

hamflx@hamflx-workstation:~/coreutils/test$ git fetch origin
hamflx@hamflx-workstation:~/coreutils/test$ git reset --hard origin/main
HEAD is now at dcb53b6c9 head: add missing features
hamflx@hamflx-workstation:~/coreutils/test$ cargo build -p uu_mv
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
hamflx@hamflx-workstation:~/coreutils/test$ find . -delete && mkdir C D E && touch C/c D/d E/e
hamflx@hamflx-workstation:~/coreutils/test$ mv -T --backup=numbered C E/
hamflx@hamflx-workstation:~/coreutils/test$ tree -a
.
├── D
│   └── d
├── E
│   └── c
└── E.~1~
    └── e

3 directories, 3 files
hamflx@hamflx-workstation:~/coreutils/test$ find . -delete && mkdir C D E && touch C/c D/d E/e
hamflx@hamflx-workstation:~/coreutils/test$ ../target/debug/mv -T --backup=numbered C E/
hamflx@hamflx-workstation:~/coreutils/test$ tree -a
.
├── D
│   └── d
└── E
    └── c

2 directories, 2 files

I have created a new branch to try to fix this problem: #6119. The outputs after this change:

➜  test git:(fix/invalid-backup-numbered-path) find . -delete && mkdir C D E && touch C/c D/d E/e
➜  test git:(fix/invalid-backup-numbered-path) ../target/debug/mv -T --backup=numbered C E/
➜  test git:(fix/invalid-backup-numbered-path) tree -a
.
├── D
│   └── d
├── E
│   └── c
└── E.~1~
    └── e

3 directories, 3 files

[github-actions[bot]]

GNU testsuite comparison:

GNU test failed: tests/mv/backup-dir. tests/mv/backup-dir is passing on 'main'. Maybe you have to rebase?