diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index e45797750..bd227da56 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -15,6 +15,7 @@ extern crate uucore; use clap::{crate_version, App, Arg, ArgMatches}; use file_diff::diff; use filetime::{set_file_times, FileTime}; +use uucore::backup_control::{self, BackupMode}; use uucore::entries::{grp2gid, usr2uid}; use uucore::perms::{wrap_chgrp, wrap_chown, Verbosity}; @@ -33,6 +34,7 @@ const DEFAULT_STRIP_PROGRAM: &str = "strip"; pub struct Behavior { main_function: MainFunction, specified_mode: Option, + backup_mode: BackupMode, suffix: String, owner: String, group: String, @@ -68,7 +70,7 @@ static ABOUT: &str = "Copy SOURCE to DEST or multiple SOURCE(s) to the existing static OPT_COMPARE: &str = "compare"; static OPT_BACKUP: &str = "backup"; -static OPT_BACKUP_2: &str = "backup2"; +static OPT_BACKUP_NO_ARG: &str = "backup2"; static OPT_DIRECTORY: &str = "directory"; static OPT_IGNORED: &str = "ignored"; static OPT_CREATE_LEADING: &str = "create-leading"; @@ -130,14 +132,17 @@ pub fn uu_app() -> App<'static, 'static> { .arg( Arg::with_name(OPT_BACKUP) .long(OPT_BACKUP) - .help("(unimplemented) make a backup of each existing destination file") + .help("make a backup of each existing destination file") + .takes_value(true) + .require_equals(true) + .min_values(0) .value_name("CONTROL") ) .arg( // TODO implement flag - Arg::with_name(OPT_BACKUP_2) + Arg::with_name(OPT_BACKUP_NO_ARG) .short("b") - .help("(unimplemented) like --backup but does not accept an argument") + .help("like --backup but does not accept an argument") ) .arg( Arg::with_name(OPT_IGNORED) @@ -210,7 +215,7 @@ pub fn uu_app() -> App<'static, 'static> { Arg::with_name(OPT_SUFFIX) .short("S") .long(OPT_SUFFIX) - .help("(unimplemented) override the usual backup suffix") + .help("override the usual backup suffix") .value_name("SUFFIX") .takes_value(true) .min_values(1) @@ -265,13 +270,7 @@ pub fn uu_app() -> App<'static, 'static> { /// /// fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> { - if matches.is_present(OPT_BACKUP) { - Err("--backup") - } else if matches.is_present(OPT_BACKUP_2) { - Err("-b") - } else if matches.is_present(OPT_SUFFIX) { - Err("--suffix, -S") - } else if matches.is_present(OPT_NO_TARGET_DIRECTORY) { + if matches.is_present(OPT_NO_TARGET_DIRECTORY) { Err("--no-target-directory, -T") } else if matches.is_present(OPT_PRESERVE_CONTEXT) { Err("--preserve-context, -P") @@ -309,18 +308,16 @@ fn behavior(matches: &ArgMatches) -> Result { None }; - let backup_suffix = if matches.is_present(OPT_SUFFIX) { - matches.value_of(OPT_SUFFIX).ok_or(1)? - } else { - "~" - }; - let target_dir = matches.value_of(OPT_TARGET_DIRECTORY).map(|d| d.to_owned()); Ok(Behavior { main_function, specified_mode, - suffix: backup_suffix.to_string(), + backup_mode: backup_control::determine_backup_mode( + matches.is_present(OPT_BACKUP_NO_ARG) || matches.is_present(OPT_BACKUP), + matches.value_of(OPT_BACKUP), + ), + suffix: backup_control::determine_backup_suffix(matches.value_of(OPT_SUFFIX)), owner: matches.value_of(OPT_OWNER).unwrap_or("").to_string(), group: matches.value_of(OPT_GROUP).unwrap_or("").to_string(), verbose: matches.is_present(OPT_VERBOSE), @@ -517,6 +514,28 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { if b.compare && !need_copy(from, to, b) { return Ok(()); } + // Declare the path here as we may need it for the verbose output below. + let mut backup_path = None; + + // Perform backup, if any, before overwriting 'to' + // + // The codes actually making use of the backup process don't seem to agree + // on how best to approach the issue. (mv and ln, for example) + if to.exists() { + backup_path = backup_control::get_backup_path(b.backup_mode, to, &b.suffix); + if let Some(ref backup_path) = backup_path { + // TODO!! + if let Err(err) = fs::rename(to, backup_path) { + show_error!( + "install: cannot backup file '{}' to '{}': {}", + to.display(), + backup_path.display(), + err + ); + return Err(()); + } + } + } if from.to_string_lossy() == "/dev/null" { /* workaround a limitation of fs::copy @@ -624,7 +643,11 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { } if b.verbose { - show_error!("'{}' -> '{}'", from.display(), to.display()); + print!("'{}' -> '{}'", from.display(), to.display()); + match backup_path { + Some(path) => println!(" (backup: '{}')", path.display()), + None => println!(), + } } Ok(()) diff --git a/src/uucore/src/lib/mods/backup_control.rs b/src/uucore/src/lib/mods/backup_control.rs index 83268d351..b8f389c83 100644 --- a/src/uucore/src/lib/mods/backup_control.rs +++ b/src/uucore/src/lib/mods/backup_control.rs @@ -37,6 +37,19 @@ pub fn determine_backup_suffix(supplied_suffix: Option<&str>) -> String { } } +/// # TODO +/// +/// This function currently deviates slightly from how the [manual][1] describes +/// that it should work. In particular, the current implementation: +/// +/// 1. Doesn't strictly respect the order in which to determine the backup type, +/// which is (in order of precedence) +/// 1. Take a valid value to the '--backup' option +/// 2. Take the value of the `VERSION_CONTROL` env var +/// 3. default to 'existing' +/// 2. Doesn't accept abbreviations to the 'backup_option' parameter +/// +/// [1]: https://www.gnu.org/software/coreutils/manual/html_node/Backup-options.html pub fn determine_backup_mode(backup_opt_exists: bool, backup_opt: Option<&str>) -> BackupMode { if backup_opt_exists { match backup_opt.map(String::from) { diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index ea2c2818e..06808db6b 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -696,3 +696,388 @@ fn test_install_dir() { assert!(at.file_exists(&format!("{}/{}", dir, file1))); assert!(at.file_exists(&format!("{}/{}", dir, file2))); } +// +// test backup functionality +#[test] +fn test_install_backup_short_no_args_files() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_simple_backup_file_a"; + let file_b = "test_install_simple_backup_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .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_install_backup_short_no_args_file_to_dir() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file = "test_install_simple_backup_file_a"; + let dest_dir = "test_install_dest/"; + let expect = format!("{}{}", dest_dir, file); + + at.touch(file); + at.mkdir(dest_dir); + at.touch(&expect); + scene + .ucmd() + .arg("-b") + .arg(file) + .arg(dest_dir) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(file)); + assert!(at.file_exists(&expect)); + assert!(at.file_exists(&format!("{}~", expect))); +} + +// Long --backup option is tested separately as it requires a slightly different +// handling than '-b' does. +#[test] +fn test_install_backup_long_no_args_files() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_simple_backup_file_a"; + let file_b = "test_install_simple_backup_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("--backup") + .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_install_backup_long_no_args_file_to_dir() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file = "test_install_simple_backup_file_a"; + let dest_dir = "test_install_dest/"; + let expect = format!("{}{}", dest_dir, file); + + at.touch(file); + at.mkdir(dest_dir); + at.touch(&expect); + scene + .ucmd() + .arg("--backup") + .arg(file) + .arg(dest_dir) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(file)); + assert!(at.file_exists(&expect)); + assert!(at.file_exists(&format!("{}~", expect))); +} + +#[test] +fn test_install_backup_short_custom_suffix() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_custom_suffix_file_a"; + let file_b = "test_install_backup_custom_suffix_file_b"; + let suffix = "super-suffix-of-the-century"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("-b") + .arg(format!("--suffix={}", suffix)) + .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, suffix))); +} + +#[test] +fn test_install_backup_custom_suffix_via_env() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_custom_suffix_file_a"; + let file_b = "test_install_backup_custom_suffix_file_b"; + let suffix = "super-suffix-of-the-century"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("-b") + .env("SIMPLE_BACKUP_SUFFIX", suffix) + .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, suffix))); +} + +#[test] +fn test_install_backup_numbered_with_t() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("--backup=t") + .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_install_backup_numbered_with_numbered() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .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_install_backup_existing() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("--backup=existing") + .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_install_backup_nil() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .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_install_backup_numbered_if_existing_backup_existing() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + let file_b_backup = "test_install_backup_numbering_file_b.~1~"; + + at.touch(file_a); + at.touch(file_b); + at.touch(file_b_backup); + scene + .ucmd() + .arg("--backup=existing") + .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(file_b_backup)); + assert!(at.file_exists(&*format!("{}.~2~", file_b))); +} + +#[test] +fn test_install_backup_numbered_if_existing_backup_nil() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + let file_b_backup = "test_install_backup_numbering_file_b.~1~"; + + at.touch(file_a); + at.touch(file_b); + at.touch(file_b_backup); + scene + .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(file_b_backup)); + assert!(at.file_exists(&*format!("{}.~2~", file_b))); +} + +#[test] +fn test_install_backup_simple() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("--backup=simple") + .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_install_backup_never() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .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_install_backup_none() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("--backup=none") + .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_install_backup_off() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_numbering_file_a"; + let file_b = "test_install_backup_numbering_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("--backup=off") + .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))); +}