diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index a0ff1bcc6..c61c7caf1 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -20,6 +20,7 @@ use std::os::unix; #[cfg(windows)] use std::os::windows; use std::path::{Path, PathBuf}; +use uucore::backup_control::{self, BackupMode}; use fs_extra::dir::{move_dir, CopyOptions as DirCopyOptions}; @@ -40,16 +41,9 @@ pub enum OverwriteMode { Force, } -#[derive(Clone, Copy, Eq, PartialEq)] -pub enum BackupMode { - NoBackup, - SimpleBackup, - NumberedBackup, - ExistingBackup, -} - static ABOUT: &str = "Move SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY."; static VERSION: &str = env!("CARGO_PKG_VERSION"); +static LONG_HELP: &str = ""; static OPT_BACKUP: &str = "backup"; static OPT_BACKUP_NO_ARG: &str = "b"; @@ -80,20 +74,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let matches = App::new(executable!()) .version(VERSION) .about(ABOUT) + .after_help(&*format!("{}\n{}", LONG_HELP, backup_control::BACKUP_CONTROL_LONG_HELP)) .usage(&usage[..]) .arg( Arg::with_name(OPT_BACKUP) .long(OPT_BACKUP) .help("make a backup of each existing destination file") .takes_value(true) - .possible_value("simple") - .possible_value("never") - .possible_value("numbered") - .possible_value("t") - .possible_value("existing") - .possible_value("nil") - .possible_value("none") - .possible_value("off") + .require_equals(true) + .min_values(0) + .possible_values(backup_control::BACKUP_CONTROL_VALUES) .value_name("CONTROL") ) .arg( @@ -172,18 +162,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .unwrap_or_default(); let overwrite_mode = determine_overwrite_mode(&matches); - let backup_mode = determine_backup_mode(&matches); + let backup_mode = backup_control::determine_backup_mode( + matches.is_present(OPT_BACKUP_NO_ARG) || matches.is_present(OPT_BACKUP), + matches.value_of(OPT_BACKUP), + ); if overwrite_mode == OverwriteMode::NoClobber && backup_mode != BackupMode::NoBackup { - show_error!( - "options --backup and --no-clobber are mutually exclusive\n\ - Try '{} --help' for more information.", - executable!() - ); + show_usage_error!("options --backup and --no-clobber are mutually exclusive"); return 1; } - let backup_suffix = determine_backup_suffix(backup_mode, &matches); + let backup_suffix = backup_control::determine_backup_suffix(matches.value_of(OPT_SUFFIX)); let behavior = Behavior { overwrite: overwrite_mode, @@ -227,37 +216,6 @@ fn determine_overwrite_mode(matches: &ArgMatches) -> OverwriteMode { } } -fn determine_backup_mode(matches: &ArgMatches) -> BackupMode { - if matches.is_present(OPT_BACKUP_NO_ARG) { - BackupMode::SimpleBackup - } else if matches.is_present(OPT_BACKUP) { - match matches.value_of(OPT_BACKUP).map(String::from) { - None => BackupMode::SimpleBackup, - Some(mode) => match &mode[..] { - "simple" | "never" => BackupMode::SimpleBackup, - "numbered" | "t" => BackupMode::NumberedBackup, - "existing" | "nil" => BackupMode::ExistingBackup, - "none" | "off" => BackupMode::NoBackup, - _ => panic!(), // cannot happen as it is managed by clap - }, - } - } else { - BackupMode::NoBackup - } -} - -fn determine_backup_suffix(backup_mode: BackupMode, matches: &ArgMatches) -> String { - if matches.is_present(OPT_SUFFIX) { - matches.value_of(OPT_SUFFIX).map(String::from).unwrap() - } else if let (Ok(s), BackupMode::SimpleBackup) = - (env::var("SIMPLE_BACKUP_SUFFIX"), backup_mode) - { - s - } else { - "~".to_owned() - } -} - fn exec(files: &[PathBuf], b: Behavior) -> i32 { if let Some(ref name) = b.target_dir { return move_files_into_dir(files, &PathBuf::from(name), &b); @@ -389,12 +347,7 @@ fn rename(from: &Path, to: &Path, b: &Behavior) -> io::Result<()> { OverwriteMode::Force => {} }; - backup_path = match b.backup { - BackupMode::NoBackup => None, - BackupMode::SimpleBackup => Some(simple_backup_path(to, &b.suffix)), - BackupMode::NumberedBackup => Some(numbered_backup_path(to)), - BackupMode::ExistingBackup => Some(existing_backup_path(to, &b.suffix)), - }; + backup_path = backup_control::get_backup_path(b.backup, to, &b.suffix); if let Some(ref backup_path) = backup_path { rename_with_fallback(to, backup_path)?; } @@ -514,28 +467,6 @@ fn read_yes() -> bool { } } -fn simple_backup_path(path: &Path, suffix: &str) -> PathBuf { - let mut p = path.to_string_lossy().into_owned(); - p.push_str(suffix); - PathBuf::from(p) -} - -fn numbered_backup_path(path: &Path) -> PathBuf { - (1_u64..) - .map(|i| path.with_extension(format!("~{}~", i))) - .find(|p| !p.exists()) - .expect("cannot create backup") -} - -fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf { - let test_path = path.with_extension("~1~"); - if test_path.exists() { - numbered_backup_path(path) - } else { - simple_backup_path(path, suffix) - } -} - fn is_empty_dir(path: &Path) -> bool { match fs::read_dir(path) { Ok(contents) => contents.peekable().peek().is_none(), diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index e0723a479..e0bdd9ef3 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -251,6 +251,40 @@ fn test_mv_simple_backup() { assert!(at.file_exists(&format!("{}~", file_b))); } +#[test] +fn test_mv_simple_backup_with_file_extension() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_simple_backup_file_a.txt"; + let file_b = "test_mv_simple_backup_file_b.txt"; + + at.touch(file_a); + at.touch(file_b); + ucmd.arg("-b") + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(!at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(&format!("{}~", file_b))); +} + +#[test] +fn test_mv_arg_backup_arg_first() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_simple_backup_file_a"; + let file_b = "test_mv_simple_backup_file_b"; + + at.touch(file_a); + at.touch(file_b); + ucmd.arg("--backup").arg(file_a).arg(file_b).succeeds(); + + assert!(!at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(&format!("{}~", file_b))); +} + #[test] fn test_mv_custom_backup_suffix() { let (at, mut ucmd) = at_and_ucmd!(); @@ -293,7 +327,7 @@ fn test_mv_custom_backup_suffix_via_env() { } #[test] -fn test_mv_backup_numbering() { +fn test_mv_backup_numbered_with_t() { let (at, mut ucmd) = at_and_ucmd!(); let file_a = "test_mv_backup_numbering_file_a"; let file_b = "test_mv_backup_numbering_file_b"; @@ -311,6 +345,25 @@ fn test_mv_backup_numbering() { assert!(at.file_exists(&format!("{}.~1~", file_b))); } +#[test] +fn test_mv_backup_numbered() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_backup_numbering_file_a"; + let file_b = "test_mv_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + ucmd.arg("--backup=numbered") + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(!at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(&format!("{}.~1~", file_b))); +} + #[test] fn test_mv_backup_existing() { let (at, mut ucmd) = at_and_ucmd!(); @@ -330,6 +383,67 @@ fn test_mv_backup_existing() { assert!(at.file_exists(&format!("{}~", file_b))); } +#[test] +fn test_mv_backup_nil() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_backup_numbering_file_a"; + let file_b = "test_mv_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + ucmd.arg("--backup=nil") + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(!at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(&format!("{}~", file_b))); +} + +#[test] +fn test_mv_numbered_if_existing_backup_existing() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_backup_numbering_file_a"; + let file_b = "test_mv_backup_numbering_file_b"; + let file_b_backup = "test_mv_backup_numbering_file_b.~1~"; + + at.touch(file_a); + at.touch(file_b); + at.touch(file_b_backup); + ucmd.arg("--backup=existing") + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(file_b)); + assert!(at.file_exists(file_b_backup)); + assert!(at.file_exists(&*format!("{}.~2~", file_b))); +} + +#[test] +fn test_mv_numbered_if_existing_backup_nil() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_backup_numbering_file_a"; + let file_b = "test_mv_backup_numbering_file_b"; + let file_b_backup = "test_mv_backup_numbering_file_b.~1~"; + + at.touch(file_a); + at.touch(file_b); + at.touch(file_b_backup); + ucmd.arg("--backup=nil") + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(file_b)); + assert!(at.file_exists(file_b_backup)); + assert!(at.file_exists(&*format!("{}.~2~", file_b))); +} + #[test] fn test_mv_backup_simple() { let (at, mut ucmd) = at_and_ucmd!(); @@ -349,6 +463,25 @@ fn test_mv_backup_simple() { assert!(at.file_exists(&format!("{}~", file_b))); } +#[test] +fn test_mv_backup_never() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_backup_numbering_file_a"; + let file_b = "test_mv_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + ucmd.arg("--backup=never") + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(!at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(&format!("{}~", file_b))); +} + #[test] fn test_mv_backup_none() { let (at, mut ucmd) = at_and_ucmd!(); @@ -369,17 +502,14 @@ fn test_mv_backup_none() { } #[test] -fn test_mv_existing_backup() { +fn test_mv_backup_off() { let (at, mut ucmd) = at_and_ucmd!(); - let file_a = "test_mv_existing_backup_file_a"; - let file_b = "test_mv_existing_backup_file_b"; - let file_b_backup = "test_mv_existing_backup_file_b.~1~"; - let resulting_backup = "test_mv_existing_backup_file_b.~2~"; + let file_a = "test_mv_backup_numbering_file_a"; + let file_b = "test_mv_backup_numbering_file_b"; at.touch(file_a); at.touch(file_b); - at.touch(file_b_backup); - ucmd.arg("--backup=nil") + ucmd.arg("--backup=off") .arg(file_a) .arg(file_b) .succeeds() @@ -387,8 +517,19 @@ fn test_mv_existing_backup() { assert!(!at.file_exists(file_a)); assert!(at.file_exists(file_b)); - assert!(at.file_exists(file_b_backup)); - assert!(at.file_exists(resulting_backup)); + assert!(!at.file_exists(&format!("{}~", file_b))); +} + +#[test] +fn test_mv_backup_no_clobber_conflicting_options() { + let (_, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup") + .arg("--no-clobber") + .arg("file1") + .arg("file2") + .fails() + .stderr_is("mv: options --backup and --no-clobber are mutually exclusive\nTry 'mv --help' for more information."); } #[test]