From 8ec6cd2d4905ae69c35227a51e3ca4b8f25c1798 Mon Sep 17 00:00:00 2001 From: hz2 Date: Fri, 25 Jul 2025 13:20:30 -0700 Subject: [PATCH 1/2] fixes #8220 implements the --exchange flag for linux systems with testing clippy checks - added test for testing -T - switched print statement to match GNU exchange format - switched to using the translate! macro instead of get_message took out duplicate uses removed mv-exchange checks in build-gnu.sh cargo fmt swapped RENAME_EXCHANGE with nix renameat Cargo.lock clippy fix formatting, test error messages and reverted build-gnu.sh --- Cargo.lock | 1 + src/uu/mv/Cargo.toml | 1 + src/uu/mv/locales/en-US.ftl | 7 ++ src/uu/mv/locales/fr-FR.ftl | 7 ++ src/uu/mv/src/mv.rs | 99 +++++++++++++++++++++- tests/by-util/test_mv.rs | 160 ++++++++++++++++++++++++++++++++++++ 6 files changed, 274 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index b8c738ac596..8086dd0c72a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3492,6 +3492,7 @@ dependencies = [ "fs_extra", "indicatif", "libc", + "nix", "thiserror 2.0.12", "uucore", "windows-sys 0.60.2", diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index 9958626090b..b06f89f48ed 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -40,6 +40,7 @@ windows-sys = { workspace = true, features = [ [target.'cfg(unix)'.dependencies] libc = { workspace = true } +nix = { workspace = true, features = ["fs"] } [[bin]] name = "mv" diff --git a/src/uu/mv/locales/en-US.ftl b/src/uu/mv/locales/en-US.ftl index 07d1155b89b..1a9993397c8 100644 --- a/src/uu/mv/locales/en-US.ftl +++ b/src/uu/mv/locales/en-US.ftl @@ -26,6 +26,12 @@ mv-error-not-directory = target {$path}: Not a directory mv-error-target-not-directory = target directory {$path}: Not a directory mv-error-failed-access-not-directory = failed to access {$path}: Not a directory mv-error-backup-with-no-clobber = cannot combine --backup with -n/--no-clobber or --update=none-fail +mv-error-exchange-needs-two-files = --exchange requires exactly two files to exchange +mv-error-exchange-conflicts-with-target-directory = --exchange conflicts with --target-directory +mv-error-exchange-conflicts-with-backup = --exchange conflicts with backup options +mv-error-exchange-conflicts-with-update = --exchange conflicts with update options +mv-error-exchange-not-supported = --exchange is not supported on this system +mv-error-exchange-cross-device = --exchange requires both files to be on the same filesystem mv-error-extra-operand = mv: extra operand {$operand} mv-error-backup-might-destroy-source = backing up {$target} might destroy source; {$source} not moved mv-error-will-not-overwrite-just-created = will not overwrite just-created '{$target}' with '{$source}' @@ -48,6 +54,7 @@ mv-help-verbose = explain what is being done mv-help-progress = Display a progress bar. Note: this feature is not supported by GNU coreutils. mv-help-debug = explain how a file is copied. Implies -v +mv-help-exchange = exchange two files atomically (Linux only) # Verbose messages mv-verbose-renamed = renamed {$from} -> {$to} diff --git a/src/uu/mv/locales/fr-FR.ftl b/src/uu/mv/locales/fr-FR.ftl index 25d82631418..1d04f07b481 100644 --- a/src/uu/mv/locales/fr-FR.ftl +++ b/src/uu/mv/locales/fr-FR.ftl @@ -26,6 +26,12 @@ mv-error-not-directory = cible {$path} : N'est pas un répertoire mv-error-target-not-directory = répertoire cible {$path} : N'est pas un répertoire mv-error-failed-access-not-directory = impossible d'accéder à {$path} : N'est pas un répertoire mv-error-backup-with-no-clobber = impossible de combiner --backup avec -n/--no-clobber ou --update=none-fail +mv-error-exchange-needs-two-files = --exchange nécessite exactement deux fichiers à échanger +mv-error-exchange-conflicts-with-target-directory = --exchange est en conflit avec --target-directory +mv-error-exchange-conflicts-with-backup = --exchange est en conflit avec les options de sauvegarde +mv-error-exchange-conflicts-with-update = --exchange est en conflit avec les options de mise à jour +mv-error-exchange-not-supported = --exchange n'est pas pris en charge sur ce système +mv-error-exchange-cross-device = --exchange nécessite que les deux fichiers soient sur le même système de fichiers mv-error-extra-operand = mv : opérande supplémentaire {$operand} mv-error-backup-might-destroy-source = sauvegarder {$target} pourrait détruire la source ; {$source} non déplacé mv-error-will-not-overwrite-just-created = ne va pas écraser le fichier qui vient d'être créé '{$target}' avec '{$source}' @@ -48,6 +54,7 @@ mv-help-verbose = expliquer ce qui est fait mv-help-progress = Afficher une barre de progression. Note : cette fonctionnalité n'est pas supportée par GNU coreutils. mv-help-debug = expliquer comment un fichier est copié. Implique -v +mv-help-exchange = échanger deux fichiers de manière atomique (Linux uniquement) # Messages verbeux mv-verbose-renamed = renommé {$from} -> {$to} diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 6576d971784..abd9f05df2d 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) sourcepath targetpath nushell canonicalized +// spell-checker:ignore (ToDO) sourcepath targetpath nushell canonicalized renameat FDCWD ENOTSUP mod error; #[cfg(unix)] @@ -99,6 +99,9 @@ pub struct Options { /// `--debug` pub debug: bool, + + /// `--exchange` + pub exchange: bool, } impl Default for Options { @@ -114,6 +117,7 @@ impl Default for Options { strip_slashes: false, progress_bar: false, debug: false, + exchange: false, } } } @@ -140,6 +144,7 @@ static OPT_VERBOSE: &str = "verbose"; static OPT_PROGRESS: &str = "progress"; static ARG_FILES: &str = "files"; static OPT_DEBUG: &str = "debug"; +static OPT_EXCHANGE: &str = "exchange"; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { @@ -177,6 +182,34 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { )); } + // Validate exchange flag + if matches.get_flag(OPT_EXCHANGE) { + if files.len() != 2 { + return Err(UUsageError::new( + 1, + translate!("--exchange requires exactly two files"), + )); + } + if matches.contains_id(OPT_TARGET_DIRECTORY) { + return Err(UUsageError::new( + 1, + translate!("--exchange conflicts with --target-directory"), + )); + } + if backup_mode != BackupMode::None { + return Err(UUsageError::new( + 1, + translate!("--exchange conflicts with backup options"), + )); + } + if update_mode != UpdateMode::All { + return Err(UUsageError::new( + 1, + translate!("--exchange conflicts with update options"), + )); + } + } + let backup_suffix = backup_control::determine_backup_suffix(&matches); let target_dir = matches @@ -200,6 +233,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { strip_slashes: matches.get_flag(OPT_STRIP_TRAILING_SLASHES), progress_bar: matches.get_flag(OPT_PROGRESS), debug: matches.get_flag(OPT_DEBUG), + exchange: matches.get_flag(OPT_EXCHANGE), }; mv(&files[..], &opts) @@ -296,6 +330,12 @@ pub fn uu_app() -> Command { .help(translate!("mv-help-debug")) .action(ArgAction::SetTrue), ) + .arg( + Arg::new(OPT_EXCHANGE) + .long(OPT_EXCHANGE) + .help(translate!("exchange two files")) + .action(ArgAction::SetTrue), + ) } fn determine_overwrite_mode(matches: &ArgMatches) -> OverwriteMode { @@ -313,6 +353,52 @@ fn determine_overwrite_mode(matches: &ArgMatches) -> OverwriteMode { } } +/// Atomically exchange two files using renameat2 with `RENAME_EXCHANGE` +#[cfg(target_os = "linux")] +fn exchange_files(path1: &Path, path2: &Path, opts: &Options) -> UResult<()> { + use nix::fcntl::{AT_FDCWD, RenameFlags, renameat2}; + + // Use renameat2 to atomically exchange the files + match renameat2( + AT_FDCWD, + path1, + AT_FDCWD, + path2, + RenameFlags::RENAME_EXCHANGE, + ) { + Ok(()) => { + if opts.verbose { + println!("exchanged '{}' <-> '{}'", path1.display(), path2.display()); + } + Ok(()) + } + Err(err) => match err { + nix::Error::ENOTSUP | nix::Error::EINVAL => Err(USimpleError::new( + 1, + translate!("--exchange is not supported on this filesystem"), + )), + nix::Error::ENOENT => { + let missing_path = if path1.exists() { path2 } else { path1 }; + Err(MvError::NoSuchFile(missing_path.display().to_string()).into()) + } + nix::Error::EXDEV => Err(USimpleError::new( + 1, + translate!("--exchange cannot exchange files across different filesystems"), + )), + _ => Err(USimpleError::new(1, format!("exchange failed: {err}"))), + }, + } +} + +/// Fallback exchange implementation for non-Linux systems +#[cfg(not(target_os = "linux"))] +fn exchange_files(_path1: &Path, _path2: &Path, _opts: &Options) -> UResult<()> { + Err(USimpleError::new( + 1, + translate!("--exchange is not supported on this system"), + )) +} + fn parse_paths(files: &[OsString], opts: &Options) -> Vec { let paths = files.iter().map(Path::new); @@ -520,6 +606,17 @@ fn handle_multiple_paths(paths: &[PathBuf], opts: &Options) -> UResult<()> { pub fn mv(files: &[OsString], opts: &Options) -> UResult<()> { let paths = parse_paths(files, opts); + // Handle exchange mode + if opts.exchange { + if paths.len() != 2 { + return Err(USimpleError::new( + 1, + translate!("--exchange requires exactly two files"), + )); + } + return exchange_files(&paths[0], &paths[1], opts); + } + if let Some(ref name) = opts.target_dir { return move_files_into_dir(&paths, &PathBuf::from(name), opts); } diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 62c12c1d29a..c530d6d1cb6 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2488,3 +2488,163 @@ fn test_mv_cross_device_permission_denied() { set_permissions(other_fs_tempdir.path(), PermissionsExt::from_mode(0o755)) .expect("Unable to restore directory permissions"); } + +// Tests for --exchange flag +#[test] +#[cfg(target_os = "linux")] +fn test_mv_exchange_basic() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + at.write("file2", "content2"); + + ucmd.arg("--exchange").arg("file1").arg("file2").succeeds(); + + // After exchange, file1 should have content2 and file2 should have content1 + assert_eq!(at.read("file1"), "content2"); + assert_eq!(at.read("file2"), "content1"); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_mv_exchange_verbose() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + at.write("file2", "content2"); + + ucmd.arg("--exchange") + .arg("--verbose") + .arg("file1") + .arg("file2") + .succeeds() + .stdout_contains("exchanged 'file1' <-> 'file2'"); +} + +#[test] +fn test_mv_exchange_wrong_number_of_args() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + + ucmd.arg("--exchange") + .arg("file1") + .fails() + .stderr_contains("requires at least 2 values"); +} + +#[test] +fn test_mv_exchange_three_files() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + at.write("file2", "content2"); + at.write("file3", "content3"); + + ucmd.arg("--exchange") + .arg("file1") + .arg("file2") + .arg("file3") + .fails() + .stderr_contains("--exchange requires exactly two files"); +} + +#[test] +fn test_mv_exchange_conflicts_with_target_directory() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + at.write("file2", "content2"); + at.mkdir("dir"); + + ucmd.arg("--exchange") + .arg("--target-directory") + .arg("dir") + .arg("file1") + .arg("file2") + .fails() + .stderr_contains("--exchange conflicts with --target-directory"); +} + +#[test] +fn test_mv_exchange_conflicts_with_backup() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + at.write("file2", "content2"); + + ucmd.arg("--exchange") + .arg("--backup") + .arg("file1") + .arg("file2") + .fails() + .stderr_contains("--exchange conflicts with backup options"); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_mv_exchange_missing_file() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + // file2 doesn't exist + + ucmd.arg("--exchange") + .arg("file1") + .arg("file2") + .fails() + .stderr_contains("cannot stat file2: No such file or directory"); +} + +#[test] +#[cfg(not(target_os = "linux"))] +fn test_mv_exchange_missing_file() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + // file2 doesn't exist + + ucmd.arg("--exchange") + .arg("file1") + .arg("file2") + .fails() + .stderr_contains("--exchange is not supported on this system"); +} + +#[test] +#[cfg(not(target_os = "linux"))] +fn test_mv_exchange_not_supported() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file1", "content1"); + at.write("file2", "content2"); + + ucmd.arg("--exchange") + .arg("file1") + .arg("file2") + .fails() + .stderr_contains("--exchange is not supported on this system"); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_mv_exchange_with_no_target_directory() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.mkdir("d1"); + at.mkdir("d2"); + at.write("d1/file1", "content1"); + at.write("d2/file2", "content2"); + + ucmd.arg("-T") + .arg("--exchange") + .arg("d1") + .arg("d2") + .succeeds(); + + // after exchange, d1 should contain file2 and d2 should contain file1 + assert_eq!(at.read("d1/file2"), "content2"); + assert_eq!(at.read("d2/file1"), "content1"); + assert!(!at.file_exists("d1/file1")); + assert!(!at.file_exists("d2/file2")); +} From 8a94ea882bb19a13fd5873f0d3e20f4887eb9e78 Mon Sep 17 00:00:00 2001 From: hz2 Date: Wed, 30 Jul 2025 20:10:15 -0700 Subject: [PATCH 2/2] revert back to raw libc syscalls --- Cargo.lock | 1 - src/uu/mv/Cargo.toml | 1 - src/uu/mv/src/mv.rs | 57 +++++++++++++++++++++++++++++--------------- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8086dd0c72a..b8c738ac596 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3492,7 +3492,6 @@ dependencies = [ "fs_extra", "indicatif", "libc", - "nix", "thiserror 2.0.12", "uucore", "windows-sys 0.60.2", diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index b06f89f48ed..9958626090b 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -40,7 +40,6 @@ windows-sys = { workspace = true, features = [ [target.'cfg(unix)'.dependencies] libc = { workspace = true } -nix = { workspace = true, features = ["fs"] } [[bin]] name = "mv" diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index abd9f05df2d..fed7a6f0c3d 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -356,37 +356,56 @@ fn determine_overwrite_mode(matches: &ArgMatches) -> OverwriteMode { /// Atomically exchange two files using renameat2 with `RENAME_EXCHANGE` #[cfg(target_os = "linux")] fn exchange_files(path1: &Path, path2: &Path, opts: &Options) -> UResult<()> { - use nix::fcntl::{AT_FDCWD, RenameFlags, renameat2}; + use std::ffi::CString; + use std::os::unix::ffi::OsStrExt; + + // Convert paths to C strings + let c_path1 = CString::new(path1.as_os_str().as_bytes()).unwrap(); + let c_path2 = CString::new(path2.as_os_str().as_bytes()).unwrap(); + + // RENAME_EXCHANGE flag for renameat2 + const RENAME_EXCHANGE: libc::c_int = 2; // Use renameat2 to atomically exchange the files - match renameat2( - AT_FDCWD, - path1, - AT_FDCWD, - path2, - RenameFlags::RENAME_EXCHANGE, - ) { - Ok(()) => { - if opts.verbose { - println!("exchanged '{}' <-> '{}'", path1.display(), path2.display()); - } - Ok(()) + let result = unsafe { + libc::syscall( + libc::SYS_renameat2, + libc::AT_FDCWD, + c_path1.as_ptr(), + libc::AT_FDCWD, + c_path2.as_ptr(), + RENAME_EXCHANGE, + ) + }; + + if result == 0 { + if opts.verbose { + println!("exchanged '{}' <-> '{}'", path1.display(), path2.display()); } - Err(err) => match err { - nix::Error::ENOTSUP | nix::Error::EINVAL => Err(USimpleError::new( + Ok(()) + } else { + let errno = unsafe { *libc::__errno_location() }; + match errno { + libc::ENOTSUP | libc::EINVAL => Err(USimpleError::new( 1, translate!("--exchange is not supported on this filesystem"), )), - nix::Error::ENOENT => { + libc::ENOENT => { let missing_path = if path1.exists() { path2 } else { path1 }; Err(MvError::NoSuchFile(missing_path.display().to_string()).into()) } - nix::Error::EXDEV => Err(USimpleError::new( + libc::EXDEV => Err(USimpleError::new( 1, translate!("--exchange cannot exchange files across different filesystems"), )), - _ => Err(USimpleError::new(1, format!("exchange failed: {err}"))), - }, + _ => { + let error_msg = io::Error::from_raw_os_error(errno); + Err(USimpleError::new( + 1, + format!("exchange failed: {error_msg}"), + )) + } + } } }