From 7240b1289523a985f6bc9999b0fec4a0cd903557 Mon Sep 17 00:00:00 2001 From: Matt Blessed Date: Tue, 25 May 2021 16:38:34 -0400 Subject: [PATCH 1/3] uucore: implement backup control Most of these changes were sourced from mv's existing backup control implementation. A later commit will update the mv utility to use this new share backup control. --- src/uucore/src/lib/lib.rs | 1 + src/uucore/src/lib/mods.rs | 1 + src/uucore/src/lib/mods/backup_control.rs | 97 +++++++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 src/uucore/src/lib/mods/backup_control.rs diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index eb630f53a..c17f14516 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -25,6 +25,7 @@ mod features; // feature-gated code modules mod mods; // core cross-platform modules // * cross-platform modules +pub use crate::mods::backup_control; pub use crate::mods::coreopts; pub use crate::mods::os; pub use crate::mods::panic; diff --git a/src/uucore/src/lib/mods.rs b/src/uucore/src/lib/mods.rs index 74725e141..2689361a0 100644 --- a/src/uucore/src/lib/mods.rs +++ b/src/uucore/src/lib/mods.rs @@ -1,5 +1,6 @@ // mods ~ cross-platforms modules (core/bundler file) +pub mod backup_control; pub mod coreopts; pub mod os; pub mod panic; diff --git a/src/uucore/src/lib/mods/backup_control.rs b/src/uucore/src/lib/mods/backup_control.rs new file mode 100644 index 000000000..6004ae84d --- /dev/null +++ b/src/uucore/src/lib/mods/backup_control.rs @@ -0,0 +1,97 @@ +use std::{ + env, + path::{Path, PathBuf}, +}; + +pub static BACKUP_CONTROL_VALUES: &[&str] = &[ + "simple", "never", "numbered", "t", "existing", "nil", "none", "off", +]; + +pub static BACKUP_CONTROL_LONG_HELP: &str = "The backup suffix is '~', unless set with --suffix or SIMPLE_BACKUP_SUFFIX. Here are the version control values: + +none, off + never make backups (even if --backup is given) + +numbered, t + make numbered backups + +existing, nil + numbered if numbered backups exist, simple otherwise + +simple, never + always make simple backups"; + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum BackupMode { + NoBackup, + SimpleBackup, + NumberedBackup, + ExistingBackup, +} + +pub fn determine_backup_suffix(supplied_suffix: Option<&str>) -> String { + if let Some(suffix) = supplied_suffix { + String::from(suffix) + } else { + env::var("SIMPLE_BACKUP_SUFFIX").unwrap_or("~".to_owned()) + } +} + +pub fn determine_backup_mode(backup_opt_exists: bool, backup_opt: Option<&str>) -> BackupMode { + if backup_opt_exists { + match backup_opt.map(String::from) { + // default is existing, see: + // https://www.gnu.org/software/coreutils/manual/html_node/Backup-options.html + None => BackupMode::ExistingBackup, + 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 + } +} + +pub fn get_backup_path( + backup_mode: BackupMode, + backup_path: &Path, + suffix: &str, +) -> Option { + match backup_mode { + BackupMode::NoBackup => None, + BackupMode::SimpleBackup => Some(simple_backup_path(backup_path, suffix)), + BackupMode::NumberedBackup => Some(numbered_backup_path(backup_path)), + BackupMode::ExistingBackup => Some(existing_backup_path(backup_path, suffix)), + } +} + +pub 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) +} + +pub fn numbered_backup_path(path: &Path) -> PathBuf { + for i in 1_u64.. { + let path_str = &format!("{}.~{}~", path.to_string_lossy(), i); + let path = Path::new(path_str); + if !path.exists() { + return path.to_path_buf(); + } + } + panic!("cannot create backup") +} + +pub fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf { + let test_path_str = &format!("{}.~1~", path.to_string_lossy()); + let test_path = Path::new(test_path_str); + if test_path.exists() { + numbered_backup_path(path) + } else { + simple_backup_path(path, suffix) + } +} From a8a1ec7faf7ef1994366f538f040dc866a2c9686 Mon Sep 17 00:00:00 2001 From: Matt Blessed Date: Tue, 25 May 2021 16:41:07 -0400 Subject: [PATCH 2/3] cp: implement backup control with tests --- src/uu/cp/src/cp.rs | 64 +++++++----- tests/by-util/test_cp.rs | 210 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 247 insertions(+), 27 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 3d6faf66a..7eaa21c11 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -47,6 +47,7 @@ use std::os::windows::ffi::OsStrExt; use std::path::{Path, PathBuf, StripPrefixError}; use std::str::FromStr; use std::string::ToString; +use uucore::backup_control::{self, BackupMode}; use uucore::fs::resolve_relative_path; use uucore::fs::{canonicalize, CanonicalizeMode}; use walkdir::WalkDir; @@ -169,14 +170,6 @@ pub enum TargetType { File, } -#[derive(Clone, Eq, PartialEq)] -pub enum BackupMode { - ExistingBackup, - NoBackup, - NumberedBackup, - SimpleBackup, -} - pub enum CopyMode { Link, SymLink, @@ -201,7 +194,7 @@ pub enum Attribute { #[allow(dead_code)] pub struct Options { attributes_only: bool, - backup: bool, + backup: BackupMode, copy_contents: bool, copy_mode: CopyMode, dereference: bool, @@ -222,6 +215,7 @@ pub struct Options { static VERSION: &str = env!("CARGO_PKG_VERSION"); static ABOUT: &str = "Copy SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY."; +static LONG_HELP: &str = ""; static EXIT_OK: i32 = 0; static EXIT_ERR: i32 = 1; @@ -301,6 +295,7 @@ 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_TARGET_DIRECTORY) .short("t") @@ -364,12 +359,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .arg(Arg::with_name(OPT_BACKUP) .short("b") .long(OPT_BACKUP) - .help("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) + .possible_values(backup_control::BACKUP_CONTROL_VALUES) + .value_name("CONTROL") + ) .arg(Arg::with_name(OPT_SUFFIX) .short("S") .long(OPT_SUFFIX) .takes_value(true) - .default_value("~") .value_name("SUFFIX") .help("override the usual backup suffix")) .arg(Arg::with_name(OPT_UPDATE) @@ -585,7 +585,24 @@ impl Options { || matches.is_present(OPT_RECURSIVE_ALIAS) || matches.is_present(OPT_ARCHIVE); - let backup = matches.is_present(OPT_BACKUP) || (matches.occurrences_of(OPT_SUFFIX) > 0); + let backup_mode = backup_control::determine_backup_mode( + matches.is_present(OPT_BACKUP), + matches.value_of(OPT_BACKUP), + ); + let backup_suffix = backup_control::determine_backup_suffix(matches.value_of(OPT_SUFFIX)); + + let overwrite = OverwriteMode::from_matches(matches); + + if overwrite == OverwriteMode::NoClobber && backup_mode != BackupMode::NoBackup { + show_error!( + "options --backup and --no-clobber are mutually exclusive\n\ + Try '{} --help' for more information.", + executable!() + ); + return Err(Error::Error( + "options --backup and --no-clobber are mutually exclusive".to_owned(), + )); + } // Parse target directory options let no_target_dir = matches.is_present(OPT_NO_TARGET_DIRECTORY); @@ -631,9 +648,7 @@ impl Options { || matches.is_present(OPT_NO_DEREFERENCE_PRESERVE_LINKS) || matches.is_present(OPT_ARCHIVE), one_file_system: matches.is_present(OPT_ONE_FILE_SYSTEM), - overwrite: OverwriteMode::from_matches(matches), parents: matches.is_present(OPT_PARENTS), - backup_suffix: matches.value_of(OPT_SUFFIX).unwrap().to_string(), update: matches.is_present(OPT_UPDATE), verbose: matches.is_present(OPT_VERBOSE), strip_trailing_slashes: matches.is_present(OPT_STRIP_TRAILING_SLASHES), @@ -654,7 +669,9 @@ impl Options { ReflinkMode::Never } }, - backup, + backup: backup_mode, + backup_suffix: backup_suffix, + overwrite: overwrite, no_target_dir, preserve_attributes, recursive, @@ -1090,14 +1107,10 @@ fn context_for(src: &Path, dest: &Path) -> String { format!("'{}' -> '{}'", src.display(), dest.display()) } -/// Implements a relatively naive backup that is not as full featured -/// as GNU cp. No CONTROL version control method argument is taken -/// for backups. -/// TODO: Add version control methods -fn backup_file(path: &Path, suffix: &str) -> CopyResult { - let mut backup_path = path.to_path_buf().into_os_string(); - backup_path.push(suffix); - fs::copy(path, &backup_path)?; +/// Implements a simple backup copy for the destination file. +/// TODO: for the backup, should this function be replaced by `copy_file(...)`? +fn backup_dest(dest: &Path, backup_path: &PathBuf) -> CopyResult { + fs::copy(dest, &backup_path)?; Ok(backup_path.into()) } @@ -1108,8 +1121,9 @@ fn handle_existing_dest(source: &Path, dest: &Path, options: &Options) -> CopyRe options.overwrite.verify(dest)?; - if options.backup { - backup_file(dest, &options.backup_suffix)?; + let backup_path = backup_control::get_backup_path(options.backup, dest, &options.backup_suffix); + if let Some(backup_path) = backup_path { + backup_dest(dest, &backup_path)?; } match options.overwrite { diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 1e99da0fb..dddba595c 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -214,8 +214,8 @@ fn test_cp_arg_symlink() { fn test_cp_arg_no_clobber() { let (at, mut ucmd) = at_and_ucmd!(); ucmd.arg(TEST_HELLO_WORLD_SOURCE) - .arg("--no-clobber") .arg(TEST_HOW_ARE_YOU_SOURCE) + .arg("--no-clobber") .succeeds(); assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "How are you?\n"); @@ -305,7 +305,23 @@ fn test_cp_arg_backup() { let (at, mut ucmd) = at_and_ucmd!(); ucmd.arg(TEST_HELLO_WORLD_SOURCE) - .arg("--backup") + .arg(TEST_HOW_ARE_YOU_SOURCE) + .arg("-b") + .succeeds(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_arg_backup_arg_first() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup") + .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_HOW_ARE_YOU_SOURCE) .succeeds(); @@ -321,6 +337,7 @@ fn test_cp_arg_suffix() { let (at, mut ucmd) = at_and_ucmd!(); ucmd.arg(TEST_HELLO_WORLD_SOURCE) + .arg("-b") .arg("--suffix") .arg(".bak") .arg(TEST_HOW_ARE_YOU_SOURCE) @@ -333,6 +350,195 @@ fn test_cp_arg_suffix() { ); } +#[test] +fn test_cp_custom_backup_suffix_via_env() { + let (at, mut ucmd) = at_and_ucmd!(); + let suffix = "super-suffix-of-the-century"; + + ucmd.arg("-b") + .env("SIMPLE_BACKUP_SUFFIX", suffix) + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}{}", TEST_HOW_ARE_YOU_SOURCE, suffix)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_backup_numbered_with_t() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup=t") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}.~1~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_backup_numbered() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup=numbered") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}.~1~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_backup_existing() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup=existing") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_backup_nil() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup=nil") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_numbered_if_existing_backup_existing() { + let (at, mut ucmd) = at_and_ucmd!(); + let existing_backup = &*format!("{}.~1~", TEST_HOW_ARE_YOU_SOURCE); + at.touch(existing_backup); + + ucmd.arg("--backup=existing") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(TEST_HOW_ARE_YOU_SOURCE)); + assert!(at.file_exists(existing_backup)); + assert_eq!( + at.read(&*format!("{}.~2~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_numbered_if_existing_backup_nil() { + let (at, mut ucmd) = at_and_ucmd!(); + let existing_backup = &*format!("{}.~1~", TEST_HOW_ARE_YOU_SOURCE); + + at.touch(existing_backup); + ucmd.arg("--backup=nil") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(TEST_HOW_ARE_YOU_SOURCE)); + assert!(at.file_exists(existing_backup)); + assert_eq!( + at.read(&*format!("{}.~2~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_backup_simple() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup=simple") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_backup_never() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup=never") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}~", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + +#[test] +fn test_cp_backup_none() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup=none") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert!(!at.file_exists(&format!("{}~", TEST_HOW_ARE_YOU_SOURCE))); +} + +#[test] +fn test_cp_backup_off() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup=off") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds() + .no_stderr(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert!(!at.file_exists(&format!("{}~", TEST_HOW_ARE_YOU_SOURCE))); +} + #[test] fn test_cp_deref_conflicting_options() { new_ucmd!() From 25ed5eeb0e470b7d1695f9caebee8a192efecc79 Mon Sep 17 00:00:00 2001 From: Matt Blessed Date: Wed, 26 May 2021 10:50:41 -0400 Subject: [PATCH 3/3] cp: move option check to uumain and use `show_usage_error` - add test for conflicting options `--backup` and `--no-clobber` --- src/uu/cp/src/cp.rs | 17 ++++++----------- tests/by-util/test_cp.rs | 12 ++++++++++++ 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 7eaa21c11..7e64a288c 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -463,6 +463,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .get_matches_from(args); let options = crash_if_err!(EXIT_ERR, Options::from_matches(&matches)); + + if options.overwrite == OverwriteMode::NoClobber && options.backup != BackupMode::NoBackup { + show_usage_error!("options --backup and --no-clobber are mutually exclusive"); + return 1; + } + let paths: Vec = matches .values_of(OPT_PATHS) .map(|v| v.map(ToString::to_string).collect()) @@ -593,17 +599,6 @@ impl Options { let overwrite = OverwriteMode::from_matches(matches); - if overwrite == OverwriteMode::NoClobber && backup_mode != BackupMode::NoBackup { - show_error!( - "options --backup and --no-clobber are mutually exclusive\n\ - Try '{} --help' for more information.", - executable!() - ); - return Err(Error::Error( - "options --backup and --no-clobber are mutually exclusive".to_owned(), - )); - } - // Parse target directory options let no_target_dir = matches.is_present(OPT_NO_TARGET_DIRECTORY); let target_dir = matches diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index dddba595c..d41d3f6ed 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -539,6 +539,18 @@ fn test_cp_backup_off() { assert!(!at.file_exists(&format!("{}~", TEST_HOW_ARE_YOU_SOURCE))); } +#[test] +fn test_cp_backup_no_clobber_conflicting_options() { + let (_, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("--backup") + .arg("--no-clobber") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .fails() + .stderr_is("cp: options --backup and --no-clobber are mutually exclusive\nTry 'cp --help' for more information."); +} + #[test] fn test_cp_deref_conflicting_options() { new_ucmd!()