From ecde4b6aa3894f4ee7bdd408fe9ebc0f3f8ec29c Mon Sep 17 00:00:00 2001 From: John Shin Date: Thu, 27 Apr 2023 04:46:48 -0700 Subject: [PATCH 01/25] core: introduce update controller for mv and cp --- src/uucore/src/lib/lib.rs | 1 + src/uucore/src/lib/mods.rs | 1 + src/uucore/src/lib/mods/update_control.rs | 57 +++++++++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 src/uucore/src/lib/mods/update_control.rs diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 00162ddbba5..e76e540c8d8 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -26,6 +26,7 @@ pub use crate::mods::os; pub use crate::mods::panic; pub use crate::mods::quoting_style; pub use crate::mods::ranges; +pub use crate::mods::update_control; pub use crate::mods::version_cmp; // * string parsing modules diff --git a/src/uucore/src/lib/mods.rs b/src/uucore/src/lib/mods.rs index 4b6c53f9531..71d288c69a5 100644 --- a/src/uucore/src/lib/mods.rs +++ b/src/uucore/src/lib/mods.rs @@ -6,6 +6,7 @@ pub mod error; pub mod os; pub mod panic; pub mod ranges; +pub mod update_control; pub mod version_cmp; // dir and vdir also need access to the quoting_style module pub mod quoting_style; diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs new file mode 100644 index 00000000000..a743b93409d --- /dev/null +++ b/src/uucore/src/lib/mods/update_control.rs @@ -0,0 +1,57 @@ +use clap::ArgMatches; + +pub static UPDATE_CONTROL_VALUES: &[&str] = &["all", "none", "old", ""]; + +pub const UPDATE_CONTROL_LONG_HELP: &str = "VERY LONG HELP"; + +#[derive(Clone, Eq, PartialEq)] +pub enum UpdateMode { + ReplaceAll, + ReplaceNone, + ReplaceIfOlder, +} + +pub mod arguments { + use clap::ArgAction; + + pub static OPT_UPDATE: &str = "update"; + pub static OPT_UPDATE_NO_ARG: &str = "u"; + + pub fn update() -> clap::Arg { + clap::Arg::new(OPT_UPDATE) + .long("update") + .help("some help") + .value_parser(["", "none", "all", "older"]) + .num_args(0..=1) + .default_missing_value("all") + .require_equals(true) + .overrides_with("update") + .action(clap::ArgAction::Set) + } + + pub fn update_no_args() -> clap::Arg { + clap::Arg::new(OPT_UPDATE_NO_ARG) + .short('u') + .help("like ") + .action(ArgAction::SetTrue) + } +} + +pub fn determine_update_mode(matches: &ArgMatches) -> UpdateMode { + if matches.contains_id(arguments::OPT_UPDATE) { + if let Some(mode) = matches.get_one::(arguments::OPT_UPDATE) { + match mode.as_str() { + "all" | "" => UpdateMode::ReplaceAll, + "none" => UpdateMode::ReplaceNone, + "older" => UpdateMode::ReplaceIfOlder, + _ => unreachable!("other args restricted by clap"), + } + } else { + unreachable!("other args restricted by clap") + } + } else if matches.get_flag(arguments::OPT_UPDATE_NO_ARG) { + UpdateMode::ReplaceIfOlder + } else { + UpdateMode::ReplaceAll + } +} From 0e8dd894a3700835ca63f04c8636533f4343aa4e Mon Sep 17 00:00:00 2001 From: John Shin Date: Thu, 27 Apr 2023 04:48:41 -0700 Subject: [PATCH 02/25] mv: implement update=[switch] --- src/uu/mv/src/mv.rs | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index d0a1a766f94..2951550c63c 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -25,6 +25,7 @@ use std::path::{Path, PathBuf}; use uucore::backup_control::{self, BackupMode}; use uucore::display::Quotable; use uucore::error::{set_exit_code, FromIo, UError, UResult, USimpleError, UUsageError}; +use uucore::update_control::{self, UpdateMode}; use uucore::{format_usage, help_about, help_usage, prompt_yes, show}; use fs_extra::dir::{ @@ -38,7 +39,7 @@ pub struct Behavior { overwrite: OverwriteMode, backup: BackupMode, suffix: String, - update: bool, + update: UpdateMode, target_dir: Option, no_target_dir: bool, verbose: bool, @@ -62,14 +63,15 @@ static OPT_NO_CLOBBER: &str = "no-clobber"; static OPT_STRIP_TRAILING_SLASHES: &str = "strip-trailing-slashes"; static OPT_TARGET_DIRECTORY: &str = "target-directory"; static OPT_NO_TARGET_DIRECTORY: &str = "no-target-directory"; -static OPT_UPDATE: &str = "update"; static OPT_VERBOSE: &str = "verbose"; static OPT_PROGRESS: &str = "progress"; static ARG_FILES: &str = "files"; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let mut app = uu_app().after_help(backup_control::BACKUP_CONTROL_LONG_HELP); + let mut app = uu_app() + .after_help(backup_control::BACKUP_CONTROL_LONG_HELP) + .after_help(update_control::UPDATE_CONTROL_LONG_HELP); let matches = app.try_get_matches_from_mut(args)?; if !matches.contains_id(OPT_TARGET_DIRECTORY) @@ -96,6 +98,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let overwrite_mode = determine_overwrite_mode(&matches); let backup_mode = backup_control::determine_backup_mode(&matches)?; + let update_mode = update_control::determine_update_mode(&matches); if overwrite_mode == OverwriteMode::NoClobber && backup_mode != BackupMode::NoBackup { return Err(UUsageError::new( @@ -110,7 +113,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { overwrite: overwrite_mode, backup: backup_mode, suffix: backup_suffix, - update: matches.get_flag(OPT_UPDATE), + update: update_mode, target_dir: matches .get_one::(OPT_TARGET_DIRECTORY) .map(OsString::from), @@ -131,6 +134,8 @@ pub fn uu_app() -> Command { .infer_long_args(true) .arg(backup_control::arguments::backup()) .arg(backup_control::arguments::backup_no_args()) + .arg(update_control::arguments::update()) + .arg(update_control::arguments::update_no_args()) .arg( Arg::new(OPT_FORCE) .short('f') @@ -176,16 +181,6 @@ pub fn uu_app() -> Command { .help("treat DEST as a normal file") .action(ArgAction::SetTrue), ) - .arg( - Arg::new(OPT_UPDATE) - .short('u') - .long(OPT_UPDATE) - .help( - "move only when the SOURCE file is newer than the destination file \ - or when the destination file is missing", - ) - .action(ArgAction::SetTrue), - ) .arg( Arg::new(OPT_VERBOSE) .short('v') @@ -412,7 +407,9 @@ fn rename( let mut backup_path = None; if to.exists() { - if b.update && b.overwrite == OverwriteMode::Interactive { + if (b.update == UpdateMode::ReplaceIfOlder || b.update == UpdateMode::ReplaceNone) + && b.overwrite == OverwriteMode::Interactive + { // `mv -i --update old new` when `new` exists doesn't move anything // and exit with 0 return Ok(()); @@ -433,7 +430,9 @@ fn rename( rename_with_fallback(to, backup_path, multi_progress)?; } - if b.update && fs::metadata(from)?.modified()? <= fs::metadata(to)?.modified()? { + if (b.update == UpdateMode::ReplaceIfOlder) + && fs::metadata(from)?.modified()? <= fs::metadata(to)?.modified()? + { return Ok(()); } } From 9dc84e906114ce4368d66d621301b46f845977c1 Mon Sep 17 00:00:00 2001 From: John Shin Date: Thu, 27 Apr 2023 04:49:27 -0700 Subject: [PATCH 03/25] cp: implement update=[switch] --- src/uu/cp/src/cp.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 80540d222ec..f302d487378 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -40,6 +40,7 @@ use uucore::error::{set_exit_code, UClapError, UError, UResult, UUsageError}; use uucore::fs::{ canonicalize, paths_refer_to_same_file, FileInformation, MissingHandling, ResolveMode, }; +use uucore::update_control::{self, UpdateMode}; use uucore::{crash, format_usage, help_about, help_usage, prompt_yes, show_error, show_warning}; use crate::copydir::copy_directory; @@ -224,7 +225,7 @@ pub struct Options { recursive: bool, backup_suffix: String, target_dir: Option, - update: bool, + update: UpdateMode, verbose: bool, progress_bar: bool, } @@ -264,7 +265,6 @@ mod options { pub const STRIP_TRAILING_SLASHES: &str = "strip-trailing-slashes"; pub const SYMBOLIC_LINK: &str = "symbolic-link"; pub const TARGET_DIRECTORY: &str = "target-directory"; - pub const UPDATE: &str = "update"; pub const VERBOSE: &str = "verbose"; } @@ -296,6 +296,8 @@ pub fn uu_app() -> Command { .about(ABOUT) .override_usage(format_usage(USAGE)) .infer_long_args(true) + .arg(update_control::arguments::update()) + .arg(update_control::arguments::update_no_args()) .arg( Arg::new(options::TARGET_DIRECTORY) .short('t') @@ -393,16 +395,6 @@ pub fn uu_app() -> Command { .arg(backup_control::arguments::backup()) .arg(backup_control::arguments::backup_no_args()) .arg(backup_control::arguments::suffix()) - .arg( - Arg::new(options::UPDATE) - .short('u') - .long(options::UPDATE) - .help( - "copy only when the SOURCE file is newer than the destination file \ - or when the destination file is missing", - ) - .action(ArgAction::SetTrue), - ) .arg( Arg::new(options::REFLINK) .long(options::REFLINK) @@ -641,7 +633,11 @@ impl CopyMode { Self::Link } else if matches.get_flag(options::SYMBOLIC_LINK) { Self::SymLink - } else if matches.get_flag(options::UPDATE) { + } else if matches + .get_one::(update_control::arguments::OPT_UPDATE) + .is_some() + || matches.get_flag(update_control::arguments::OPT_UPDATE_NO_ARG) + { Self::Update } else if matches.get_flag(options::ATTRIBUTES_ONLY) { Self::AttrOnly @@ -749,6 +745,7 @@ impl Options { Err(e) => return Err(Error::Backup(format!("{e}"))), Ok(mode) => mode, }; + let update_mode = update_control::determine_update_mode(matches); let backup_suffix = backup_control::determine_backup_suffix(matches); @@ -826,7 +823,7 @@ impl Options { || matches.get_flag(options::DEREFERENCE), one_file_system: matches.get_flag(options::ONE_FILE_SYSTEM), parents: matches.get_flag(options::PARENTS), - update: matches.get_flag(options::UPDATE), + update: update_mode, verbose: matches.get_flag(options::VERBOSE), strip_trailing_slashes: matches.get_flag(options::STRIP_TRAILING_SLASHES), reflink_mode: { @@ -1473,7 +1470,9 @@ fn copy_file( symlinked_files: &mut HashSet, source_in_command_line: bool, ) -> CopyResult<()> { - if options.update && options.overwrite == OverwriteMode::Interactive(ClobberMode::Standard) { + if (options.update == UpdateMode::ReplaceIfOlder || options.update == UpdateMode::ReplaceNone) + && options.overwrite == OverwriteMode::Interactive(ClobberMode::Standard) + { // `cp -i --update old new` when `new` exists doesn't copy anything // and exit with 0 return Ok(()); From 2f8df653c51718f5a3e75c39eaec157c10825ad6 Mon Sep 17 00:00:00 2001 From: John Shin Date: Thu, 27 Apr 2023 05:14:46 -0700 Subject: [PATCH 04/25] core mv cp: update help doc for 'update' functionality --- src/uu/cp/src/cp.rs | 64 +++++++++++++++++------ src/uu/mv/src/mv.rs | 14 ++++- src/uucore/src/lib/mods/update_control.rs | 8 ++- 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index f302d487378..b0c77967901 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -279,6 +279,21 @@ static PRESERVABLE_ATTRIBUTES: &[&str] = &[ "all", ]; +static CP_UPDATE_LONG_HELP: &str = +"Do not copy a non-directory that has an existing destination with the same or newer modification timestamp; +instead, silently skip the file without failing. If timestamps are being preserved, the comparison is to the +source timestamp truncated to the resolutions of the destination file system and of the system calls used to +update timestamps; this avoids duplicate work if several ‘cp -pu’ commands are executed with the same source +and destination. This option is ignored if the -n or --no-clobber option is also specified. Also, if +--preserve=links is also specified (like with ‘cp -au’ for example), that will take precedence; consequently, +depending on the order that files are processed from the source, newer files in the destination may be +replaced, to mirror hard links in the source. which gives more control over which existing files in the +destination are replaced, and its value can be one of the following: + +all This is the default operation when an --update option is not specified, and results in all existing files in the destination being replaced. +none This is similar to the --no-clobber option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. +older This is the default operation when --update is specified, and results in files being replaced if they’re older than the corresponding source file."; + #[cfg(not(unix))] static PRESERVABLE_ATTRIBUTES: &[&str] = &["mode", "timestamps", "context", "links", "xattr", "all"]; @@ -558,6 +573,7 @@ pub fn uu_app() -> Command { pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app() .after_help(backup_control::BACKUP_CONTROL_LONG_HELP) + .after_help(CP_UPDATE_LONG_HELP) .try_get_matches_from(args); // The error is parsed here because we do not want version or help being printed to stderr. @@ -1628,22 +1644,38 @@ fn copy_file( } CopyMode::Update => { if dest.exists() { - let dest_metadata = fs::symlink_metadata(dest)?; - - let src_time = source_metadata.modified()?; - let dest_time = dest_metadata.modified()?; - if src_time <= dest_time { - return Ok(()); - } else { - copy_helper( - source, - dest, - options, - context, - source_is_symlink, - source_is_fifo, - symlinked_files, - )?; + match options.update { + update_control::UpdateMode::ReplaceAll => { + copy_helper( + source, + dest, + options, + context, + source_is_symlink, + source_is_fifo, + symlinked_files, + )?; + } + update_control::UpdateMode::ReplaceNone => return Ok(()), + update_control::UpdateMode::ReplaceIfOlder => { + let dest_metadata = fs::symlink_metadata(dest)?; + + let src_time = source_metadata.modified()?; + let dest_time = dest_metadata.modified()?; + if src_time <= dest_time { + return Ok(()); + } else { + copy_helper( + source, + dest, + options, + context, + source_is_symlink, + source_is_fifo, + symlinked_files, + )?; + } + } } } else { copy_helper( diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 2951550c63c..2524466005d 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -67,11 +67,23 @@ static OPT_VERBOSE: &str = "verbose"; static OPT_PROGRESS: &str = "progress"; static ARG_FILES: &str = "files"; +static MV_UPDATE_LONG_HELP: &str = + "Do not move a non-directory that has an existing destination with the same or newer modification timestamp; +instead, silently skip the file without failing. If the move is across file system boundaries, the comparison is +to the source timestamp truncated to the resolutions of the destination file system and of the system calls used +to update timestamps; this avoids duplicate work if several ‘mv -u’ commands are executed with the same source +and destination. This option is ignored if the -n or --no-clobber option is also specified. which gives more control +over which existing files in the destination are replaced, and its value can be one of the following: + +all This is the default operation when an --update option is not specified, and results in all existing files in the destination being replaced. +none This is similar to the --no-clobber option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. +older This is the default operation when --update is specified, and results in files being replaced if they’re older than the corresponding source file."; + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let mut app = uu_app() .after_help(backup_control::BACKUP_CONTROL_LONG_HELP) - .after_help(update_control::UPDATE_CONTROL_LONG_HELP); + .after_help(MV_UPDATE_LONG_HELP); let matches = app.try_get_matches_from_mut(args)?; if !matches.contains_id(OPT_TARGET_DIRECTORY) diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs index a743b93409d..ab97b0412b0 100644 --- a/src/uucore/src/lib/mods/update_control.rs +++ b/src/uucore/src/lib/mods/update_control.rs @@ -2,8 +2,6 @@ use clap::ArgMatches; pub static UPDATE_CONTROL_VALUES: &[&str] = &["all", "none", "old", ""]; -pub const UPDATE_CONTROL_LONG_HELP: &str = "VERY LONG HELP"; - #[derive(Clone, Eq, PartialEq)] pub enum UpdateMode { ReplaceAll, @@ -20,10 +18,10 @@ pub mod arguments { pub fn update() -> clap::Arg { clap::Arg::new(OPT_UPDATE) .long("update") - .help("some help") + .help("move only when the SOURCE file is newer than the destination file or when the destination file is missing") .value_parser(["", "none", "all", "older"]) .num_args(0..=1) - .default_missing_value("all") + .default_missing_value("older") .require_equals(true) .overrides_with("update") .action(clap::ArgAction::Set) @@ -32,7 +30,7 @@ pub mod arguments { pub fn update_no_args() -> clap::Arg { clap::Arg::new(OPT_UPDATE_NO_ARG) .short('u') - .help("like ") + .help("like --update but does not accept an argument") .action(ArgAction::SetTrue) } } From ed3ff105400df0221a3b904143df1e17f8185405 Mon Sep 17 00:00:00 2001 From: John Shin Date: Sun, 30 Apr 2023 18:52:13 -0700 Subject: [PATCH 05/25] cp: write tests for --update --- src/uu/mv/src/mv.rs | 16 +++-- tests/by-util/test_cp.rs | 127 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 6 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 2524466005d..2edf9162496 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -427,6 +427,16 @@ fn rename( return Ok(()); } + if b.update == UpdateMode::ReplaceNone { + return Ok(()); + } + + if (b.update == UpdateMode::ReplaceIfOlder) + && fs::metadata(from)?.modified()? <= fs::metadata(to)?.modified()? + { + return Ok(()); + } + match b.overwrite { OverwriteMode::NoClobber => return Ok(()), OverwriteMode::Interactive => { @@ -441,12 +451,6 @@ fn rename( if let Some(ref backup_path) = backup_path { rename_with_fallback(to, backup_path, multi_progress)?; } - - if (b.update == UpdateMode::ReplaceIfOlder) - && fs::metadata(from)?.modified()? <= fs::metadata(to)?.modified()? - { - return Ok(()); - } } // "to" may no longer exist if it was backed up diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index dfbbc1473a5..4e241e0644f 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -244,6 +244,133 @@ fn test_cp_arg_update_interactive_error() { .no_stdout(); } +#[test] +fn test_cp_arg_update_none() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .arg("--update=none") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "How are you?\n") +} + +#[test] +fn test_cp_arg_update_all() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HOW_ARE_YOU_SOURCE) + .arg("--update=all") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!( + at.read(TEST_HOW_ARE_YOU_SOURCE), + at.read(TEST_HELLO_WORLD_SOURCE) + ) +} + +#[test] +fn test_cp_arg_update_older_dest_not_older_than_src() { + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_cp_arg_update_oldler_file1"; + let new = "test_cp_arg_update_oldler_file2"; + + at.touch(old); + sleep(Duration::from_secs(1)); + at.touch(new); + + at.append(old, "old content\n"); + at.append(new, "new content\n"); + + ucmd.arg(old) + .arg(new) + .arg("--update=older") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), "new content\n") +} + +#[test] +fn test_cp_arg_update_older_dest_older_than_src() { + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_cp_arg_update_oldler_file1"; + let new = "test_cp_arg_update_oldler_file2"; + + at.touch(old); + at.append(old, "old content\n"); + sleep(Duration::from_secs(1)); + at.touch(new); + at.append(new, "new content\n"); + + ucmd.arg(new) + .arg(old) + .arg("--update=older") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(old), "new content\n") +} + +#[test] +fn test_cp_arg_update_short_fail() { + // same as --update=older + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_cp_arg_update_oldler_file1"; + let new = "test_cp_arg_update_oldler_file2"; + + at.touch(old); + sleep(Duration::from_secs(1)); + at.touch(new); + + at.append(old, "old content\n"); + at.append(new, "new content\n"); + + ucmd.arg(old) + .arg(new) + .arg("-u") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), "new content\n") +} + +#[test] +fn test_cp_arg_update_short_succeed() { + // same as --update=older + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_cp_arg_update_oldler_file1"; + let new = "test_cp_arg_update_oldler_file2"; + + at.touch(old); + at.touch(new); + + at.append(old, "old content\n"); + at.append(new, "new content\n"); + + ucmd.arg(new) + .arg(old) + .arg("-u") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), "new content\n") +} + #[test] fn test_cp_arg_interactive() { let (at, mut ucmd) = at_and_ucmd!(); From 78412c5a6111677c590f4736955e5632b0e3ac82 Mon Sep 17 00:00:00 2001 From: John Shin Date: Sun, 30 Apr 2023 19:29:19 -0700 Subject: [PATCH 06/25] mv: add tests for --update --- tests/by-util/test_mv.rs | 164 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 03423410716..2c5f8591060 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1,5 +1,7 @@ use crate::common::util::TestScenario; use filetime::FileTime; +use std::thread::sleep; +use std::time::Duration; #[test] fn test_invalid_arg() { @@ -658,6 +660,168 @@ fn test_mv_update_option() { assert!(!at.file_exists(file_b)); } +#[test] +fn test_mv_arg_update_none() { + let (at, mut ucmd) = at_and_ucmd!(); + + let file1 = "test_mv_arg_update_none_file1"; + let file2 = "test_mv_arg_update_none_file2"; + + at.touch(file1); + at.touch(file2); + + let file1_content = "file1 content\n"; + let file2_content = "file2 content\n"; + + at.append(file1, file1_content); + at.append(file2, file2_content); + + ucmd.arg(file1) + .arg(file2) + .arg("--update=none") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(file2), file2_content) +} + +#[test] +fn test_mv_arg_update_all() { + let (at, mut ucmd) = at_and_ucmd!(); + + let file1 = "test_mv_arg_update_none_file1"; + let file2 = "test_mv_arg_update_none_file2"; + + at.touch(file1); + at.touch(file2); + + let file1_content = "file1 content\n"; + let file2_content = "file2 content\n"; + + at.append(file1, file1_content); + at.append(file2, file2_content); + + ucmd.arg(file1) + .arg(file2) + .arg("--update=all") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(file2), file1_content) +} + +#[test] +fn test_mv_arg_update_older_dest_not_older() { + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_mv_arg_update_none_file1"; + let new = "test_mv_arg_update_none_file2"; + let old_content = "file1 content\n"; + let new_content = "file2 content\n"; + + at.touch(old); + at.append(old, old_content); + + sleep(Duration::from_secs(1)); + + at.touch(new); + at.append(new, new_content); + + ucmd.arg(old) + .arg(new) + .arg("--update=older") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), new_content) +} + +#[test] +fn test_mv_arg_update_older_dest_older() { + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_mv_arg_update_none_file1"; + let new = "test_mv_arg_update_none_file2"; + let old_content = "file1 content\n"; + let new_content = "file2 content\n"; + + at.touch(old); + at.append(old, old_content); + + sleep(Duration::from_secs(1)); + + at.touch(new); + at.append(new, new_content); + + ucmd.arg(new) + .arg(old) + .arg("--update=all") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(old), new_content) +} + +#[test] +fn test_mv_arg_update_short_overwrite() { + // same as --update=older + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_mv_arg_update_none_file1"; + let new = "test_mv_arg_update_none_file2"; + let old_content = "file1 content\n"; + let new_content = "file2 content\n"; + + at.touch(old); + at.append(old, old_content); + + sleep(Duration::from_secs(1)); + + at.touch(new); + at.append(new, new_content); + + ucmd.arg(new) + .arg(old) + .arg("-u") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(old), new_content) +} + +#[test] +fn test_mv_arg_update_short_no_overwrite() { + // same as --update=older + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_mv_arg_update_none_file1"; + let new = "test_mv_arg_update_none_file2"; + let old_content = "file1 content\n"; + let new_content = "file2 content\n"; + + at.touch(old); + at.append(old, old_content); + + sleep(Duration::from_secs(1)); + + at.touch(new); + at.append(new, new_content); + + ucmd.arg(old) + .arg(new) + .arg("-u") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), new_content) +} + #[test] fn test_mv_target_dir() { let (at, mut ucmd) = at_and_ucmd!(); From 2f975e913e40616928b8bbcf516773cf9a824d36 Mon Sep 17 00:00:00 2001 From: John Shin Date: Sun, 30 Apr 2023 20:02:53 -0700 Subject: [PATCH 07/25] cp: move after help to md file --- src/uu/cp/cp.md | 16 ++++++++++++++++ src/uu/cp/src/cp.rs | 10 +++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/uu/cp/cp.md b/src/uu/cp/cp.md index 5f3cabc18b5..aadf4006b9e 100644 --- a/src/uu/cp/cp.md +++ b/src/uu/cp/cp.md @@ -7,3 +7,19 @@ cp [OPTION]... -t DIRECTORY SOURCE... ``` Copy SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY. + +## After Help + +Do not copy a non-directory that has an existing destination with the same or newer modification timestamp; +instead, silently skip the file without failing. If timestamps are being preserved, the comparison is to the +source timestamp truncated to the resolutions of the destination file system and of the system calls used to +update timestamps; this avoids duplicate work if several ‘cp -pu’ commands are executed with the same source +and destination. This option is ignored if the -n or --no-clobber option is also specified. Also, if +--preserve=links is also specified (like with ‘cp -au’ for example), that will take precedence; consequently, +depending on the order that files are processed from the source, newer files in the destination may be replaced, +to mirror hard links in the source. which gives more control over which existing files in the destination are +replaced, and its value can be one of the following: + +all This is the default operation when an --update option is not specified, and results in all existing files in the destination being replaced. +none This is similar to the --no-clobber option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. +older This is the default operation when --update is specified, and results in files being replaced if they’re older than the corresponding source file. diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index b0c77967901..5e791cc3bf4 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -41,7 +41,9 @@ use uucore::fs::{ canonicalize, paths_refer_to_same_file, FileInformation, MissingHandling, ResolveMode, }; use uucore::update_control::{self, UpdateMode}; -use uucore::{crash, format_usage, help_about, help_usage, prompt_yes, show_error, show_warning}; +use uucore::{ + crash, format_usage, help_about, help_section, help_usage, prompt_yes, show_error, show_warning, +}; use crate::copydir::copy_directory; @@ -232,6 +234,7 @@ pub struct Options { const ABOUT: &str = help_about!("cp.md"); const USAGE: &str = help_usage!("cp.md"); +const AFTER_HELP: &str = help_section!("after help", "cp.md"); static EXIT_ERR: i32 = 1; @@ -310,9 +313,8 @@ pub fn uu_app() -> Command { .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) + .after_help(AFTER_HELP) .infer_long_args(true) - .arg(update_control::arguments::update()) - .arg(update_control::arguments::update_no_args()) .arg( Arg::new(options::TARGET_DIRECTORY) .short('t') @@ -410,6 +412,8 @@ pub fn uu_app() -> Command { .arg(backup_control::arguments::backup()) .arg(backup_control::arguments::backup_no_args()) .arg(backup_control::arguments::suffix()) + .arg(update_control::arguments::update()) + .arg(update_control::arguments::update_no_args()) .arg( Arg::new(options::REFLINK) .long(options::REFLINK) From f83468d530d4018291e02c79aea928eea80cda3a Mon Sep 17 00:00:00 2001 From: John Shin Date: Sun, 30 Apr 2023 20:03:22 -0700 Subject: [PATCH 08/25] mv: move after help to md file --- src/uu/mv/mv.md | 14 +++++++++++++- src/uu/mv/src/mv.rs | 28 ++++++++-------------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/uu/mv/mv.md b/src/uu/mv/mv.md index 772e4bfaf4a..7a5821351fb 100644 --- a/src/uu/mv/mv.md +++ b/src/uu/mv/mv.md @@ -5,5 +5,17 @@ mv [OPTION]... [-T] SOURCE DEST mv [OPTION]... SOURCE... DIRECTORY mv [OPTION]... -t DIRECTORY SOURCE... ``` - Move `SOURCE` to `DEST`, or multiple `SOURCE`(s) to `DIRECTORY`. + +## After Help + +Do not move a non-directory that has an existing destination with the same or newer modification timestamp; +instead, silently skip the file without failing. If the move is across file system boundaries, the comparison is +to the source timestamp truncated to the resolutions of the destination file system and of the system calls used +to update timestamps; this avoids duplicate work if several ‘mv -u’ commands are executed with the same source +and destination. This option is ignored if the -n or --no-clobber option is also specified. which gives more control +over which existing files in the destination are replaced, and its value can be one of the following: + +all This is the default operation when an --update option is not specified, and results in all existing files in the destination being replaced. +none This is similar to the --no-clobber option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. +older This is the default operation when --update is specified, and results in files being replaced if they’re older than the corresponding source file. diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 2edf9162496..db2b4153bd8 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -26,7 +26,7 @@ use uucore::backup_control::{self, BackupMode}; use uucore::display::Quotable; use uucore::error::{set_exit_code, FromIo, UError, UResult, USimpleError, UUsageError}; use uucore::update_control::{self, UpdateMode}; -use uucore::{format_usage, help_about, help_usage, prompt_yes, show}; +use uucore::{format_usage, help_about, help_section, help_usage, prompt_yes, show}; use fs_extra::dir::{ get_size as dir_get_size, move_dir, move_dir_with_progress, CopyOptions as DirCopyOptions, @@ -56,6 +56,7 @@ pub enum OverwriteMode { const ABOUT: &str = help_about!("mv.md"); const USAGE: &str = help_usage!("mv.md"); +const AFTER_HELP: &str = help_section!("after help", "mv.md"); static OPT_FORCE: &str = "force"; static OPT_INTERACTIVE: &str = "interactive"; @@ -67,23 +68,9 @@ static OPT_VERBOSE: &str = "verbose"; static OPT_PROGRESS: &str = "progress"; static ARG_FILES: &str = "files"; -static MV_UPDATE_LONG_HELP: &str = - "Do not move a non-directory that has an existing destination with the same or newer modification timestamp; -instead, silently skip the file without failing. If the move is across file system boundaries, the comparison is -to the source timestamp truncated to the resolutions of the destination file system and of the system calls used -to update timestamps; this avoids duplicate work if several ‘mv -u’ commands are executed with the same source -and destination. This option is ignored if the -n or --no-clobber option is also specified. which gives more control -over which existing files in the destination are replaced, and its value can be one of the following: - -all This is the default operation when an --update option is not specified, and results in all existing files in the destination being replaced. -none This is similar to the --no-clobber option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. -older This is the default operation when --update is specified, and results in files being replaced if they’re older than the corresponding source file."; - #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let mut app = uu_app() - .after_help(backup_control::BACKUP_CONTROL_LONG_HELP) - .after_help(MV_UPDATE_LONG_HELP); + let mut app = uu_app().after_help(backup_control::BACKUP_CONTROL_LONG_HELP); let matches = app.try_get_matches_from_mut(args)?; if !matches.contains_id(OPT_TARGET_DIRECTORY) @@ -143,11 +130,8 @@ pub fn uu_app() -> Command { .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) + .after_help(AFTER_HELP) .infer_long_args(true) - .arg(backup_control::arguments::backup()) - .arg(backup_control::arguments::backup_no_args()) - .arg(update_control::arguments::update()) - .arg(update_control::arguments::update_no_args()) .arg( Arg::new(OPT_FORCE) .short('f') @@ -175,7 +159,11 @@ pub fn uu_app() -> Command { .help("remove any trailing slashes from each SOURCE argument") .action(ArgAction::SetTrue), ) + .arg(backup_control::arguments::backup()) + .arg(backup_control::arguments::backup_no_args()) .arg(backup_control::arguments::suffix()) + .arg(update_control::arguments::update()) + .arg(update_control::arguments::update_no_args()) .arg( Arg::new(OPT_TARGET_DIRECTORY) .short('t') From b707b690c5b45ce090c3a87372366763272b13e9 Mon Sep 17 00:00:00 2001 From: John Shin Date: Sun, 30 Apr 2023 20:16:01 -0700 Subject: [PATCH 09/25] cp: remove long help --- src/uu/cp/src/cp.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 5e791cc3bf4..64278bf1313 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -282,21 +282,6 @@ static PRESERVABLE_ATTRIBUTES: &[&str] = &[ "all", ]; -static CP_UPDATE_LONG_HELP: &str = -"Do not copy a non-directory that has an existing destination with the same or newer modification timestamp; -instead, silently skip the file without failing. If timestamps are being preserved, the comparison is to the -source timestamp truncated to the resolutions of the destination file system and of the system calls used to -update timestamps; this avoids duplicate work if several ‘cp -pu’ commands are executed with the same source -and destination. This option is ignored if the -n or --no-clobber option is also specified. Also, if ---preserve=links is also specified (like with ‘cp -au’ for example), that will take precedence; consequently, -depending on the order that files are processed from the source, newer files in the destination may be -replaced, to mirror hard links in the source. which gives more control over which existing files in the -destination are replaced, and its value can be one of the following: - -all This is the default operation when an --update option is not specified, and results in all existing files in the destination being replaced. -none This is similar to the --no-clobber option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. -older This is the default operation when --update is specified, and results in files being replaced if they’re older than the corresponding source file."; - #[cfg(not(unix))] static PRESERVABLE_ATTRIBUTES: &[&str] = &["mode", "timestamps", "context", "links", "xattr", "all"]; @@ -577,7 +562,6 @@ pub fn uu_app() -> Command { pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app() .after_help(backup_control::BACKUP_CONTROL_LONG_HELP) - .after_help(CP_UPDATE_LONG_HELP) .try_get_matches_from(args); // The error is parsed here because we do not want version or help being printed to stderr. From c5327cf0a0158d357ba116399291c7e4b715d702 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 1 May 2023 03:46:14 -0700 Subject: [PATCH 10/25] core: add docs for update control --- src/uucore/src/lib/mods/update_control.rs | 87 ++++++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs index ab97b0412b0..5bbca0e49bb 100644 --- a/src/uucore/src/lib/mods/update_control.rs +++ b/src/uucore/src/lib/mods/update_control.rs @@ -1,11 +1,57 @@ +//! Implement GNU-style update functionality. +//! +//! - pre-defined [`clap`-Arguments][1] for inclusion in utilities that +//! implement updates +//! - determination of the [update mode][2] +//! +//! Update-functionality is implemented by the following utilities: +//! +//! - `cp` +//! - `mv` +//! +//! +//! [1]: arguments +//! [2]: `determine_update_mode()` +//! +//! +//! # Usage example +//! +//! ``` +//! #[macro_use] +//! extern crate uucore; +//! +//! use clap::{Command, Arg, ArgMatches}; +//! use uucore::update_control::{self, UpdateMode}; +//! +//! fn main() { +//! let matches = Command::new("command") +//! .arg(update_control::arguments::update()) +//! .arg(update_control::arguments::update_no_args()) +//! .get_matches_from(vec![ +//! "commmand", "--update=older" +//! ]); +//! +//! let update_mode = update_control::determine_update_mode(&matches); +//! +//! // handle cases +//! if update_mode == UpdateMode::ReplaceIfOlder { +//! // do +//! } else { +//! unreachable!() +//! } +//! } +//! ``` use clap::ArgMatches; -pub static UPDATE_CONTROL_VALUES: &[&str] = &["all", "none", "old", ""]; - -#[derive(Clone, Eq, PartialEq)] +// Available update mode +#[derive(Clone, Debug, Eq, PartialEq)] pub enum UpdateMode { + // --update=`all`, `` ReplaceAll, + // --update=`none` ReplaceNone, + // --update=`older` + // -u ReplaceIfOlder, } @@ -15,6 +61,7 @@ pub mod arguments { pub static OPT_UPDATE: &str = "update"; pub static OPT_UPDATE_NO_ARG: &str = "u"; + // `--update` argument, defaults to `older` if no values after provided pub fn update() -> clap::Arg { clap::Arg::new(OPT_UPDATE) .long("update") @@ -27,6 +74,7 @@ pub mod arguments { .action(clap::ArgAction::Set) } + // `-u` argument pub fn update_no_args() -> clap::Arg { clap::Arg::new(OPT_UPDATE_NO_ARG) .short('u') @@ -35,6 +83,37 @@ pub mod arguments { } } +/// Determine the "mode" for the update operation to perform, if any. +/// +/// Parses the backup options and converts them to an instance of +/// `UpdateMode` for further processing. +/// +/// Takes [`clap::ArgMatches`] as argument which **must** contain the options +/// from [`arguments::update()`] or [`arguments::update_no_args()`]. Otherwise +/// the `ReplaceAll` mode is returned unconditionally. +/// +/// # Examples +/// +/// Here's how one would integrate the update mode determination into an +/// application. +/// +/// ``` +/// #[macro_use] +/// extern crate uucore; +/// use uucore::update_control::{self, UpdateMode}; +/// use clap::{Command, Arg, ArgMatches}; +/// +/// fn main() { +/// let matches = Command::new("command") +/// .arg(update_control::arguments::update()) +/// .arg(update_control::arguments::update_no_args()) +/// .get_matches_from(vec![ +/// "command", "--update=all" +/// ]); +/// +/// let update_mode = update_control::determine_update_mode(&matches); +/// assert_eq!(update_mode, UpdateMode::ReplaceAll) +/// } pub fn determine_update_mode(matches: &ArgMatches) -> UpdateMode { if matches.contains_id(arguments::OPT_UPDATE) { if let Some(mode) = matches.get_one::(arguments::OPT_UPDATE) { @@ -48,8 +127,10 @@ pub fn determine_update_mode(matches: &ArgMatches) -> UpdateMode { unreachable!("other args restricted by clap") } } else if matches.get_flag(arguments::OPT_UPDATE_NO_ARG) { + // short form of this option is equivalent to using --update=older UpdateMode::ReplaceIfOlder } else { + // no option was present UpdateMode::ReplaceAll } } From 92e1b3f7c076c08b74b510e771acf983243315a1 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 1 May 2023 03:53:52 -0700 Subject: [PATCH 11/25] cp: fix documentation --- src/uu/cp/cp.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/cp.md b/src/uu/cp/cp.md index aadf4006b9e..7485340f2ac 100644 --- a/src/uu/cp/cp.md +++ b/src/uu/cp/cp.md @@ -13,13 +13,13 @@ Copy SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY. Do not copy a non-directory that has an existing destination with the same or newer modification timestamp; instead, silently skip the file without failing. If timestamps are being preserved, the comparison is to the source timestamp truncated to the resolutions of the destination file system and of the system calls used to -update timestamps; this avoids duplicate work if several ‘cp -pu’ commands are executed with the same source -and destination. This option is ignored if the -n or --no-clobber option is also specified. Also, if ---preserve=links is also specified (like with ‘cp -au’ for example), that will take precedence; consequently, +update timestamps; this avoids duplicate work if several `cp -pu` commands are executed with the same source +and destination. This option is ignored if the `-n` or `--no-clobber` option is also specified. Also, if +`--preserve=links` is also specified (like with `cp -au` for example), that will take precedence; consequently, depending on the order that files are processed from the source, newer files in the destination may be replaced, to mirror hard links in the source. which gives more control over which existing files in the destination are replaced, and its value can be one of the following: -all This is the default operation when an --update option is not specified, and results in all existing files in the destination being replaced. -none This is similar to the --no-clobber option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. -older This is the default operation when --update is specified, and results in files being replaced if they’re older than the corresponding source file. +* `all` This is the default operation when an `--update` option is not specified, and results in all existing files in the destination being replaced. +* `none` This is similar to the `--no-clobber` option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. +* `older` This is the default operation when `--update` is specified, and results in files being replaced if they’re older than the corresponding source file. From 6625cfe88a569def06cfde66894904997eb7ab03 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 1 May 2023 03:54:04 -0700 Subject: [PATCH 12/25] mv: fix documentation --- src/uu/mv/mv.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/uu/mv/mv.md b/src/uu/mv/mv.md index 7a5821351fb..c31c6d07c40 100644 --- a/src/uu/mv/mv.md +++ b/src/uu/mv/mv.md @@ -12,10 +12,10 @@ Move `SOURCE` to `DEST`, or multiple `SOURCE`(s) to `DIRECTORY`. Do not move a non-directory that has an existing destination with the same or newer modification timestamp; instead, silently skip the file without failing. If the move is across file system boundaries, the comparison is to the source timestamp truncated to the resolutions of the destination file system and of the system calls used -to update timestamps; this avoids duplicate work if several ‘mv -u’ commands are executed with the same source -and destination. This option is ignored if the -n or --no-clobber option is also specified. which gives more control +to update timestamps; this avoids duplicate work if several `mv -u` commands are executed with the same source +and destination. This option is ignored if the `-n` or `--no-clobber` option is also specified. which gives more control over which existing files in the destination are replaced, and its value can be one of the following: -all This is the default operation when an --update option is not specified, and results in all existing files in the destination being replaced. -none This is similar to the --no-clobber option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. -older This is the default operation when --update is specified, and results in files being replaced if they’re older than the corresponding source file. +* `all` This is the default operation when an `--update` option is not specified, and results in all existing files in the destination being replaced. +* `none` This is similar to the `--no-clobber` option, in that no files in the destination are replaced, but also skipping a file does not induce a failure. +* `older` This is the default operation when `--update` is specified, and results in files being replaced if they’re older than the corresponding source file. From 66a9169e55b665e0e8a085a9c53b5a3570f7aa31 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 1 May 2023 03:54:56 -0700 Subject: [PATCH 13/25] cp: fix typos --- tests/by-util/test_cp.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 4e241e0644f..a933343b195 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -279,8 +279,8 @@ fn test_cp_arg_update_all() { fn test_cp_arg_update_older_dest_not_older_than_src() { let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_oldler_file1"; - let new = "test_cp_arg_update_oldler_file2"; + let old = "test_cp_arg_update_older_file1"; + let new = "test_cp_arg_update_older_file2"; at.touch(old); sleep(Duration::from_secs(1)); @@ -303,8 +303,8 @@ fn test_cp_arg_update_older_dest_not_older_than_src() { fn test_cp_arg_update_older_dest_older_than_src() { let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_oldler_file1"; - let new = "test_cp_arg_update_oldler_file2"; + let old = "test_cp_arg_update_older_file1"; + let new = "test_cp_arg_update_older_file2"; at.touch(old); at.append(old, "old content\n"); @@ -327,8 +327,8 @@ fn test_cp_arg_update_short_fail() { // same as --update=older let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_oldler_file1"; - let new = "test_cp_arg_update_oldler_file2"; + let old = "test_cp_arg_update_older_file1"; + let new = "test_cp_arg_update_older_file2"; at.touch(old); sleep(Duration::from_secs(1)); @@ -352,8 +352,8 @@ fn test_cp_arg_update_short_succeed() { // same as --update=older let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_oldler_file1"; - let new = "test_cp_arg_update_oldler_file2"; + let old = "test_cp_arg_update_older_file1"; + let new = "test_cp_arg_update_older_file2"; at.touch(old); at.touch(new); From 60c0b661c3476a0bdc9ffd9d52078aae9b6f910e Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 1 May 2023 17:22:08 -0700 Subject: [PATCH 14/25] core: fix typo in update control --- src/uucore/src/lib/mods/update_control.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs index 5bbca0e49bb..c0b2cc5ef99 100644 --- a/src/uucore/src/lib/mods/update_control.rs +++ b/src/uucore/src/lib/mods/update_control.rs @@ -28,7 +28,7 @@ //! .arg(update_control::arguments::update()) //! .arg(update_control::arguments::update_no_args()) //! .get_matches_from(vec![ -//! "commmand", "--update=older" +//! "command", "--update=older" //! ]); //! //! let update_mode = update_control::determine_update_mode(&matches); From 36e93e12d662b2e629862d0ca434e5e2319dccb6 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 12:38:12 -0700 Subject: [PATCH 15/25] core: add header notice for update control --- src/uucore/src/lib/mods/update_control.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs index c0b2cc5ef99..cf5deb55c72 100644 --- a/src/uucore/src/lib/mods/update_control.rs +++ b/src/uucore/src/lib/mods/update_control.rs @@ -1,3 +1,10 @@ +// This file is part of the uutils coreutils package. +// +// (c) John Shin +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + //! Implement GNU-style update functionality. //! //! - pre-defined [`clap`-Arguments][1] for inclusion in utilities that @@ -28,7 +35,7 @@ //! .arg(update_control::arguments::update()) //! .arg(update_control::arguments::update_no_args()) //! .get_matches_from(vec![ -//! "command", "--update=older" +//! "commmand", "--update=older" //! ]); //! //! let update_mode = update_control::determine_update_mode(&matches); From 06d4603bead429a17f510cc06f8886bf463b908f Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 12:40:04 -0700 Subject: [PATCH 16/25] core: fix typo in update control --- src/uucore/src/lib/mods/update_control.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs index cf5deb55c72..83dd505cf0f 100644 --- a/src/uucore/src/lib/mods/update_control.rs +++ b/src/uucore/src/lib/mods/update_control.rs @@ -68,7 +68,7 @@ pub mod arguments { pub static OPT_UPDATE: &str = "update"; pub static OPT_UPDATE_NO_ARG: &str = "u"; - // `--update` argument, defaults to `older` if no values after provided + // `--update` argument, defaults to `older` if no values are provided pub fn update() -> clap::Arg { clap::Arg::new(OPT_UPDATE) .long("update") From 460d3460691a5b27141b247e792ffc9605fe445f Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 12:42:39 -0700 Subject: [PATCH 17/25] core: remove '' case for the update argument --- src/uucore/src/lib/mods/update_control.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs index 83dd505cf0f..13057dfc552 100644 --- a/src/uucore/src/lib/mods/update_control.rs +++ b/src/uucore/src/lib/mods/update_control.rs @@ -73,7 +73,7 @@ pub mod arguments { clap::Arg::new(OPT_UPDATE) .long("update") .help("move only when the SOURCE file is newer than the destination file or when the destination file is missing") - .value_parser(["", "none", "all", "older"]) + .value_parser(["none", "all", "older"]) .num_args(0..=1) .default_missing_value("older") .require_equals(true) @@ -125,7 +125,7 @@ pub fn determine_update_mode(matches: &ArgMatches) -> UpdateMode { if matches.contains_id(arguments::OPT_UPDATE) { if let Some(mode) = matches.get_one::(arguments::OPT_UPDATE) { match mode.as_str() { - "all" | "" => UpdateMode::ReplaceAll, + "all" => UpdateMode::ReplaceAll, "none" => UpdateMode::ReplaceNone, "older" => UpdateMode::ReplaceIfOlder, _ => unreachable!("other args restricted by clap"), From 3b8f3d04f47d5d6b5194e84cf54ea00d25baec83 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 12:49:49 -0700 Subject: [PATCH 18/25] core: remove unnecessary if statement in update control --- src/uucore/src/lib/mods/update_control.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs index 13057dfc552..3cd81e0120a 100644 --- a/src/uucore/src/lib/mods/update_control.rs +++ b/src/uucore/src/lib/mods/update_control.rs @@ -122,16 +122,12 @@ pub mod arguments { /// assert_eq!(update_mode, UpdateMode::ReplaceAll) /// } pub fn determine_update_mode(matches: &ArgMatches) -> UpdateMode { - if matches.contains_id(arguments::OPT_UPDATE) { - if let Some(mode) = matches.get_one::(arguments::OPT_UPDATE) { - match mode.as_str() { - "all" => UpdateMode::ReplaceAll, - "none" => UpdateMode::ReplaceNone, - "older" => UpdateMode::ReplaceIfOlder, - _ => unreachable!("other args restricted by clap"), - } - } else { - unreachable!("other args restricted by clap") + if let Some(mode) = matches.get_one::(arguments::OPT_UPDATE) { + match mode.as_str() { + "all" => UpdateMode::ReplaceAll, + "none" => UpdateMode::ReplaceNone, + "older" => UpdateMode::ReplaceIfOlder, + _ => unreachable!("other args restricted by clap"), } } else if matches.get_flag(arguments::OPT_UPDATE_NO_ARG) { // short form of this option is equivalent to using --update=older From 6a100976c7fe5322feab654bd1f381741def4950 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 13:30:38 -0700 Subject: [PATCH 19/25] mv: simplify tests for update --- tests/by-util/test_mv.rs | 40 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 29a22ce1029..d0885a92a98 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -724,15 +724,11 @@ fn test_mv_arg_update_none() { let file1 = "test_mv_arg_update_none_file1"; let file2 = "test_mv_arg_update_none_file2"; - - at.touch(file1); - at.touch(file2); - let file1_content = "file1 content\n"; let file2_content = "file2 content\n"; - at.append(file1, file1_content); - at.append(file2, file2_content); + at.write(file1, file1_content); + at.write(file2, file2_content); ucmd.arg(file1) .arg(file2) @@ -750,15 +746,11 @@ fn test_mv_arg_update_all() { let file1 = "test_mv_arg_update_none_file1"; let file2 = "test_mv_arg_update_none_file2"; - - at.touch(file1); - at.touch(file2); - let file1_content = "file1 content\n"; let file2_content = "file2 content\n"; - at.append(file1, file1_content); - at.append(file2, file2_content); + at.write(file1, file1_content); + at.write(file2, file2_content); ucmd.arg(file1) .arg(file2) @@ -779,13 +771,11 @@ fn test_mv_arg_update_older_dest_not_older() { let old_content = "file1 content\n"; let new_content = "file2 content\n"; - at.touch(old); - at.append(old, old_content); + at.write(old, old_content); sleep(Duration::from_secs(1)); - at.touch(new); - at.append(new, new_content); + at.write(new, new_content); ucmd.arg(old) .arg(new) @@ -806,13 +796,11 @@ fn test_mv_arg_update_older_dest_older() { let old_content = "file1 content\n"; let new_content = "file2 content\n"; - at.touch(old); - at.append(old, old_content); + at.write(old, old_content); sleep(Duration::from_secs(1)); - at.touch(new); - at.append(new, new_content); + at.write(new, new_content); ucmd.arg(new) .arg(old) @@ -834,13 +822,11 @@ fn test_mv_arg_update_short_overwrite() { let old_content = "file1 content\n"; let new_content = "file2 content\n"; - at.touch(old); - at.append(old, old_content); + at.write(old, old_content); sleep(Duration::from_secs(1)); - at.touch(new); - at.append(new, new_content); + at.write(new, new_content); ucmd.arg(new) .arg(old) @@ -862,13 +848,11 @@ fn test_mv_arg_update_short_no_overwrite() { let old_content = "file1 content\n"; let new_content = "file2 content\n"; - at.touch(old); - at.append(old, old_content); + at.write(old, old_content); sleep(Duration::from_secs(1)); - at.touch(new); - at.append(new, new_content); + at.write(new, new_content); ucmd.arg(old) .arg(new) From c0e4e4f757f52c31934bfc2efb36ef462bd21bf9 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 13:35:06 -0700 Subject: [PATCH 20/25] cp: simplify tests for update --- tests/by-util/test_cp.rs | 55 +++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index a933343b195..33bdcb3a8b9 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -279,15 +279,13 @@ fn test_cp_arg_update_all() { fn test_cp_arg_update_older_dest_not_older_than_src() { let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_older_file1"; - let new = "test_cp_arg_update_older_file2"; + let old = "test_cp_arg_update_dest_not_older_file1"; + let new = "test_cp_arg_update_dest_not_older_file2"; + let old_content = "old content\n"; + let new_content = "new content\n"; - at.touch(old); - sleep(Duration::from_secs(1)); - at.touch(new); - - at.append(old, "old content\n"); - at.append(new, "new content\n"); + at.write(old, old_content); + at.write(new, new_content); ucmd.arg(old) .arg(new) @@ -303,14 +301,16 @@ fn test_cp_arg_update_older_dest_not_older_than_src() { fn test_cp_arg_update_older_dest_older_than_src() { let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_older_file1"; - let new = "test_cp_arg_update_older_file2"; + let old = "test_cp_arg_update_dest_older_file1"; + let new = "test_cp_arg_update_dest_older_file2"; + let old_content = "old content\n"; + let new_content = "new content\n"; + + at.write(old, old_content); - at.touch(old); - at.append(old, "old content\n"); sleep(Duration::from_secs(1)); - at.touch(new); - at.append(new, "new content\n"); + + at.write(new, new_content); ucmd.arg(new) .arg(old) @@ -327,15 +327,16 @@ fn test_cp_arg_update_short_fail() { // same as --update=older let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_older_file1"; - let new = "test_cp_arg_update_older_file2"; + let old = "test_cp_arg_update_short_no_overwrite_file1"; + let new = "test_cp_arg_update_short_no_overwrite_file2"; + let old_content = "old content\n"; + let new_content = "new content\n"; + + at.write(old, old_content); - at.touch(old); sleep(Duration::from_secs(1)); - at.touch(new); - at.append(old, "old content\n"); - at.append(new, "new content\n"); + at.write(new, new_content); ucmd.arg(old) .arg(new) @@ -352,14 +353,16 @@ fn test_cp_arg_update_short_succeed() { // same as --update=older let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_older_file1"; - let new = "test_cp_arg_update_older_file2"; + let old = "test_cp_arg_update_short_overwrite_file1"; + let new = "test_cp_arg_update_short_overwrite_file2"; + let old_content = "old content\n"; + let new_content = "new content\n"; + + at.write(old, old_content); - at.touch(old); - at.touch(new); + sleep(Duration::from_secs(1)); - at.append(old, "old content\n"); - at.append(new, "new content\n"); + at.write(new, new_content); ucmd.arg(new) .arg(old) From 983fee0cead753e2a0482bd5dfd3fe5d5018830f Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 13:35:52 -0700 Subject: [PATCH 21/25] cp: fix wrong test names for update --- tests/by-util/test_cp.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 33bdcb3a8b9..7f293edcf5f 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -323,7 +323,7 @@ fn test_cp_arg_update_older_dest_older_than_src() { } #[test] -fn test_cp_arg_update_short_fail() { +fn test_cp_arg_update_short_no_overwrite() { // same as --update=older let (at, mut ucmd) = at_and_ucmd!(); @@ -349,7 +349,7 @@ fn test_cp_arg_update_short_fail() { } #[test] -fn test_cp_arg_update_short_succeed() { +fn test_cp_arg_update_short_overwrite() { // same as --update=older let (at, mut ucmd) = at_and_ucmd!(); @@ -371,7 +371,7 @@ fn test_cp_arg_update_short_succeed() { .no_stderr() .no_stdout(); - assert_eq!(at.read(new), "new content\n") + assert_eq!(at.read(old), "new content\n") } #[test] From 918c36b4851f4ea79894687d7bcf1d896c08188b Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 13:46:08 -0700 Subject: [PATCH 22/25] cp: write test for multiple update args --- tests/by-util/test_cp.rs | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 7f293edcf5f..5376ba36382 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -374,6 +374,62 @@ fn test_cp_arg_update_short_overwrite() { assert_eq!(at.read(old), "new content\n") } +#[test] +fn test_cp_arg_update_none_then_all() { + // take last if multiple update args are supplied, + // update=all wins in this case + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_cp_arg_update_none_then_all_file1"; + let new = "test_cp_arg_update_none_then_all_file2"; + let old_content = "old content\n"; + let new_content = "new content\n"; + + at.write(old, old_content); + + sleep(Duration::from_secs(1)); + + at.write(new, new_content); + + ucmd.arg(old) + .arg(new) + .arg("--update=none") + .arg("--update=all") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), "old content\n") +} + +#[test] +fn test_cp_arg_update_all_then_none() { + // take last if multiple update args are supplied, + // update=none wins in this case + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_cp_arg_update_all_then_none_file1"; + let new = "test_cp_arg_update_all_then_none_file2"; + let old_content = "old content\n"; + let new_content = "new content\n"; + + at.write(old, old_content); + + sleep(Duration::from_secs(1)); + + at.write(new, new_content); + + ucmd.arg(old) + .arg(new) + .arg("--update=all") + .arg("--update=none") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), "new content\n") +} + #[test] fn test_cp_arg_interactive() { let (at, mut ucmd) = at_and_ucmd!(); From 8ad2fa3cc1cfe1d238607a0b783bbde681d4a594 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 13:46:43 -0700 Subject: [PATCH 23/25] mv: write test for multiple update args --- tests/by-util/test_mv.rs | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index d0885a92a98..5bf41b09be8 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -787,6 +787,62 @@ fn test_mv_arg_update_older_dest_not_older() { assert_eq!(at.read(new), new_content) } +#[test] +fn test_cp_arg_update_none_then_all() { + // take last if multiple update args are supplied, + // update=all wins in this case + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_cp_arg_update_none_then_all_file1"; + let new = "test_cp_arg_update_none_then_all_file2"; + let old_content = "old content\n"; + let new_content = "new content\n"; + + at.write(old, old_content); + + sleep(Duration::from_secs(1)); + + at.write(new, new_content); + + ucmd.arg(old) + .arg(new) + .arg("--update=none") + .arg("--update=all") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), "old content\n") +} + +#[test] +fn test_cp_arg_update_all_then_none() { + // take last if multiple update args are supplied, + // update=none wins in this case + let (at, mut ucmd) = at_and_ucmd!(); + + let old = "test_cp_arg_update_all_then_none_file1"; + let new = "test_cp_arg_update_all_then_none_file2"; + let old_content = "old content\n"; + let new_content = "new content\n"; + + at.write(old, old_content); + + sleep(Duration::from_secs(1)); + + at.write(new, new_content); + + ucmd.arg(old) + .arg(new) + .arg("--update=all") + .arg("--update=none") + .succeeds() + .no_stderr() + .no_stdout(); + + assert_eq!(at.read(new), "new content\n") +} + #[test] fn test_mv_arg_update_older_dest_older() { let (at, mut ucmd) = at_and_ucmd!(); From 898628fa3aae6f48d722e2e8821ca0e98e0017d6 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 2 May 2023 13:54:31 -0700 Subject: [PATCH 24/25] core: fix typo in update control --- src/uucore/src/lib/mods/update_control.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uucore/src/lib/mods/update_control.rs b/src/uucore/src/lib/mods/update_control.rs index 3cd81e0120a..e46afd18522 100644 --- a/src/uucore/src/lib/mods/update_control.rs +++ b/src/uucore/src/lib/mods/update_control.rs @@ -35,7 +35,7 @@ //! .arg(update_control::arguments::update()) //! .arg(update_control::arguments::update_no_args()) //! .get_matches_from(vec![ -//! "commmand", "--update=older" +//! "command", "--update=older" //! ]); //! //! let update_mode = update_control::determine_update_mode(&matches); From 923a62c6beb8fe26c371576e424c3e9d296e18f7 Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Wed, 3 May 2023 10:07:46 +0200 Subject: [PATCH 25/25] mv: fix function/file names in tests --- tests/by-util/test_mv.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 5bf41b09be8..ce9e26a819e 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -788,13 +788,13 @@ fn test_mv_arg_update_older_dest_not_older() { } #[test] -fn test_cp_arg_update_none_then_all() { +fn test_mv_arg_update_none_then_all() { // take last if multiple update args are supplied, // update=all wins in this case let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_none_then_all_file1"; - let new = "test_cp_arg_update_none_then_all_file2"; + let old = "test_mv_arg_update_none_then_all_file1"; + let new = "test_mv_arg_update_none_then_all_file2"; let old_content = "old content\n"; let new_content = "new content\n"; @@ -816,13 +816,13 @@ fn test_cp_arg_update_none_then_all() { } #[test] -fn test_cp_arg_update_all_then_none() { +fn test_mv_arg_update_all_then_none() { // take last if multiple update args are supplied, // update=none wins in this case let (at, mut ucmd) = at_and_ucmd!(); - let old = "test_cp_arg_update_all_then_none_file1"; - let new = "test_cp_arg_update_all_then_none_file2"; + let old = "test_mv_arg_update_all_then_none_file1"; + let new = "test_mv_arg_update_all_then_none_file2"; let old_content = "old content\n"; let new_content = "new content\n";