From 2edfe32c486f654353496ceb91e0858033cf5123 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 13 Nov 2020 18:20:57 +0100 Subject: [PATCH] refactor(install): move to clap --- Cargo.lock | 2 +- src/uu/install/Cargo.toml | 2 +- src/uu/install/src/install.rs | 389 ++++++++++++++++++---------------- tests/by-util/test_install.rs | 2 +- 4 files changed, 213 insertions(+), 182 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 931470178..49372d3e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1677,7 +1677,7 @@ dependencies = [ name = "uu_install" version = "0.0.1" dependencies = [ - "getopts 0.2.21 (registry+https://github.com/rust-lang/crates.io-index)", + "clap 2.33.3 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)", "time 0.1.42 (registry+https://github.com/rust-lang/crates.io-index)", "uucore 0.0.4 (git+https://github.com/uutils/uucore.git?branch=canary)", diff --git a/src/uu/install/Cargo.toml b/src/uu/install/Cargo.toml index 3e00a9bd8..54680987b 100644 --- a/src/uu/install/Cargo.toml +++ b/src/uu/install/Cargo.toml @@ -18,7 +18,7 @@ edition = "2018" path = "src/install.rs" [dependencies] -getopts = "0.2.18" +clap = "2.33" libc = ">= 0.2" uucore = { version="0.0.4", package="uucore", git="https://github.com/uutils/uucore.git", branch="canary", features=["mode"] } uucore_procs = { version="0.0.4", package="uucore_procs", git="https://github.com/uutils/uucore.git", branch="canary" } diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 4970689fb..c744579db 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -7,7 +7,7 @@ // spell-checker:ignore (ToDO) rwxr sourcepath targetpath -extern crate getopts; +extern crate clap; extern crate libc; mod mode; @@ -15,15 +15,11 @@ mod mode; #[macro_use] extern crate uucore; +use clap::{App, Arg, ArgMatches}; use std::fs; use std::path::{Path, PathBuf}; use std::result::Result; -static NAME: &str = "install"; -static SUMMARY: &str = "Copy SOURCE to DEST or multiple SOURCE(s) to the existing - DIRECTORY, while setting permission modes and owner/group"; -static LONG_HELP: &str = ""; - const DEFAULT_MODE: u32 = 755; #[allow(dead_code)] @@ -52,14 +48,179 @@ impl Behavior { } } +static ABOUT: &str = "Copy SOURCE to DEST or multiple SOURCE(s) to the existing + DIRECTORY, while setting permission modes and owner/group"; +static VERSION: &str = env!("CARGO_PKG_VERSION"); + +static OPT_COMPARE: &str = "compare"; +static OPT_BACKUP: &str = "backup"; +static OPT_BACKUP_2: &str = "backup2"; +static OPT_DIRECTORY: &str = "directory"; +static OPT_IGNORED: &str = "ignored"; +static OPT_CREATED: &str = "created"; +static OPT_GROUP: &str = "group"; +static OPT_MODE: &str = "mode"; +static OPT_OWNER: &str = "owner"; +static OPT_PRESERVE_TIMESTAMPS: &str = "preserve-timestamps"; +static OPT_STRIP: &str = "strip"; +static OPT_STRIP_PROGRAM: &str = "strip-program"; +static OPT_SUFFIX: &str = "suffix"; +static OPT_TARGET_DIRECTORY: &str = "target-directory"; +static OPT_NO_TARGET_DIRECTORY: &str = "no-target-directory"; +static OPT_VERBOSE: &str = "verbose"; +static OPT_PRESERVE_CONTEXT: &str = "preserve-context"; +static OPT_CONTEXT: &str = "context"; + +static ARG_FILES: &str = "files"; + +fn get_usage() -> String { + format!("{0} [OPTION]... [FILE]...", executable!()) +} + /// Main install utility function, called from main.rs. /// /// Returns a program return code. /// pub fn uumain(args: impl uucore::Args) -> i32 { - let args = args.collect_str(); + let usage = get_usage(); - let matches = parse_opts(args); + let matches = App::new(executable!()) + .version(VERSION) + .about(ABOUT) + .usage(&usage[..]) + .arg( + Arg::with_name(OPT_BACKUP) + .long(OPT_BACKUP) + .help("(unimplemented) make a backup of each existing destination file") + .value_name("CONTROL") + ) + // TODO implement flag + .arg( + Arg::with_name(OPT_BACKUP_2) + .short("b") + .help("(unimplemented) like --backup but does not accept an argument") + ) + .arg( + Arg::with_name(OPT_IGNORED) + .short("c") + .help("ignored") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_COMPARE) + .short("C") + .long(OPT_COMPARE) + .help("(unimplemented) compare each pair of source and destination files, and in some cases, do not modify the destination at all") + ) + .arg( + Arg::with_name(OPT_DIRECTORY) + .short("d") + .long(OPT_DIRECTORY) + .help("treat all arguments as directory names. create all components of the specified directories") + ) + + .arg( + // TODO implement flag + Arg::with_name(OPT_CREATED) + .short("D") + .help("(unimplemented) create all leading components of DEST except the last, then copy SOURCE to DEST") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_GROUP) + .short("g") + .long(OPT_GROUP) + .help("(unimplemented) set group ownership, instead of process's current group") + .value_name("GROUP") + ) + .arg( + Arg::with_name(OPT_MODE) + .short("m") + .long(OPT_MODE) + .help("set permission mode (as in chmod), instead of rwxr-xr-x") + .value_name("MODE") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_OWNER) + .short("o") + .long(OPT_OWNER) + .help("(unimplemented) set ownership (super-user only)") + .value_name("OWNER") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_PRESERVE_TIMESTAMPS) + .short("p") + .long(OPT_PRESERVE_TIMESTAMPS) + .help("(unimplemented) apply access/modification times of SOURCE files to corresponding destination files") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_STRIP) + .short("s") + .long(OPT_STRIP) + .help("(unimplemented) strip symbol tables") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_STRIP_PROGRAM) + .long(OPT_STRIP_PROGRAM) + .help("(unimplemented) program used to strip binaries") + .value_name("PROGRAM") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_SUFFIX) + .short("S") + .long(OPT_SUFFIX) + .help("(unimplemented) override the usual backup suffix") + .value_name("SUFFIX") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_TARGET_DIRECTORY) + .short("t") + .long(OPT_TARGET_DIRECTORY) + .help("(unimplemented) move all SOURCE arguments into DIRECTORY") + .value_name("DIRECTORY") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_NO_TARGET_DIRECTORY) + .short("T") + .long(OPT_NO_TARGET_DIRECTORY) + .help("(unimplemented) treat DEST as a normal file") + + ) + .arg( + Arg::with_name(OPT_VERBOSE) + .short("v") + .long(OPT_VERBOSE) + .help("explain what is being done") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_PRESERVE_CONTEXT) + .short("P") + .long(OPT_PRESERVE_CONTEXT) + .help("(unimplemented) preserve security context") + ) + .arg( + // TODO implement flag + Arg::with_name(OPT_CONTEXT) + .short("Z") + .long(OPT_CONTEXT) + .help("(unimplemented) set security context of files and directories") + .value_name("CONTEXT") + ) + .arg(Arg::with_name(ARG_FILES).multiple(true).takes_value(true)) + .get_matches_from(args); + + let mut paths: Vec = matches + .values_of(ARG_FILES) + .map(|v| v.map(ToString::to_string).collect()) + .unwrap_or_default(); if let Err(s) = check_unimplemented(&matches) { show_error!("Unimplemented feature: {}", s); @@ -73,146 +234,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } }; - let paths: Vec = { - #[allow(clippy::ptr_arg)] - fn string_to_path(s: &String) -> &Path { - Path::new(s) - }; - let to_owned = |p: &Path| p.to_owned(); - let arguments = matches.free.iter().map(string_to_path); - - arguments.map(to_owned).collect() - }; - match behavior.main_function { - MainFunction::Directory => directory(&paths[..], behavior), - MainFunction::Standard => standard(&paths[..], behavior), + MainFunction::Directory => directory(paths, behavior), + MainFunction::Standard => standard(paths, behavior), } } -/// Build a specification of the command line. -/// -/// Returns a getopts::Options struct. -/// -fn parse_opts(args: Vec) -> getopts::Matches { - let syntax = format!( - "SOURCE DEST - {} SOURCE... DIRECTORY", - NAME - ); - app!(&syntax, SUMMARY, LONG_HELP) - // TODO implement flag - .optflagopt( - "", - "backup", - "(unimplemented) make a backup of each existing destination\n \ - file", - "CONTROL", - ) - // TODO implement flag - .optflag( - "b", - "", - "(unimplemented) like --backup but does not accept an argument", - ) - .optflag("c", "", "ignored") - // TODO implement flag - .optflag( - "C", - "compare", - "(unimplemented) compare each pair of source and destination\n \ - files, and in some cases, do not modify the destination at all", - ) - .optflag( - "d", - "directory", - "treat all arguments as directory names.\n \ - create all components of the specified directories", - ) - // TODO implement flag - .optflag( - "D", - "", - "(unimplemented) create all leading components of DEST except the\n \ - last, then copy SOURCE to DEST", - ) - // TODO implement flag - .optflagopt( - "g", - "group", - "(unimplemented) set group ownership, instead of process's\n \ - current group", - "GROUP", - ) - .optflagopt( - "m", - "mode", - "set permission mode (as in chmod), instead\n \ - of rwxr-xr-x", - "MODE", - ) - // TODO implement flag - .optflagopt( - "o", - "owner", - "(unimplemented) set ownership (super-user only)", - "OWNER", - ) - // TODO implement flag - .optflag( - "p", - "preserve-timestamps", - "(unimplemented) apply access/modification times\n \ - of SOURCE files to corresponding destination files", - ) - // TODO implement flag - .optflag("s", "strip", "(unimplemented) strip symbol tables") - // TODO implement flag - .optflagopt( - "", - "strip-program", - "(unimplemented) program used to strip binaries", - "PROGRAM", - ) - // TODO implement flag - .optopt( - "S", - "suffix", - "(unimplemented) override the usual backup suffix", - "SUFFIX", - ) - // TODO implement flag - .optopt( - "t", - "target-directory", - "(unimplemented) move all SOURCE arguments into\n \ - DIRECTORY", - "DIRECTORY", - ) - // TODO implement flag - .optflag( - "T", - "no-target-directory", - "(unimplemented) treat DEST as a normal file", - ) - .optflag("v", "verbose", "explain what is being done") - // TODO implement flag - .optflag( - "P", - "preserve-context", - "(unimplemented) preserve security context", - ) - // TODO implement flag - .optflagopt( - "Z", - "context", - "(unimplemented) set security context of files and\n \ - directories", - "CONTEXT", - ) - .parse(args) -} - /// Check for unimplemented command line arguments. /// /// Either return the degenerate Ok value, or an Err with string. @@ -221,34 +248,35 @@ fn parse_opts(args: Vec) -> getopts::Matches { /// /// Error datum is a string of the unimplemented argument. /// -fn check_unimplemented(matches: &getopts::Matches) -> Result<(), &str> { - if matches.opt_present("backup") { +/// +fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> { + if matches.is_present(OPT_BACKUP) { Err("--backup") - } else if matches.opt_present("b") { + } else if matches.is_present(OPT_BACKUP_2) { Err("-b") - } else if matches.opt_present("compare") { + } else if matches.is_present(OPT_COMPARE) { Err("--compare, -C") - } else if matches.opt_present("D") { + } else if matches.is_present(OPT_CREATED) { Err("-D") - } else if matches.opt_present("group") { + } else if matches.is_present(OPT_GROUP) { Err("--group, -g") - } else if matches.opt_present("owner") { + } else if matches.is_present(OPT_OWNER) { Err("--owner, -o") - } else if matches.opt_present("preserve-timestamps") { + } else if matches.is_present(OPT_PRESERVE_TIMESTAMPS) { Err("--preserve-timestamps, -p") - } else if matches.opt_present("strip") { + } else if matches.is_present(OPT_STRIP) { Err("--strip, -s") - } else if matches.opt_present("strip-program") { + } else if matches.is_present(OPT_STRIP_PROGRAM) { Err("--strip-program") - } else if matches.opt_present("suffix") { + } else if matches.is_present(OPT_SUFFIX) { Err("--suffix, -S") - } else if matches.opt_present("target-directory") { + } else if matches.is_present(OPT_TARGET_DIRECTORY) { Err("--target-directory, -t") - } else if matches.opt_present("no-target-directory") { + } else if matches.is_present(OPT_NO_TARGET_DIRECTORY) { Err("--no-target-directory, -T") - } else if matches.opt_present("preserve-context") { + } else if matches.is_present(OPT_PRESERVE_CONTEXT) { Err("--preserve-context, -P") - } else if matches.opt_present("context") { + } else if matches.is_present(OPT_CONTEXT) { Err("--context, -Z") } else { Ok(()) @@ -263,8 +291,8 @@ fn check_unimplemented(matches: &getopts::Matches) -> Result<(), &str> { /// /// In event of failure, returns an integer intended as a program return code. /// -fn behavior(matches: &getopts::Matches) -> Result { - let main_function = if matches.opt_present("directory") { +fn behavior(matches: &ArgMatches) -> Result { + let main_function = if matches.is_present("directory") { MainFunction::Directory } else { MainFunction::Standard @@ -272,8 +300,8 @@ fn behavior(matches: &getopts::Matches) -> Result { let considering_dir: bool = MainFunction::Directory == main_function; - let specified_mode: Option = if matches.opt_present("mode") { - match matches.opt_str("mode") { + let specified_mode: Option = if matches.is_present(OPT_MODE) { + match matches.value_of(OPT_MODE) { Some(x) => match mode::parse(&x[..], considering_dir) { Ok(y) => Some(y), Err(err) => { @@ -285,7 +313,7 @@ fn behavior(matches: &getopts::Matches) -> Result { show_error!( "option '--mode' requires an argument\n \ Try '{} --help' for more information.", - NAME + executable!() ); return Err(1); } @@ -294,27 +322,27 @@ fn behavior(matches: &getopts::Matches) -> Result { None }; - let backup_suffix = if matches.opt_present("suffix") { - match matches.opt_str("suffix") { + let backup_suffix = if matches.is_present(OPT_SUFFIX) { + match matches.value_of(OPT_SUFFIX) { Some(x) => x, None => { show_error!( "option '--suffix' requires an argument\n\ Try '{} --help' for more information.", - NAME + executable!() ); return Err(1); } } } else { - "~".to_owned() + "~" }; Ok(Behavior { main_function, specified_mode, - suffix: backup_suffix, - verbose: matches.opt_present("v"), + suffix: backup_suffix.to_string(), + verbose: matches.is_present("v"), }) } @@ -325,15 +353,15 @@ fn behavior(matches: &getopts::Matches) -> Result { /// /// Returns an integer intended as a program return code. /// -fn directory(paths: &[PathBuf], b: Behavior) -> i32 { +fn directory(paths: Vec, b: Behavior) -> i32 { if paths.is_empty() { - println!("{} with -d requires at least one argument.", NAME); + println!("{} with -d requires at least one argument.", executable!()); 1 } else { let mut all_successful = true; for directory in paths.iter() { - let path = directory.as_path(); + let path = Path::new(directory); if path.exists() { show_info!("cannot create directory '{}': File exists", path.display()); @@ -371,18 +399,21 @@ fn is_new_file_path(path: &Path) -> bool { /// /// Returns an integer intended as a program return code. /// -fn standard(paths: &[PathBuf], b: Behavior) -> i32 { +fn standard(paths: Vec, b: Behavior) -> i32 { if paths.len() < 2 { - println!("{} requires at least 2 arguments.", NAME); + println!("{} requires at least 2 arguments.", executable!()); 1 } else { - let sources = &paths[0..paths.len() - 1]; - let target = &paths[paths.len() - 1]; + let sources = &paths[0..paths.len() - 1] + .iter() + .map(PathBuf::from) + .collect::>(); + let target = Path::new(paths.last().unwrap()); if (target.is_file() || is_new_file_path(target)) && sources.len() == 1 { - copy_file_to_file(&sources[0], target, &b) + copy_file_to_file(&sources[0], &target.to_path_buf(), &b) } else { - copy_files_into_dir(sources, target, &b) + copy_files_into_dir(sources, &target.to_path_buf(), &b) } } } diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index e9e70bd93..3fea04187 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -10,7 +10,7 @@ fn test_install_help() { .succeeds() .no_stderr() .stdout - .contains("Options:")); + .contains("FLAGS:")); } #[test]