diff --git a/src/uu/cp/Cargo.toml b/src/uu/cp/Cargo.toml index 025c168d3..2c71b725f 100644 --- a/src/uu/cp/Cargo.toml +++ b/src/uu/cp/Cargo.toml @@ -19,7 +19,7 @@ edition = "2021" path = "src/cp.rs" [dependencies] -clap = { version = "3.2", features = ["wrap_help", "cargo"] } +clap = { version = "4.0", features = ["wrap_help", "cargo"] } filetime = "0.2" libc = "0.2.135" quick-error = "2.0.1" diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 16d498744..8b9226c1a 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -31,7 +31,7 @@ use std::path::{Path, PathBuf, StripPrefixError}; use std::str::FromStr; use std::string::ToString; -use clap::{crate_version, Arg, ArgMatches, Command}; +use clap::{crate_version, Arg, ArgAction, ArgMatches, Command}; use filetime::FileTime; #[cfg(unix)] use libc::mkfifo; @@ -92,6 +92,8 @@ quick_error! { /// Invalid arguments to backup Backup(description: String) { display("{}\nTry '{} --help' for more information.", description, uucore::execution_phrase()) } + + NotADirectory(path: String) { display("'{}' is not a directory", path) } } } @@ -224,7 +226,6 @@ pub struct Options { } static ABOUT: &str = "Copy SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY."; -static LONG_HELP: &str = ""; static EXIT_ERR: i32 = 1; const USAGE: &str = "\ @@ -255,7 +256,6 @@ mod options { pub const PRESERVE: &str = "preserve"; pub const PRESERVE_DEFAULT_ATTRIBUTES: &str = "preserve-default-attributes"; pub const RECURSIVE: &str = "recursive"; - pub const RECURSIVE_ALIAS: &str = "recursive_alias"; pub const REFLINK: &str = "reflink"; pub const REMOVE_DESTINATION: &str = "remove-destination"; pub const SPARSE: &str = "sparse"; @@ -289,7 +289,7 @@ static DEFAULT_ATTRIBUTES: &[Attribute] = &[ Attribute::Timestamps, ]; -pub fn uu_app<'a>() -> Command<'a> { +pub fn uu_app() -> Command { const MODE_ARGS: &[&str] = &[ options::LINK, options::REFLINK, @@ -309,13 +309,6 @@ pub fn uu_app<'a>() -> Command<'a> { .long(options::TARGET_DIRECTORY) .value_name(options::TARGET_DIRECTORY) .value_hint(clap::ValueHint::DirPath) - .takes_value(true) - .validator(|s| { - if Path::new(s).is_dir() { - return Ok(()); - } - Err(format!("'{}' is not a directory", s)) - }) .help("copy all SOURCE arguments into target-directory"), ) .arg( @@ -323,58 +316,62 @@ pub fn uu_app<'a>() -> Command<'a> { .short('T') .long(options::NO_TARGET_DIRECTORY) .conflicts_with(options::TARGET_DIRECTORY) - .help("Treat DEST as a regular file and not a directory"), + .help("Treat DEST as a regular file and not a directory") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::INTERACTIVE) .short('i') .long(options::INTERACTIVE) .overrides_with(options::NO_CLOBBER) - .help("ask before overwriting files"), + .help("ask before overwriting files") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::LINK) .short('l') .long(options::LINK) .overrides_with_all(MODE_ARGS) - .help("hard-link files instead of copying"), + .help("hard-link files instead of copying") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::NO_CLOBBER) .short('n') .long(options::NO_CLOBBER) .overrides_with(options::INTERACTIVE) - .help("don't overwrite a file that already exists"), + .help("don't overwrite a file that already exists") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::RECURSIVE) .short('r') + .visible_short_alias('R') .long(options::RECURSIVE) // --archive sets this option - .help("copy directories recursively"), - ) - .arg( - Arg::new(options::RECURSIVE_ALIAS) - .short('R') - .help("same as -r"), + .help("copy directories recursively") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::STRIP_TRAILING_SLASHES) .long(options::STRIP_TRAILING_SLASHES) - .help("remove any trailing slashes from each SOURCE argument"), + .help("remove any trailing slashes from each SOURCE argument") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::VERBOSE) .short('v') .long(options::VERBOSE) - .help("explicitly state what is being done"), + .help("explicitly state what is being done") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::SYMBOLIC_LINK) .short('s') .long(options::SYMBOLIC_LINK) .overrides_with_all(MODE_ARGS) - .help("make symbolic links instead of copying"), + .help("make symbolic links instead of copying") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::FORCE) @@ -384,7 +381,8 @@ pub fn uu_app<'a>() -> Command<'a> { "if an existing destination file cannot be opened, remove it and \ try again (this option is ignored when the -n option is also used). \ Currently not implemented for Windows.", - ), + ) + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::REMOVE_DESTINATION) @@ -394,7 +392,8 @@ pub fn uu_app<'a>() -> Command<'a> { "remove each existing destination file before attempting to open it \ (contrast with --force). On Windows, currently only works for \ writeable files.", - ), + ) + .action(ArgAction::SetTrue), ) .arg(backup_control::arguments::backup()) .arg(backup_control::arguments::backup_no_args()) @@ -406,36 +405,36 @@ pub fn uu_app<'a>() -> Command<'a> { .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) - .takes_value(true) .value_name("WHEN") .overrides_with_all(MODE_ARGS) .require_equals(true) .default_missing_value("always") .value_parser(["auto", "always", "never"]) - .min_values(0) + .num_args(0..=1) .help("control clone/CoW copies. See below"), ) .arg( Arg::new(options::ATTRIBUTES_ONLY) .long(options::ATTRIBUTES_ONLY) .overrides_with_all(MODE_ARGS) - .help("Don't copy the file data, just the attributes"), + .help("Don't copy the file data, just the attributes") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::PRESERVE) .long(options::PRESERVE) - .takes_value(true) - .multiple_occurrences(true) + .action(ArgAction::Append) .use_value_delimiter(true) .value_parser(clap::builder::PossibleValuesParser::new( PRESERVABLE_ATTRIBUTES, )) - .min_values(0) + .num_args(0..) .value_name("ATTR_LIST") .overrides_with_all(&[ options::ARCHIVE, @@ -454,12 +453,12 @@ pub fn uu_app<'a>() -> Command<'a> { .short('p') .long(options::PRESERVE_DEFAULT_ATTRIBUTES) .overrides_with_all(&[options::PRESERVE, options::NO_PRESERVE, options::ARCHIVE]) - .help("same as --preserve=mode,ownership(unix only),timestamps"), + .help("same as --preserve=mode,ownership(unix only),timestamps") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::NO_PRESERVE) .long(options::NO_PRESERVE) - .takes_value(true) .value_name("ATTR_LIST") .overrides_with_all(&[ options::PRESERVE_DEFAULT_ATTRIBUTES, @@ -472,7 +471,8 @@ pub fn uu_app<'a>() -> Command<'a> { Arg::new(options::PARENTS) .long(options::PARENTS) .alias(options::PARENT) - .help("use full source file name under DIRECTORY"), + .help("use full source file name under DIRECTORY") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::NO_DEREFERENCE) @@ -480,19 +480,22 @@ pub fn uu_app<'a>() -> Command<'a> { .long(options::NO_DEREFERENCE) .overrides_with(options::DEREFERENCE) // -d sets this option - .help("never follow symbolic links in SOURCE"), + .help("never follow symbolic links in SOURCE") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::DEREFERENCE) .short('L') .long(options::DEREFERENCE) .overrides_with(options::NO_DEREFERENCE) - .help("always follow symbolic links in SOURCE"), + .help("always follow symbolic links in SOURCE") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::CLI_SYMBOLIC_LINKS) .short('H') - .help("follow command-line symbolic links in SOURCE"), + .help("follow command-line symbolic links in SOURCE") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::ARCHIVE) @@ -503,23 +506,25 @@ pub fn uu_app<'a>() -> Command<'a> { options::PRESERVE, options::NO_PRESERVE, ]) - .help("Same as -dR --preserve=all"), + .help("Same as -dR --preserve=all") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::NO_DEREFERENCE_PRESERVE_LINKS) .short('d') - .help("same as --no-dereference --preserve=links"), + .help("same as --no-dereference --preserve=links") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::ONE_FILE_SYSTEM) .short('x') .long(options::ONE_FILE_SYSTEM) - .help("stay on this file system"), + .help("stay on this file system") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::SPARSE) .long(options::SPARSE) - .takes_value(true) .value_name("WHEN") .value_parser(["never", "auto", "always"]) .help("NotImplemented: control creation of sparse files. See below"), @@ -529,12 +534,12 @@ pub fn uu_app<'a>() -> Command<'a> { Arg::new(options::COPY_CONTENTS) .long(options::COPY_CONTENTS) .overrides_with(options::ATTRIBUTES_ONLY) - .help("NotImplemented: copy contents of special files when recursive"), + .help("NotImplemented: copy contents of special files when recursive") + .action(ArgAction::SetTrue), ) .arg( Arg::new(options::CONTEXT) .long(options::CONTEXT) - .takes_value(true) .value_name("CTX") .help( "NotImplemented: set SELinux security context of destination file to \ @@ -544,29 +549,26 @@ pub fn uu_app<'a>() -> Command<'a> { // END TODO .arg( Arg::new(options::PATHS) - .multiple_occurrences(true) + .action(ArgAction::Append) .value_hint(clap::ValueHint::AnyPath), ) } #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let after_help = &*format!( - "{}\n{}", - LONG_HELP, - backup_control::BACKUP_CONTROL_LONG_HELP - ); - let matches = uu_app().after_help(after_help).try_get_matches_from(args); + let matches = uu_app() + .after_help(backup_control::BACKUP_CONTROL_LONG_HELP) + .try_get_matches_from(args); // The error is parsed here because we do not want version or help being printed to stderr. if let Err(e) = matches { - let mut app = uu_app().after_help(after_help); + let mut app = uu_app().after_help(backup_control::BACKUP_CONTROL_LONG_HELP); match e.kind() { - clap::ErrorKind::DisplayHelp => { + clap::error::ErrorKind::DisplayHelp => { app.print_help()?; } - clap::ErrorKind::DisplayVersion => println!("{}", app.render_version()), + clap::error::ErrorKind::DisplayVersion => println!("{}", app.render_version()), _ => return Err(Box::new(e.with_exit_code(1))), }; } else if let Ok(matches) = matches { @@ -603,9 +605,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { impl ClobberMode { fn from_matches(matches: &ArgMatches) -> Self { - if matches.contains_id(options::FORCE) { + if matches.get_flag(options::FORCE) { Self::Force - } else if matches.contains_id(options::REMOVE_DESTINATION) { + } else if matches.get_flag(options::REMOVE_DESTINATION) { Self::RemoveDestination } else { Self::Standard @@ -615,9 +617,9 @@ impl ClobberMode { impl OverwriteMode { fn from_matches(matches: &ArgMatches) -> Self { - if matches.contains_id(options::INTERACTIVE) { + if matches.get_flag(options::INTERACTIVE) { Self::Interactive(ClobberMode::from_matches(matches)) - } else if matches.contains_id(options::NO_CLOBBER) { + } else if matches.get_flag(options::NO_CLOBBER) { Self::NoClobber } else { Self::Clobber(ClobberMode::from_matches(matches)) @@ -627,13 +629,13 @@ impl OverwriteMode { impl CopyMode { fn from_matches(matches: &ArgMatches) -> Self { - if matches.contains_id(options::LINK) { + if matches.get_flag(options::LINK) { Self::Link - } else if matches.contains_id(options::SYMBOLIC_LINK) { + } else if matches.get_flag(options::SYMBOLIC_LINK) { Self::SymLink - } else if matches.contains_id(options::UPDATE) { + } else if matches.get_flag(options::UPDATE) { Self::Update - } else if matches.contains_id(options::ATTRIBUTES_ONLY) { + } else if matches.get_flag(options::ATTRIBUTES_ONLY) { Self::AttrOnly } else { Self::Copy @@ -693,14 +695,15 @@ impl Options { ]; for not_implemented_opt in not_implemented_opts { - if matches.contains_id(not_implemented_opt) { + if matches.contains_id(not_implemented_opt) + && matches.value_source(not_implemented_opt) + == Some(clap::parser::ValueSource::CommandLine) + { return Err(Error::NotImplemented(not_implemented_opt.to_string())); } } - let recursive = matches.contains_id(options::RECURSIVE) - || matches.contains_id(options::RECURSIVE_ALIAS) - || matches.contains_id(options::ARCHIVE); + let recursive = matches.get_flag(options::RECURSIVE) || matches.get_flag(options::ARCHIVE); let backup_mode = match backup_control::determine_backup_mode(matches) { Err(e) => return Err(Error::Backup(format!("{}", e))), @@ -712,11 +715,17 @@ impl Options { let overwrite = OverwriteMode::from_matches(matches); // Parse target directory options - let no_target_dir = matches.contains_id(options::NO_TARGET_DIRECTORY); + let no_target_dir = matches.get_flag(options::NO_TARGET_DIRECTORY); let target_dir = matches .get_one::(options::TARGET_DIRECTORY) .map(ToString::to_string); + if let Some(dir) = &target_dir { + if !Path::new(dir).is_dir() { + return Err(Error::NotADirectory(dir.clone())); + } + }; + // Parse attributes to preserve let mut preserve_attributes: Vec = if matches.contains_id(options::PRESERVE) { match matches.get_many::(options::PRESERVE) { @@ -734,12 +743,12 @@ impl Options { attributes } } - } else if matches.contains_id(options::ARCHIVE) { + } else if matches.get_flag(options::ARCHIVE) { // --archive is used. Same as --preserve=all add_all_attributes() - } else if matches.contains_id(options::NO_DEREFERENCE_PRESERVE_LINKS) { + } else if matches.get_flag(options::NO_DEREFERENCE_PRESERVE_LINKS) { vec![Attribute::Links] - } else if matches.contains_id(options::PRESERVE_DEFAULT_ATTRIBUTES) { + } else if matches.get_flag(options::PRESERVE_DEFAULT_ATTRIBUTES) { DEFAULT_ATTRIBUTES.to_vec() } else { vec![] @@ -751,21 +760,21 @@ impl Options { preserve_attributes.sort_unstable(); let options = Self { - attributes_only: matches.contains_id(options::ATTRIBUTES_ONLY), - copy_contents: matches.contains_id(options::COPY_CONTENTS), - cli_dereference: matches.contains_id(options::CLI_SYMBOLIC_LINKS), + attributes_only: matches.get_flag(options::ATTRIBUTES_ONLY), + copy_contents: matches.get_flag(options::COPY_CONTENTS), + cli_dereference: matches.get_flag(options::CLI_SYMBOLIC_LINKS), copy_mode: CopyMode::from_matches(matches), // No dereference is set with -p, -d and --archive - dereference: !(matches.contains_id(options::NO_DEREFERENCE) - || matches.contains_id(options::NO_DEREFERENCE_PRESERVE_LINKS) - || matches.contains_id(options::ARCHIVE) + dereference: !(matches.get_flag(options::NO_DEREFERENCE) + || matches.get_flag(options::NO_DEREFERENCE_PRESERVE_LINKS) + || matches.get_flag(options::ARCHIVE) || recursive) - || matches.contains_id(options::DEREFERENCE), - one_file_system: matches.contains_id(options::ONE_FILE_SYSTEM), - parents: matches.contains_id(options::PARENTS), - update: matches.contains_id(options::UPDATE), - verbose: matches.contains_id(options::VERBOSE), - strip_trailing_slashes: matches.contains_id(options::STRIP_TRAILING_SLASHES), + || 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), + verbose: matches.get_flag(options::VERBOSE), + strip_trailing_slashes: matches.get_flag(options::STRIP_TRAILING_SLASHES), reflink_mode: { if let Some(reflink) = matches.get_one::(options::REFLINK) { match reflink.as_str() { diff --git a/src/uucore/src/lib/mods/backup_control.rs b/src/uucore/src/lib/mods/backup_control.rs index 2a30cc86d..52b2771c6 100644 --- a/src/uucore/src/lib/mods/backup_control.rs +++ b/src/uucore/src/lib/mods/backup_control.rs @@ -94,7 +94,7 @@ pub static BACKUP_CONTROL_VALUES: &[&str] = &[ "simple", "never", "numbered", "t", "existing", "nil", "none", "off", ]; -pub static BACKUP_CONTROL_LONG_HELP: &str = +pub const BACKUP_CONTROL_LONG_HELP: &str = "The backup suffix is '~', unless set with --suffix or SIMPLE_BACKUP_SUFFIX. The version control method may be selected via the --backup option or through the VERSION_CONTROL environment variable. Here are the values: @@ -198,6 +198,8 @@ impl Display for BackupError { /// This way the backup-specific arguments are handled uniformly across /// utilities and can be maintained in one central place. pub mod arguments { + use clap::ArgAction; + extern crate clap; pub static OPT_BACKUP: &str = "backupopt_backup"; @@ -220,6 +222,7 @@ pub mod arguments { clap::Arg::new(OPT_BACKUP_NO_ARG) .short('b') .help("like --backup but does not accept an argument") + .action(ArgAction::SetTrue) } /// '-S, --suffix' argument @@ -347,7 +350,7 @@ pub fn determine_backup_mode(matches: &ArgMatches) -> UResult { // Default if no argument is provided to '--backup' Ok(BackupMode::ExistingBackup) } - } else if matches.contains_id(arguments::OPT_BACKUP_NO_ARG) { + } else if matches.get_flag(arguments::OPT_BACKUP_NO_ARG) { // the short form of this option, -b does not accept any argument. // Using -b is equivalent to using --backup=existing. Ok(BackupMode::ExistingBackup) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 2c7157851..17cc25d82 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1395,7 +1395,7 @@ fn test_cp_reflink_bad() { .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_EXISTING_FILE) .fails() - .stderr_contains("error: \"bad\" isn't a valid value for '--reflink[=...]'"); + .stderr_contains("error: \"bad\" isn't a valid value for '--reflink[=]'"); } #[test] @@ -1551,6 +1551,7 @@ fn test_cp_sparse_never_reflink_always() { } #[cfg(any(target_os = "linux", target_os = "android"))] +#[cfg(feature = "truncate")] #[test] fn test_cp_reflink_always_override() { let scene = TestScenario::new(util_name!()); @@ -1639,6 +1640,7 @@ fn test_copy_dir_symlink() { #[test] #[cfg(not(target_os = "freebsd"))] // FIXME: fix this test for FreeBSD +#[cfg(feature = "ln")] fn test_copy_dir_with_symlinks() { let (at, mut ucmd) = at_and_ucmd!(); at.mkdir("dir");