From 403e71eff3aacadf2b03232c2930a5470a1b690f Mon Sep 17 00:00:00 2001 From: hamflx Date: Fri, 1 Mar 2024 20:46:21 +0800 Subject: [PATCH 1/2] mv: Make mv command fallback to copy only if the src and dst are on different device --- .../workspace.wordlist.txt | 1 + Cargo.lock | 2 + src/uu/mv/Cargo.toml | 6 +++ src/uu/mv/src/mv.rs | 10 ++++- tests/by-util/test_mv.rs | 45 +++++++++++++++++++ 5 files changed, 63 insertions(+), 1 deletion(-) diff --git a/.vscode/cspell.dictionaries/workspace.wordlist.txt b/.vscode/cspell.dictionaries/workspace.wordlist.txt index ee34a38110e..45373d95c72 100644 --- a/.vscode/cspell.dictionaries/workspace.wordlist.txt +++ b/.vscode/cspell.dictionaries/workspace.wordlist.txt @@ -136,6 +136,7 @@ vmsplice # * vars/libc COMFOLLOW +EXDEV FILENO FTSENT HOSTSIZE diff --git a/Cargo.lock b/Cargo.lock index 42aeed44e36..5e05d2ecf05 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2989,8 +2989,10 @@ dependencies = [ "clap", "fs_extra", "indicatif", + "libc", "thiserror 2.0.11", "uucore", + "windows-sys 0.59.0", ] [[package]] diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index 13b40f7fb0c..adcb09db261 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -28,6 +28,12 @@ uucore = { workspace = true, features = [ ] } thiserror = { workspace = true } +[target.'cfg(windows)'.dependencies] +windows-sys = { workspace = true, features = ["Win32_Foundation"] } + +[target.'cfg(unix)'.dependencies] +libc = { workspace = true } + [[bin]] name = "mv" path = "src/main.rs" diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 6e533dace85..2d88c410dc9 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -657,7 +657,15 @@ fn rename_with_fallback( to: &Path, multi_progress: Option<&MultiProgress>, ) -> io::Result<()> { - if fs::rename(from, to).is_err() { + if let Err(err) = fs::rename(from, to) { + // We will only copy if they're not on the same device, otherwise we'll report an error. + #[cfg(windows)] + const EXDEV: i32 = windows_sys::Win32::Foundation::ERROR_NOT_SAME_DEVICE as _; + #[cfg(unix)] + const EXDEV: i32 = libc::EXDEV as _; + if err.raw_os_error() != Some(EXDEV) { + return Err(err); + } // Get metadata without following symlinks let metadata = from.symlink_metadata()?; let file_type = metadata.file_type(); diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 6441357f12e..20881adf083 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1670,6 +1670,51 @@ fn test_acl() { assert!(compare_xattrs(&file, &file_target)); } +#[test] +#[cfg(windows)] +fn test_move_should_not_fallback_to_copy() { + use std::os::windows::fs::OpenOptionsExt; + + let (at, mut ucmd) = at_and_ucmd!(); + + let locked_file = "a_file_is_locked"; + let locked_file_path = at.plus(locked_file); + let file = std::fs::OpenOptions::new() + .create(true) + .write(true) + .share_mode( + uucore::windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ + | uucore::windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE, + ) + .open(locked_file_path); + + let target_file = "target_file"; + ucmd.arg(locked_file).arg(target_file).fails(); + + assert!(at.file_exists(locked_file)); + assert!(!at.file_exists(target_file)); + + drop(file); +} + +#[test] +#[cfg(unix)] +fn test_move_should_not_fallback_to_copy() { + let (at, mut ucmd) = at_and_ucmd!(); + + let readonly_dir = "readonly_dir"; + let locked_file = "readonly_dir/a_file_is_locked"; + at.mkdir(readonly_dir); + at.touch(locked_file); + at.set_mode(readonly_dir, 0o555); + + let target_file = "target_file"; + ucmd.arg(locked_file).arg(target_file).fails(); + + assert!(at.file_exists(locked_file)); + assert!(!at.file_exists(target_file)); +} + // Todo: // $ at.touch a b From f1f2be90540f004e555a220778d7ac1bf4ede316 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 18 Jan 2025 12:14:44 -0500 Subject: [PATCH 2/2] Skip test on Windows --- src/uu/mv/src/mv.rs | 18 +++++++++++++----- tests/by-util/test_mv.rs | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 2d88c410dc9..582ea7aa229 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -650,6 +650,17 @@ fn rename( Ok(()) } +#[cfg(unix)] +fn is_err_not_same_device(err: &std::io::Error) -> bool { + matches!(err.raw_os_error(), Some(libc::EXDEV)) +} + +#[cfg(windows)] +fn is_err_not_same_device(err: &std::io::Error) -> bool { + let errno = windows_sys::Win32::Foundation::ERROR_NOT_SAME_DEVICE as i32; + matches!(err.raw_os_error(), Some(e) if e == errno) +} + /// A wrapper around `fs::rename`, so that if it fails, we try falling back on /// copying and removing. fn rename_with_fallback( @@ -659,13 +670,10 @@ fn rename_with_fallback( ) -> io::Result<()> { if let Err(err) = fs::rename(from, to) { // We will only copy if they're not on the same device, otherwise we'll report an error. - #[cfg(windows)] - const EXDEV: i32 = windows_sys::Win32::Foundation::ERROR_NOT_SAME_DEVICE as _; - #[cfg(unix)] - const EXDEV: i32 = libc::EXDEV as _; - if err.raw_os_error() != Some(EXDEV) { + if !is_err_not_same_device(&err) { return Err(err); } + // Get metadata without following symlinks let metadata = from.symlink_metadata()?; let file_type = metadata.file_type(); diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 20881adf083..f5f2d4c007a 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1670,6 +1670,7 @@ fn test_acl() { assert!(compare_xattrs(&file, &file_target)); } +#[ignore = "broken on windows"] #[test] #[cfg(windows)] fn test_move_should_not_fallback_to_copy() {