From c5d7cbda32f443964559c3e66f1c3360b561af4c Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 16:59:06 +0200 Subject: [PATCH 01/25] timeout: handle arguments for the command to run To prevent clap from parsing flags for the command to run as flags for timeout, remove the "args" positional argument, but allow to pass flags via the "command" positional arg. --- src/uu/timeout/src/timeout.rs | 28 ++++++++++------------------ tests/by-util/test_timeout.rs | 8 ++++++++ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index afe560ee5..ea9a0dc65 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -37,7 +37,6 @@ pub mod options { // Positional args. pub static DURATION: &str = "duration"; pub static COMMAND: &str = "command"; - pub static ARGS: &str = "args"; } struct Config { @@ -47,8 +46,7 @@ struct Config { duration: Duration, preserve_status: bool, - command: String, - command_args: Vec, + command: Vec, } impl Config { @@ -77,12 +75,11 @@ impl Config { let preserve_status: bool = options.is_present(options::PRESERVE_STATUS); let foreground = options.is_present(options::FOREGROUND); - let command: String = options.value_of(options::COMMAND).unwrap().to_string(); - - let command_args: Vec = match options.values_of(options::ARGS) { - Some(values) => values.map(|x| x.to_owned()).collect(), - None => vec![], - }; + let command = options + .values_of(options::COMMAND) + .unwrap() + .map(String::from) + .collect::>(); Config { foreground, @@ -91,7 +88,6 @@ impl Config { duration, preserve_status, command, - command_args, } } } @@ -137,9 +133,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(options::COMMAND) .index(2) .required(true) - ) - .arg( - Arg::with_name(options::ARGS).multiple(true) + .multiple(true) ) .setting(AppSettings::TrailingVarArg); @@ -148,7 +142,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let config = Config::from(matches); timeout( &config.command, - &config.command_args, config.duration, config.signal, config.kill_after, @@ -160,8 +153,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { /// TODO: Improve exit codes, and make them consistent with the GNU Coreutils exit codes. fn timeout( - cmdname: &str, - args: &[String], + cmd: &[String], duration: Duration, signal: usize, kill_after: Duration, @@ -171,8 +163,8 @@ fn timeout( if !foreground { unsafe { libc::setpgid(0, 0) }; } - let mut process = match Command::new(cmdname) - .args(args) + let mut process = match Command::new(&cmd[0]) + .args(&cmd[1..]) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 28273e00f..2346a17aa 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -9,3 +9,11 @@ fn test_subcommand_return_code() { new_ucmd!().arg("1").arg("false").run().status_code(1); } + +#[test] +fn test_command_with_args() { + new_ucmd!() + .args(&["1700", "echo", "-n", "abcd"]) + .succeeds() + .stdout_only("abcd"); +} From ed646090c209fe4376d20439ecf6279b6ee26131 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 17:00:12 +0200 Subject: [PATCH 02/25] timeout: fix usage string --- src/uu/timeout/src/timeout.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index ea9a0dc65..3a35c351f 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -23,7 +23,7 @@ use uucore::InvalidEncodingHandling; static ABOUT: &str = "Start COMMAND, and kill it if still running after DURATION."; fn get_usage() -> String { - format!("{0} [OPTION]... [FILE]...", executable!()) + format!("{0} [OPTION] DURATION COMMAND...", executable!()) } const ERR_EXIT_STATUS: i32 = 125; From ceb5a2998c18215bad1684d337da3220cb8201d0 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 18:28:37 +0200 Subject: [PATCH 03/25] core: add EXIT signal EXIT is supported by GNU (see https://github.com/coreutils/gnulib/blob/993ca832d232c33da1d2bb07e91acd6d301ebea0/lib/sig2str.c#L258), so we have to support it too to pass GNU tests. --- src/uucore/src/lib/features/signals.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index d22fa1791..2e6069cb7 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -29,7 +29,11 @@ Linux Programmer's Manual */ #[cfg(target_os = "linux")] -pub static ALL_SIGNALS: [Signal<'static>; 31] = [ +pub static ALL_SIGNALS: [Signal<'static>; 32] = [ + Signal { + name: "EXIT", + value: 0, + }, Signal { name: "HUP", value: 1, @@ -198,7 +202,11 @@ No Name Default Action Description */ #[cfg(any(target_vendor = "apple", target_os = "freebsd"))] -pub static ALL_SIGNALS: [Signal<'static>; 31] = [ +pub static ALL_SIGNALS: [Signal<'static>; 32] = [ + Signal { + name: "EXIT", + value: 0, + }, Signal { name: "HUP", value: 1, From b0b937dc3e2a81c95b0d621f726fd9b39673dfa6 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 18:29:06 +0200 Subject: [PATCH 04/25] core: add signal name lookup by value --- src/uucore/src/lib/features/signals.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 2e6069cb7..3c52a9158 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -349,6 +349,13 @@ pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { .map(|s| s.value) } +pub fn signal_name_by_value(signal_value: usize) -> Option<&'static str> { + ALL_SIGNALS + .iter() + .find(|signal| signal.value == signal_value) + .map(|signal| signal.name) +} + #[inline(always)] pub fn is_signal(num: usize) -> bool { // Named signals start at 1 @@ -358,7 +365,7 @@ pub fn is_signal(num: usize) -> bool { #[test] fn signals_all_contiguous() { for (i, signal) in ALL_SIGNALS.iter().enumerate() { - assert_eq!(signal.value, i + 1); + assert_eq!(signal.value, i); } } @@ -396,3 +403,10 @@ fn signal_by_long_name() { ); } } + +#[test] +fn name() { + for signal in &ALL_SIGNALS { + assert_eq!(signal_name_by_value(signal.value), Some(signal.name)); + } +} From 8e0ed2d20e03b56a7ade4f41ff3e1ab71bcb5e41 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 18:34:05 +0200 Subject: [PATCH 05/25] timeout: support --verbose --- src/uu/timeout/src/timeout.rs | 24 +++++++++++++++++++++++- tests/by-util/test_timeout.rs | 14 ++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 3a35c351f..3c82c4be2 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -17,7 +17,7 @@ use std::io::ErrorKind; use std::process::{Command, Stdio}; use std::time::Duration; use uucore::process::ChildExt; -use uucore::signals::signal_by_name_or_value; +use uucore::signals::{signal_by_name_or_value, signal_name_by_value}; use uucore::InvalidEncodingHandling; static ABOUT: &str = "Start COMMAND, and kill it if still running after DURATION."; @@ -33,6 +33,7 @@ pub mod options { pub static KILL_AFTER: &str = "kill-after"; pub static SIGNAL: &str = "signal"; pub static PRESERVE_STATUS: &str = "preserve-status"; + pub static VERBOSE: &str = "verbose"; // Positional args. pub static DURATION: &str = "duration"; @@ -45,6 +46,7 @@ struct Config { signal: usize, duration: Duration, preserve_status: bool, + verbose: bool, command: Vec, } @@ -74,6 +76,7 @@ impl Config { let preserve_status: bool = options.is_present(options::PRESERVE_STATUS); let foreground = options.is_present(options::FOREGROUND); + let verbose = options.is_present(options::VERBOSE); let command = options .values_of(options::COMMAND) @@ -88,6 +91,7 @@ impl Config { duration, preserve_status, command, + verbose, } } } @@ -124,6 +128,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("specify the signal to be sent on timeout; SIGNAL may be a name like 'HUP' or a number; see 'kill -l' for a list of signals") .takes_value(true) ) + .arg( + Arg::with_name(options::VERBOSE) + .short("v") + .long(options::VERBOSE) + .help("diagnose to stderr any signal sent upon timeout") + ) .arg( Arg::with_name(options::DURATION) .index(1) @@ -147,6 +157,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.kill_after, config.foreground, config.preserve_status, + config.verbose, ) } @@ -159,6 +170,7 @@ fn timeout( kill_after: Duration, foreground: bool, preserve_status: bool, + verbose: bool, ) -> i32 { if !foreground { unsafe { libc::setpgid(0, 0) }; @@ -185,6 +197,13 @@ fn timeout( match process.wait_or_timeout(duration) { Ok(Some(status)) => status.code().unwrap_or_else(|| status.signal().unwrap()), Ok(None) => { + if verbose { + show_error!( + "sending signal {} to command '{}'", + signal_name_by_value(signal).unwrap(), + cmd[0] + ); + } return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); match process.wait_or_timeout(kill_after) { Ok(Some(status)) => { @@ -199,6 +218,9 @@ fn timeout( // XXX: this may not be right return 124; } + if verbose { + show_error!("sending signal KILL to command '{}'", cmd[0]); + } return_if_err!( ERR_EXIT_STATUS, process diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 2346a17aa..4d2451c7e 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -17,3 +17,17 @@ fn test_command_with_args() { .succeeds() .stdout_only("abcd"); } + +#[test] +fn test_verbose() { + for &verbose_flag in &["-v", "--verbose"] { + new_ucmd!() + .args(&[verbose_flag, ".1", "sleep", "10"]) + .fails() + .stderr_only("timeout: sending signal TERM to command 'sleep'"); + new_ucmd!() + .args(&[verbose_flag, "-s0", "-k.1", ".1", "sleep", "10"]) + .fails() + .stderr_only("timeout: sending signal EXIT to command 'sleep'\ntimeout: sending signal KILL to command 'sleep'"); + } +} From 0f9bc8e9746809e1fe9b59e656e62f395e347610 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 18:51:41 +0200 Subject: [PATCH 06/25] timeout: disable timeout if it is set to zero --- src/uu/timeout/src/timeout.rs | 4 ---- src/uucore/src/lib/features/process.rs | 6 ++++++ tests/by-util/test_timeout.rs | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 3c82c4be2..b671d9d3e 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -214,10 +214,6 @@ fn timeout( } } Ok(None) => { - if kill_after == Duration::new(0, 0) { - // XXX: this may not be right - return 124; - } if verbose { show_error!("sending signal KILL to command '{}'", cmd[0]); } diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 975123cf7..cda41bb4f 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -93,6 +93,7 @@ pub trait ChildExt { fn send_signal(&mut self, signal: usize) -> io::Result<()>; /// Wait for a process to finish or return after the specified duration. + /// A `timeout` of zero disables the timeout. fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result>; } @@ -106,6 +107,11 @@ impl ChildExt for Child { } fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result> { + if timeout == Duration::from_micros(0) { + return self + .wait() + .map(|status| Some(ExitStatus::from_std_status(status))); + } // .try_wait() doesn't drop stdin, so we do it manually drop(self.stdin.take()); diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 4d2451c7e..9be29065a 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -31,3 +31,17 @@ fn test_verbose() { .stderr_only("timeout: sending signal EXIT to command 'sleep'\ntimeout: sending signal KILL to command 'sleep'"); } } + +#[test] +fn test_zero_timeout() { + new_ucmd!() + .args(&["-v", "0", "sleep", ".1"]) + .succeeds() + .no_stderr() + .no_stdout(); + new_ucmd!() + .args(&["-v", "0", "-s0", "-k0", "sleep", ".1"]) + .succeeds() + .no_stderr() + .no_stdout(); +} From b4efd5a749592758bf0d813f6bd2668b813a1e66 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 20:03:33 +0200 Subject: [PATCH 07/25] timeout: disable pre-existing SIGCHLD handlers Needed to make a GNU test pass --- Cargo.lock | 3 +++ src/uu/timeout/Cargo.toml | 1 + src/uu/timeout/src/timeout.rs | 14 +++++++++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c2c3a6242..6e9c22049 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,7 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +version = 3 + [[package]] name = "Inflector" version = "0.11.4" @@ -2606,6 +2608,7 @@ version = "0.0.6" dependencies = [ "clap", "libc", + "nix 0.20.0", "uucore", "uucore_procs", ] diff --git a/src/uu/timeout/Cargo.toml b/src/uu/timeout/Cargo.toml index d16559858..a09342c0a 100644 --- a/src/uu/timeout/Cargo.toml +++ b/src/uu/timeout/Cargo.toml @@ -17,6 +17,7 @@ path = "src/timeout.rs" [dependencies] clap = "2.33" libc = "0.2.42" +nix = "0.20.0" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["parse_time", "process", "signals"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index b671d9d3e..eabf0192a 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -5,7 +5,7 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// spell-checker:ignore (ToDO) tstr sigstr cmdname setpgid +// spell-checker:ignore (ToDO) tstr sigstr cmdname setpgid sigchld #[macro_use] extern crate uucore; @@ -161,6 +161,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ) } +/// Remove pre-existing SIGCHLD handlers that would make waiting for the child's exit code fail. +fn unblock_sigchld() { + unsafe { + nix::sys::signal::signal( + nix::sys::signal::Signal::SIGCHLD, + nix::sys::signal::SigHandler::SigDfl, + ) + .unwrap(); + } +} + /// TODO: Improve exit codes, and make them consistent with the GNU Coreutils exit codes. fn timeout( @@ -194,6 +205,7 @@ fn timeout( } } }; + unblock_sigchld(); match process.wait_or_timeout(duration) { Ok(Some(status)) => status.code().unwrap_or_else(|| status.signal().unwrap()), Ok(None) => { From f90975115538ba4d2043113f4cecf39bd17af94c Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 11 Jun 2021 20:44:25 +0200 Subject: [PATCH 08/25] timeout: don't kill the process if -k is not set `timeout` used to set the timeout to 0 when -k was not set. This collided with the behavior of 0 timeouts, which disable the timeout. When -k is not set the process should not be killed. --- src/uu/timeout/src/timeout.rs | 52 +++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index eabf0192a..bc92157ca 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -42,7 +42,7 @@ pub mod options { struct Config { foreground: bool, - kill_after: Duration, + kill_after: Option, signal: usize, duration: Duration, preserve_status: bool, @@ -66,10 +66,9 @@ impl Config { _ => uucore::signals::signal_by_name_or_value("TERM").unwrap(), }; - let kill_after: Duration = match options.value_of(options::KILL_AFTER) { - Some(time) => uucore::parse_time::from_str(time).unwrap(), - None => Duration::new(0, 0), - }; + let kill_after = options + .value_of(options::KILL_AFTER) + .map(|time| uucore::parse_time::from_str(time).unwrap()); let duration: Duration = uucore::parse_time::from_str(options.value_of(options::DURATION).unwrap()).unwrap(); @@ -178,7 +177,7 @@ fn timeout( cmd: &[String], duration: Duration, signal: usize, - kill_after: Duration, + kill_after: Option, foreground: bool, preserve_status: bool, verbose: bool, @@ -217,27 +216,32 @@ fn timeout( ); } return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); - match process.wait_or_timeout(kill_after) { - Ok(Some(status)) => { - if preserve_status { - status.code().unwrap_or_else(|| status.signal().unwrap()) - } else { - 124 + if let Some(kill_after) = kill_after { + match process.wait_or_timeout(kill_after) { + Ok(Some(status)) => { + if preserve_status { + status.code().unwrap_or_else(|| status.signal().unwrap()) + } else { + 124 + } } - } - Ok(None) => { - if verbose { - show_error!("sending signal KILL to command '{}'", cmd[0]); + Ok(None) => { + if verbose { + show_error!("sending signal KILL to command '{}'", cmd[0]); + } + return_if_err!( + ERR_EXIT_STATUS, + process.send_signal( + uucore::signals::signal_by_name_or_value("KILL").unwrap() + ) + ); + return_if_err!(ERR_EXIT_STATUS, process.wait()); + 137 } - return_if_err!( - ERR_EXIT_STATUS, - process - .send_signal(uucore::signals::signal_by_name_or_value("KILL").unwrap()) - ); - return_if_err!(ERR_EXIT_STATUS, process.wait()); - 137 + Err(_) => 124, } - Err(_) => 124, + } else { + 124 } } Err(_) => { From cc0df6ea43621856d884248e72ee4e088ef49c0f Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 22:21:45 +0200 Subject: [PATCH 09/25] sort: move options to the `options` module Be more consistent with other utilities --- src/uu/sort/src/sort.rs | 287 ++++++++++++++++++++++------------------ 1 file changed, 156 insertions(+), 131 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 53619d0d6..11fd05c4e 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -45,8 +45,8 @@ use std::path::PathBuf; use unicode_width::UnicodeWidthStr; use uucore::InvalidEncodingHandling; -static NAME: &str = "sort"; -static ABOUT: &str = "Display sorted concatenation of all FILE(s)."; +const NAME: &str = "sort"; +const ABOUT: &str = "Display sorted concatenation of all FILE(s)."; const LONG_HELP_KEYS: &str = "The key format is FIELD[.CHAR][OPTIONS][,FIELD[.CHAR]][OPTIONS]. @@ -58,49 +58,53 @@ If CHAR is set 0, it means the end of the field. CHAR defaults to 1 for the star Valid options are: MbdfhnRrV. They override the global options for this key."; -static OPT_HUMAN_NUMERIC_SORT: &str = "human-numeric-sort"; -static OPT_MONTH_SORT: &str = "month-sort"; -static OPT_NUMERIC_SORT: &str = "numeric-sort"; -static OPT_GENERAL_NUMERIC_SORT: &str = "general-numeric-sort"; -static OPT_VERSION_SORT: &str = "version-sort"; +mod options { + pub mod modes { + pub const SORT: &str = "sort"; -static OPT_SORT: &str = "sort"; + pub const HUMAN_NUMERIC: &str = "human-numeric-sort"; + pub const MONTH: &str = "month-sort"; + pub const NUMERIC: &str = "numeric-sort"; + pub const GENERAL_NUMERIC: &str = "general-numeric-sort"; + pub const VERSION: &str = "version-sort"; + pub const RANDOM: &str = "random-sort"; -static ALL_SORT_MODES: &[&str] = &[ - OPT_GENERAL_NUMERIC_SORT, - OPT_HUMAN_NUMERIC_SORT, - OPT_MONTH_SORT, - OPT_NUMERIC_SORT, - OPT_VERSION_SORT, - OPT_RANDOM, -]; + pub const ALL_SORT_MODES: [&str; 6] = [ + GENERAL_NUMERIC, + HUMAN_NUMERIC, + MONTH, + NUMERIC, + VERSION, + RANDOM, + ]; + } -static OPT_DICTIONARY_ORDER: &str = "dictionary-order"; -static OPT_MERGE: &str = "merge"; -static OPT_CHECK: &str = "check"; -static OPT_CHECK_SILENT: &str = "check-silent"; -static OPT_DEBUG: &str = "debug"; -static OPT_IGNORE_CASE: &str = "ignore-case"; -static OPT_IGNORE_BLANKS: &str = "ignore-blanks"; -static OPT_IGNORE_NONPRINTING: &str = "ignore-nonprinting"; -static OPT_OUTPUT: &str = "output"; -static OPT_REVERSE: &str = "reverse"; -static OPT_STABLE: &str = "stable"; -static OPT_UNIQUE: &str = "unique"; -static OPT_KEY: &str = "key"; -static OPT_SEPARATOR: &str = "field-separator"; -static OPT_RANDOM: &str = "random-sort"; -static OPT_ZERO_TERMINATED: &str = "zero-terminated"; -static OPT_PARALLEL: &str = "parallel"; -static OPT_FILES0_FROM: &str = "files0-from"; -static OPT_BUF_SIZE: &str = "buffer-size"; -static OPT_TMP_DIR: &str = "temporary-directory"; -static OPT_COMPRESS_PROG: &str = "compress-program"; -static OPT_BATCH_SIZE: &str = "batch-size"; + pub const DICTIONARY_ORDER: &str = "dictionary-order"; + pub const MERGE: &str = "merge"; + pub const CHECK: &str = "check"; + pub const CHECK_SILENT: &str = "check-silent"; + pub const DEBUG: &str = "debug"; + pub const IGNORE_CASE: &str = "ignore-case"; + pub const IGNORE_BLANKS: &str = "ignore-blanks"; + pub const IGNORE_NONPRINTING: &str = "ignore-nonprinting"; + pub const OUTPUT: &str = "output"; + pub const REVERSE: &str = "reverse"; + pub const STABLE: &str = "stable"; + pub const UNIQUE: &str = "unique"; + pub const KEY: &str = "key"; + pub const SEPARATOR: &str = "field-separator"; + pub const ZERO_TERMINATED: &str = "zero-terminated"; + pub const PARALLEL: &str = "parallel"; + pub const FILES0_FROM: &str = "files0-from"; + pub const BUF_SIZE: &str = "buffer-size"; + pub const TMP_DIR: &str = "temporary-directory"; + pub const COMPRESS_PROG: &str = "compress-program"; + pub const BATCH_SIZE: &str = "batch-size"; -static ARG_FILES: &str = "files"; + pub const FILES: &str = "files"; +} -static DECIMAL_PT: char = '.'; +const DECIMAL_PT: char = '.'; const NEGATIVE: char = '-'; const POSITIVE: char = '+'; @@ -108,7 +112,7 @@ const POSITIVE: char = '+'; // Choosing a higher buffer size does not result in performance improvements // (at least not on my machine). TODO: In the future, we should also take the amount of // available memory into consideration, instead of relying on this constant only. -static DEFAULT_BUF_SIZE: usize = 1_000_000_000; // 1 GB +const DEFAULT_BUF_SIZE: usize = 1_000_000_000; // 1 GB #[derive(Eq, Ord, PartialEq, PartialOrd, Clone, Copy, Debug)] enum SortMode { @@ -889,9 +893,10 @@ With no FILE, or when FILE is -, read standard input.", ) } +/// Creates an `Arg` that conflicts with all other sort modes. fn make_sort_mode_arg<'a, 'b>(mode: &'a str, short: &'b str, help: &'b str) -> Arg<'a, 'b> { let mut arg = Arg::with_name(mode).short(short).long(mode).help(help); - for possible_mode in ALL_SORT_MODES { + for possible_mode in &options::modes::ALL_SORT_MODES { if *possible_mode != mode { arg = arg.conflicts_with(possible_mode); } @@ -911,8 +916,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .about(ABOUT) .usage(&usage[..]) .arg( - Arg::with_name(OPT_SORT) - .long(OPT_SORT) + Arg::with_name(options::modes::SORT) + .long(options::modes::SORT) .takes_value(true) .possible_values( &[ @@ -924,199 +929,213 @@ pub fn uumain(args: impl uucore::Args) -> i32 { "random", ] ) - .conflicts_with_all(ALL_SORT_MODES) + .conflicts_with_all(&options::modes::ALL_SORT_MODES) ) .arg( make_sort_mode_arg( - OPT_HUMAN_NUMERIC_SORT, + options::modes::HUMAN_NUMERIC, "h", "compare according to human readable sizes, eg 1M > 100k" ), ) .arg( make_sort_mode_arg( - OPT_MONTH_SORT, + options::modes::MONTH, "M", "compare according to month name abbreviation" ), ) .arg( make_sort_mode_arg( - OPT_NUMERIC_SORT, + options::modes::NUMERIC, "n", "compare according to string numerical value" ), ) .arg( make_sort_mode_arg( - OPT_GENERAL_NUMERIC_SORT, + options::modes::GENERAL_NUMERIC, "g", "compare according to string general numerical value" ), ) .arg( make_sort_mode_arg( - OPT_VERSION_SORT, + options::modes::VERSION, "V", "Sort by SemVer version number, eg 1.12.2 > 1.1.2", ), ) .arg( make_sort_mode_arg( - OPT_RANDOM, + options::modes::RANDOM, "R", "shuffle in random order", ), ) .arg( - Arg::with_name(OPT_DICTIONARY_ORDER) + Arg::with_name(options::DICTIONARY_ORDER) .short("d") - .long(OPT_DICTIONARY_ORDER) + .long(options::DICTIONARY_ORDER) .help("consider only blanks and alphanumeric characters") - .conflicts_with_all(&[OPT_NUMERIC_SORT, OPT_GENERAL_NUMERIC_SORT, OPT_HUMAN_NUMERIC_SORT, OPT_MONTH_SORT]), + .conflicts_with_all( + &[ + options::modes::NUMERIC, + options::modes::GENERAL_NUMERIC, + options::modes::HUMAN_NUMERIC, + options::modes::MONTH, + ] + ), ) .arg( - Arg::with_name(OPT_MERGE) + Arg::with_name(options::MERGE) .short("m") - .long(OPT_MERGE) + .long(options::MERGE) .help("merge already sorted files; do not sort"), ) .arg( - Arg::with_name(OPT_CHECK) + Arg::with_name(options::CHECK) .short("c") - .long(OPT_CHECK) + .long(options::CHECK) .help("check for sorted input; do not sort"), ) .arg( - Arg::with_name(OPT_CHECK_SILENT) + Arg::with_name(options::CHECK_SILENT) .short("C") - .long(OPT_CHECK_SILENT) + .long(options::CHECK_SILENT) .help("exit successfully if the given file is already sorted, and exit with status 1 otherwise."), ) .arg( - Arg::with_name(OPT_IGNORE_CASE) + Arg::with_name(options::IGNORE_CASE) .short("f") - .long(OPT_IGNORE_CASE) + .long(options::IGNORE_CASE) .help("fold lower case to upper case characters"), ) .arg( - Arg::with_name(OPT_IGNORE_NONPRINTING) + Arg::with_name(options::IGNORE_NONPRINTING) .short("i") - .long(OPT_IGNORE_NONPRINTING) + .long(options::IGNORE_NONPRINTING) .help("ignore nonprinting characters") - .conflicts_with_all(&[OPT_NUMERIC_SORT, OPT_GENERAL_NUMERIC_SORT, OPT_HUMAN_NUMERIC_SORT, OPT_MONTH_SORT]), + .conflicts_with_all( + &[ + options::modes::NUMERIC, + options::modes::GENERAL_NUMERIC, + options::modes::HUMAN_NUMERIC, + options::modes::MONTH + ] + ), ) .arg( - Arg::with_name(OPT_IGNORE_BLANKS) + Arg::with_name(options::IGNORE_BLANKS) .short("b") - .long(OPT_IGNORE_BLANKS) + .long(options::IGNORE_BLANKS) .help("ignore leading blanks when finding sort keys in each line"), ) .arg( - Arg::with_name(OPT_OUTPUT) + Arg::with_name(options::OUTPUT) .short("o") - .long(OPT_OUTPUT) + .long(options::OUTPUT) .help("write output to FILENAME instead of stdout") .takes_value(true) .value_name("FILENAME"), ) .arg( - Arg::with_name(OPT_REVERSE) + Arg::with_name(options::REVERSE) .short("r") - .long(OPT_REVERSE) + .long(options::REVERSE) .help("reverse the output"), ) .arg( - Arg::with_name(OPT_STABLE) + Arg::with_name(options::STABLE) .short("s") - .long(OPT_STABLE) + .long(options::STABLE) .help("stabilize sort by disabling last-resort comparison"), ) .arg( - Arg::with_name(OPT_UNIQUE) + Arg::with_name(options::UNIQUE) .short("u") - .long(OPT_UNIQUE) + .long(options::UNIQUE) .help("output only the first of an equal run"), ) .arg( - Arg::with_name(OPT_KEY) + Arg::with_name(options::KEY) .short("k") - .long(OPT_KEY) + .long(options::KEY) .help("sort by a key") .long_help(LONG_HELP_KEYS) .multiple(true) .takes_value(true), ) .arg( - Arg::with_name(OPT_SEPARATOR) + Arg::with_name(options::SEPARATOR) .short("t") - .long(OPT_SEPARATOR) + .long(options::SEPARATOR) .help("custom separator for -k") .takes_value(true)) .arg( - Arg::with_name(OPT_ZERO_TERMINATED) + Arg::with_name(options::ZERO_TERMINATED) .short("z") - .long(OPT_ZERO_TERMINATED) + .long(options::ZERO_TERMINATED) .help("line delimiter is NUL, not newline"), ) .arg( - Arg::with_name(OPT_PARALLEL) - .long(OPT_PARALLEL) + Arg::with_name(options::PARALLEL) + .long(options::PARALLEL) .help("change the number of threads running concurrently to NUM_THREADS") .takes_value(true) .value_name("NUM_THREADS"), ) .arg( - Arg::with_name(OPT_BUF_SIZE) + Arg::with_name(options::BUF_SIZE) .short("S") - .long(OPT_BUF_SIZE) + .long(options::BUF_SIZE) .help("sets the maximum SIZE of each segment in number of sorted items") .takes_value(true) .value_name("SIZE"), ) .arg( - Arg::with_name(OPT_TMP_DIR) + Arg::with_name(options::TMP_DIR) .short("T") - .long(OPT_TMP_DIR) + .long(options::TMP_DIR) .help("use DIR for temporaries, not $TMPDIR or /tmp") .takes_value(true) .value_name("DIR"), ) .arg( - Arg::with_name(OPT_COMPRESS_PROG) - .long(OPT_COMPRESS_PROG) + Arg::with_name(options::COMPRESS_PROG) + .long(options::COMPRESS_PROG) .help("compress temporary files with PROG, decompress with PROG -d") .long_help("PROG has to take input from stdin and output to stdout") .value_name("PROG") ) .arg( - Arg::with_name(OPT_BATCH_SIZE) - .long(OPT_BATCH_SIZE) + Arg::with_name(options::BATCH_SIZE) + .long(options::BATCH_SIZE) .help("Merge at most N_MERGE inputs at once.") .value_name("N_MERGE") ) .arg( - Arg::with_name(OPT_FILES0_FROM) - .long(OPT_FILES0_FROM) + Arg::with_name(options::FILES0_FROM) + .long(options::FILES0_FROM) .help("read input from the files specified by NUL-terminated NUL_FILES") .takes_value(true) .value_name("NUL_FILES") .multiple(true), ) .arg( - Arg::with_name(OPT_DEBUG) - .long(OPT_DEBUG) + Arg::with_name(options::DEBUG) + .long(options::DEBUG) .help("underline the parts of the line that are actually used for sorting"), ) - .arg(Arg::with_name(ARG_FILES).multiple(true).takes_value(true)) + .arg(Arg::with_name(options::FILES).multiple(true).takes_value(true)) .get_matches_from(args); - settings.debug = matches.is_present(OPT_DEBUG); + settings.debug = matches.is_present(options::DEBUG); // check whether user specified a zero terminated list of files for input, otherwise read files from args - let mut files: Vec = if matches.is_present(OPT_FILES0_FROM) { + let mut files: Vec = if matches.is_present(options::FILES0_FROM) { let files0_from: Vec = matches - .values_of(OPT_FILES0_FROM) + .values_of(options::FILES0_FROM) .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); @@ -1135,80 +1154,86 @@ pub fn uumain(args: impl uucore::Args) -> i32 { files } else { matches - .values_of(ARG_FILES) + .values_of(options::FILES) .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default() }; - settings.mode = if matches.is_present(OPT_HUMAN_NUMERIC_SORT) - || matches.value_of(OPT_SORT) == Some("human-numeric") + settings.mode = if matches.is_present(options::modes::HUMAN_NUMERIC) + || matches.value_of(options::modes::SORT) == Some("human-numeric") { SortMode::HumanNumeric - } else if matches.is_present(OPT_MONTH_SORT) || matches.value_of(OPT_SORT) == Some("month") { + } else if matches.is_present(options::modes::MONTH) + || matches.value_of(options::modes::SORT) == Some("month") + { SortMode::Month - } else if matches.is_present(OPT_GENERAL_NUMERIC_SORT) - || matches.value_of(OPT_SORT) == Some("general-numeric") + } else if matches.is_present(options::modes::GENERAL_NUMERIC) + || matches.value_of(options::modes::SORT) == Some("general-numeric") { SortMode::GeneralNumeric - } else if matches.is_present(OPT_NUMERIC_SORT) || matches.value_of(OPT_SORT) == Some("numeric") + } else if matches.is_present(options::modes::NUMERIC) + || matches.value_of(options::modes::SORT) == Some("numeric") { SortMode::Numeric - } else if matches.is_present(OPT_VERSION_SORT) || matches.value_of(OPT_SORT) == Some("version") + } else if matches.is_present(options::modes::VERSION) + || matches.value_of(options::modes::SORT) == Some("version") { SortMode::Version - } else if matches.is_present(OPT_RANDOM) || matches.value_of(OPT_SORT) == Some("random") { + } else if matches.is_present(options::modes::RANDOM) + || matches.value_of(options::modes::SORT) == Some("random") + { settings.salt = get_rand_string(); SortMode::Random } else { SortMode::Default }; - settings.dictionary_order = matches.is_present(OPT_DICTIONARY_ORDER); - settings.ignore_non_printing = matches.is_present(OPT_IGNORE_NONPRINTING); - if matches.is_present(OPT_PARALLEL) { + settings.dictionary_order = matches.is_present(options::DICTIONARY_ORDER); + settings.ignore_non_printing = matches.is_present(options::IGNORE_NONPRINTING); + if matches.is_present(options::PARALLEL) { // "0" is default - threads = num of cores settings.threads = matches - .value_of(OPT_PARALLEL) + .value_of(options::PARALLEL) .map(String::from) .unwrap_or_else(|| "0".to_string()); env::set_var("RAYON_NUM_THREADS", &settings.threads); } settings.buffer_size = matches - .value_of(OPT_BUF_SIZE) + .value_of(options::BUF_SIZE) .map(GlobalSettings::parse_byte_count) .unwrap_or(DEFAULT_BUF_SIZE); settings.tmp_dir = matches - .value_of(OPT_TMP_DIR) + .value_of(options::TMP_DIR) .map(PathBuf::from) .unwrap_or_else(env::temp_dir); - settings.compress_prog = matches.value_of(OPT_COMPRESS_PROG).map(String::from); + settings.compress_prog = matches.value_of(options::COMPRESS_PROG).map(String::from); - if let Some(n_merge) = matches.value_of(OPT_BATCH_SIZE) { + if let Some(n_merge) = matches.value_of(options::BATCH_SIZE) { settings.merge_batch_size = n_merge .parse() .unwrap_or_else(|_| crash!(2, "invalid --batch-size argument '{}'", n_merge)); } - settings.zero_terminated = matches.is_present(OPT_ZERO_TERMINATED); - settings.merge = matches.is_present(OPT_MERGE); + settings.zero_terminated = matches.is_present(options::ZERO_TERMINATED); + settings.merge = matches.is_present(options::MERGE); - settings.check = matches.is_present(OPT_CHECK); - if matches.is_present(OPT_CHECK_SILENT) { - settings.check_silent = matches.is_present(OPT_CHECK_SILENT); + settings.check = matches.is_present(options::CHECK); + if matches.is_present(options::CHECK_SILENT) { + settings.check_silent = matches.is_present(options::CHECK_SILENT); settings.check = true; }; - settings.ignore_case = matches.is_present(OPT_IGNORE_CASE); + settings.ignore_case = matches.is_present(options::IGNORE_CASE); - settings.ignore_blanks = matches.is_present(OPT_IGNORE_BLANKS); + settings.ignore_blanks = matches.is_present(options::IGNORE_BLANKS); - settings.output_file = matches.value_of(OPT_OUTPUT).map(String::from); - settings.reverse = matches.is_present(OPT_REVERSE); - settings.stable = matches.is_present(OPT_STABLE); - settings.unique = matches.is_present(OPT_UNIQUE); + settings.output_file = matches.value_of(options::OUTPUT).map(String::from); + settings.reverse = matches.is_present(options::REVERSE); + settings.stable = matches.is_present(options::STABLE); + settings.unique = matches.is_present(options::UNIQUE); if files.is_empty() { /* if no file, default to stdin */ @@ -1217,7 +1242,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { crash!(1, "extra operand `{}' not allowed with -c", files[1]) } - if let Some(arg) = matches.args.get(OPT_SEPARATOR) { + if let Some(arg) = matches.args.get(options::SEPARATOR) { let separator = arg.vals[0].to_string_lossy(); let separator = separator; if separator.len() != 1 { @@ -1226,15 +1251,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.separator = Some(separator.chars().next().unwrap()) } - if matches.is_present(OPT_KEY) { - for key in &matches.args[OPT_KEY].vals { + if matches.is_present(options::KEY) { + for key in &matches.args[options::KEY].vals { settings .selectors .push(FieldSelector::parse(&key.to_string_lossy(), &settings)); } } - if !matches.is_present(OPT_KEY) { + if !matches.is_present(options::KEY) { // add a default selector matching the whole line let key_settings = KeySettings::from(&settings); settings.selectors.push( From fb035aa049d241ce88291944ba6cdee5a96c7a8c Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 22:38:43 +0200 Subject: [PATCH 10/25] sort: allow --check= syntax * --check=silent and --check=quiet, which are equivalent to -C. * --check=diagnose-first, which is the same as --check We also allow -c=, which confuses GNU sort. --- src/uu/sort/src/sort.rs | 37 ++++++++++++++++++++++++++++--------- tests/by-util/test_sort.rs | 34 +++++++++++++++++++--------------- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 11fd05c4e..1156a9437 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -79,10 +79,16 @@ mod options { ]; } + pub mod check { + pub const CHECK: &str = "check"; + pub const CHECK_SILENT: &str = "check-silent"; + pub const SILENT: &str = "silent"; + pub const QUIET: &str = "quiet"; + pub const DIAGNOSE_FIRST: &str = "diagnose-first"; + } + pub const DICTIONARY_ORDER: &str = "dictionary-order"; pub const MERGE: &str = "merge"; - pub const CHECK: &str = "check"; - pub const CHECK_SILENT: &str = "check-silent"; pub const DEBUG: &str = "debug"; pub const IGNORE_CASE: &str = "ignore-case"; pub const IGNORE_BLANKS: &str = "ignore-blanks"; @@ -994,15 +1000,23 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("merge already sorted files; do not sort"), ) .arg( - Arg::with_name(options::CHECK) + Arg::with_name(options::check::CHECK) .short("c") - .long(options::CHECK) + .long(options::check::CHECK) + .takes_value(true) + .require_equals(true) + .min_values(0) + .possible_values(&[ + options::check::SILENT, + options::check::QUIET, + options::check::DIAGNOSE_FIRST, + ]) .help("check for sorted input; do not sort"), ) .arg( - Arg::with_name(options::CHECK_SILENT) + Arg::with_name(options::check::CHECK_SILENT) .short("C") - .long(options::CHECK_SILENT) + .long(options::check::CHECK_SILENT) .help("exit successfully if the given file is already sorted, and exit with status 1 otherwise."), ) .arg( @@ -1220,9 +1234,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.zero_terminated = matches.is_present(options::ZERO_TERMINATED); settings.merge = matches.is_present(options::MERGE); - settings.check = matches.is_present(options::CHECK); - if matches.is_present(options::CHECK_SILENT) { - settings.check_silent = matches.is_present(options::CHECK_SILENT); + settings.check = matches.is_present(options::check::CHECK); + if matches.is_present(options::check::CHECK_SILENT) + || matches!( + matches.value_of(options::check::CHECK), + Some(options::check::SILENT) | Some(options::check::QUIET) + ) + { + settings.check_silent = true; settings.check = true; }; diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 7a0143b43..1624c2caf 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -717,26 +717,30 @@ fn test_pipe() { #[test] fn test_check() { - new_ucmd!() - .arg("-c") - .arg("check_fail.txt") - .fails() - .stdout_is("sort: check_fail.txt:6: disorder: 5\n"); + for diagnose_arg in &["-c", "--check", "--check=diagnose-first"] { + new_ucmd!() + .arg(diagnose_arg) + .arg("check_fail.txt") + .fails() + .stdout_is("sort: check_fail.txt:6: disorder: 5\n"); - new_ucmd!() - .arg("-c") - .arg("multiple_files.expected") - .succeeds() - .stdout_is(""); + new_ucmd!() + .arg(diagnose_arg) + .arg("multiple_files.expected") + .succeeds() + .stdout_is(""); + } } #[test] fn test_check_silent() { - new_ucmd!() - .arg("-C") - .arg("check_fail.txt") - .fails() - .stdout_is(""); + for silent_arg in &["-C", "--check=silent", "--check=quiet"] { + new_ucmd!() + .arg(silent_arg) + .arg("check_fail.txt") + .fails() + .stdout_is(""); + } } #[test] From b8d44112918421a0cc2c5b923d2d5eb03ce090b8 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 22:49:17 +0200 Subject: [PATCH 11/25] sort: fix ignore-leading-blanks long option --- src/uu/sort/src/sort.rs | 24 +++++++++++++----------- tests/by-util/test_sort.rs | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 1156a9437..934a4da49 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -91,7 +91,7 @@ mod options { pub const MERGE: &str = "merge"; pub const DEBUG: &str = "debug"; pub const IGNORE_CASE: &str = "ignore-case"; - pub const IGNORE_BLANKS: &str = "ignore-blanks"; + pub const IGNORE_LEADING_BLANKS: &str = "ignore-leading-blanks"; pub const IGNORE_NONPRINTING: &str = "ignore-nonprinting"; pub const OUTPUT: &str = "output"; pub const REVERSE: &str = "reverse"; @@ -149,7 +149,7 @@ impl SortMode { pub struct GlobalSettings { mode: SortMode, debug: bool, - ignore_blanks: bool, + ignore_leading_blanks: bool, ignore_case: bool, dictionary_order: bool, ignore_non_printing: bool, @@ -219,7 +219,7 @@ impl Default for GlobalSettings { GlobalSettings { mode: SortMode::Default, debug: false, - ignore_blanks: false, + ignore_leading_blanks: false, ignore_case: false, dictionary_order: false, ignore_non_printing: false, @@ -310,7 +310,7 @@ impl From<&GlobalSettings> for KeySettings { fn from(settings: &GlobalSettings) -> Self { Self { mode: settings.mode, - ignore_blanks: settings.ignore_blanks, + ignore_blanks: settings.ignore_leading_blanks, ignore_case: settings.ignore_case, ignore_non_printing: settings.ignore_non_printing, reverse: settings.reverse, @@ -517,7 +517,7 @@ impl<'a> Line<'a> { && !settings.stable && !settings.unique && (settings.dictionary_order - || settings.ignore_blanks + || settings.ignore_leading_blanks || settings.ignore_case || settings.ignore_non_printing || settings.mode != SortMode::Default @@ -681,9 +681,11 @@ impl FieldSelector { // This would be ideal for a try block, I think. In the meantime this closure allows // to use the `?` operator here. Self::new( - KeyPosition::new(from, 1, global_settings.ignore_blanks)?, - to.map(|(to, _)| KeyPosition::new(to, 0, global_settings.ignore_blanks)) - .transpose()?, + KeyPosition::new(from, 1, global_settings.ignore_leading_blanks)?, + to.map(|(to, _)| { + KeyPosition::new(to, 0, global_settings.ignore_leading_blanks) + }) + .transpose()?, KeySettings::from(global_settings), ) })() @@ -1040,9 +1042,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ), ) .arg( - Arg::with_name(options::IGNORE_BLANKS) + Arg::with_name(options::IGNORE_LEADING_BLANKS) .short("b") - .long(options::IGNORE_BLANKS) + .long(options::IGNORE_LEADING_BLANKS) .help("ignore leading blanks when finding sort keys in each line"), ) .arg( @@ -1247,7 +1249,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.ignore_case = matches.is_present(options::IGNORE_CASE); - settings.ignore_blanks = matches.is_present(options::IGNORE_BLANKS); + settings.ignore_leading_blanks = matches.is_present(options::IGNORE_LEADING_BLANKS); settings.output_file = matches.value_of(options::OUTPUT).map(String::from); settings.reverse = matches.is_present(options::REVERSE); diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 1624c2caf..c1d1a85b5 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -794,7 +794,7 @@ fn test_nonexistent_file() { #[test] fn test_blanks() { - test_helper("blanks", &["-b", "--ignore-blanks"]); + test_helper("blanks", &["-b", "--ignore-leading-blanks"]); } #[test] From a390383d2d71f51a4512abb641e61655ac280484 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 12 Jun 2021 13:01:42 +0200 Subject: [PATCH 12/25] core: represent signal values by their index --- src/uucore/src/lib/features/signals.rs | 326 ++----------------------- 1 file changed, 23 insertions(+), 303 deletions(-) diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 3c52a9158..e6d2e7763 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -10,11 +10,6 @@ pub static DEFAULT_SIGNAL: usize = 15; -pub struct Signal<'a> { - pub name: &'a str, - pub value: usize, -} - /* Linux Programmer's Manual @@ -29,135 +24,10 @@ Linux Programmer's Manual */ #[cfg(target_os = "linux")] -pub static ALL_SIGNALS: [Signal<'static>; 32] = [ - Signal { - name: "EXIT", - value: 0, - }, - Signal { - name: "HUP", - value: 1, - }, - Signal { - name: "INT", - value: 2, - }, - Signal { - name: "QUIT", - value: 3, - }, - Signal { - name: "ILL", - value: 4, - }, - Signal { - name: "TRAP", - value: 5, - }, - Signal { - name: "ABRT", - value: 6, - }, - Signal { - name: "BUS", - value: 7, - }, - Signal { - name: "FPE", - value: 8, - }, - Signal { - name: "KILL", - value: 9, - }, - Signal { - name: "USR1", - value: 10, - }, - Signal { - name: "SEGV", - value: 11, - }, - Signal { - name: "USR2", - value: 12, - }, - Signal { - name: "PIPE", - value: 13, - }, - Signal { - name: "ALRM", - value: 14, - }, - Signal { - name: "TERM", - value: 15, - }, - Signal { - name: "STKFLT", - value: 16, - }, - Signal { - name: "CHLD", - value: 17, - }, - Signal { - name: "CONT", - value: 18, - }, - Signal { - name: "STOP", - value: 19, - }, - Signal { - name: "TSTP", - value: 20, - }, - Signal { - name: "TTIN", - value: 21, - }, - Signal { - name: "TTOU", - value: 22, - }, - Signal { - name: "URG", - value: 23, - }, - Signal { - name: "XCPU", - value: 24, - }, - Signal { - name: "XFSZ", - value: 25, - }, - Signal { - name: "VTALRM", - value: 26, - }, - Signal { - name: "PROF", - value: 27, - }, - Signal { - name: "WINCH", - value: 28, - }, - Signal { - name: "POLL", - value: 29, - }, - Signal { - name: "PWR", - value: 30, - }, - Signal { - name: "SYS", - value: 31, - }, +pub static ALL_SIGNALS: [&str; 32] = [ + "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "BUS", "FPE", "KILL", "USR1", "SEGV", + "USR2", "PIPE", "ALRM", "TERM", "STKFLT", "CHLD", "CONT", "STOP", "TSTP", "TTIN", "TTOU", + "URG", "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "POLL", "PWR", "SYS", ]; /* @@ -202,135 +72,10 @@ No Name Default Action Description */ #[cfg(any(target_vendor = "apple", target_os = "freebsd"))] -pub static ALL_SIGNALS: [Signal<'static>; 32] = [ - Signal { - name: "EXIT", - value: 0, - }, - Signal { - name: "HUP", - value: 1, - }, - Signal { - name: "INT", - value: 2, - }, - Signal { - name: "QUIT", - value: 3, - }, - Signal { - name: "ILL", - value: 4, - }, - Signal { - name: "TRAP", - value: 5, - }, - Signal { - name: "ABRT", - value: 6, - }, - Signal { - name: "EMT", - value: 7, - }, - Signal { - name: "FPE", - value: 8, - }, - Signal { - name: "KILL", - value: 9, - }, - Signal { - name: "BUS", - value: 10, - }, - Signal { - name: "SEGV", - value: 11, - }, - Signal { - name: "SYS", - value: 12, - }, - Signal { - name: "PIPE", - value: 13, - }, - Signal { - name: "ALRM", - value: 14, - }, - Signal { - name: "TERM", - value: 15, - }, - Signal { - name: "URG", - value: 16, - }, - Signal { - name: "STOP", - value: 17, - }, - Signal { - name: "TSTP", - value: 18, - }, - Signal { - name: "CONT", - value: 19, - }, - Signal { - name: "CHLD", - value: 20, - }, - Signal { - name: "TTIN", - value: 21, - }, - Signal { - name: "TTOU", - value: 22, - }, - Signal { - name: "IO", - value: 23, - }, - Signal { - name: "XCPU", - value: 24, - }, - Signal { - name: "XFSZ", - value: 25, - }, - Signal { - name: "VTALRM", - value: 26, - }, - Signal { - name: "PROF", - value: 27, - }, - Signal { - name: "WINCH", - value: 28, - }, - Signal { - name: "INFO", - value: 29, - }, - Signal { - name: "USR1", - value: 30, - }, - Signal { - name: "USR2", - value: 31, - }, +pub static ALL_SIGNALS: [&str; 32] = [ + "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "EMT", "FPE", "KILL", "BUS", "SEGV", + "SYS", "PIPE", "ALRM", "TERM", "URG", "STOP", "TSTP", "CONT", "CHLD", "TTIN", "TTOU", "IO", + "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "INFO", "USR1", "USR2", ]; pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { @@ -343,70 +88,45 @@ pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { } let signal_name = signal_name_or_value.trim_start_matches("SIG"); - ALL_SIGNALS - .iter() - .find(|s| s.name == signal_name) - .map(|s| s.value) + ALL_SIGNALS.iter().position(|&s| s == signal_name) +} + +pub fn is_signal(num: usize) -> bool { + num < ALL_SIGNALS.len() } pub fn signal_name_by_value(signal_value: usize) -> Option<&'static str> { - ALL_SIGNALS - .iter() - .find(|signal| signal.value == signal_value) - .map(|signal| signal.name) -} - -#[inline(always)] -pub fn is_signal(num: usize) -> bool { - // Named signals start at 1 - num <= ALL_SIGNALS.len() -} - -#[test] -fn signals_all_contiguous() { - for (i, signal) in ALL_SIGNALS.iter().enumerate() { - assert_eq!(signal.value, i); - } -} - -#[test] -fn signals_all_are_signal() { - for signal in &ALL_SIGNALS { - assert!(is_signal(signal.value)); - } + ALL_SIGNALS.get(signal_value).copied() } #[test] fn signal_by_value() { assert_eq!(signal_by_name_or_value("0"), Some(0)); - for signal in &ALL_SIGNALS { - assert_eq!( - signal_by_name_or_value(&signal.value.to_string()), - Some(signal.value) - ); + for (value, _signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_by_name_or_value(&value.to_string()), Some(value)); } } #[test] fn signal_by_short_name() { - for signal in &ALL_SIGNALS { - assert_eq!(signal_by_name_or_value(signal.name), Some(signal.value)); + for (value, signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_by_name_or_value(signal), Some(value)); } } #[test] fn signal_by_long_name() { - for signal in &ALL_SIGNALS { + for (value, signal) in ALL_SIGNALS.iter().enumerate() { assert_eq!( - signal_by_name_or_value(&format!("SIG{}", signal.name)), - Some(signal.value) + signal_by_name_or_value(&format!("SIG{}", signal)), + Some(value) ); } } #[test] fn name() { - for signal in &ALL_SIGNALS { - assert_eq!(signal_name_by_value(signal.value), Some(signal.name)); + for (value, signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_name_by_value(value), Some(*signal)); } } From a57313f01b29f2b363eea3dcace5d2ceeaa02754 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 12 Jun 2021 13:01:42 +0200 Subject: [PATCH 13/25] core: represent signal values by their index --- src/uu/kill/src/kill.rs | 22 +- src/uucore/src/lib/features/signals.rs | 326 ++----------------------- 2 files changed, 33 insertions(+), 315 deletions(-) diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index a49acaa05..7965c40a9 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -132,13 +132,13 @@ fn table() { let mut name_width = 0; /* Compute the maximum width of a signal name. */ for s in &ALL_SIGNALS { - if s.name.len() > name_width { - name_width = s.name.len() + if s.len() > name_width { + name_width = s.len() } } for (idx, signal) in ALL_SIGNALS.iter().enumerate() { - print!("{0: >#2} {1: <#8}", idx + 1, signal.name); + print!("{0: >#2} {1: <#8}", idx + 1, signal); //TODO: obtain max signal width here if (idx + 1) % 7 == 0 { @@ -148,14 +148,12 @@ fn table() { } fn print_signal(signal_name_or_value: &str) { - for signal in &ALL_SIGNALS { - if signal.name == signal_name_or_value - || (format!("SIG{}", signal.name)) == signal_name_or_value - { - println!("{}", signal.value); + for (value, &signal) in ALL_SIGNALS.iter().enumerate() { + if signal == signal_name_or_value || (format!("SIG{}", signal)) == signal_name_or_value { + println!("{}", value); exit!(EXIT_OK as i32) - } else if signal_name_or_value == signal.value.to_string() { - println!("{}", signal.name); + } else if signal_name_or_value == value.to_string() { + println!("{}", signal); exit!(EXIT_OK as i32) } } @@ -165,8 +163,8 @@ fn print_signal(signal_name_or_value: &str) { fn print_signals() { let mut pos = 0; for (idx, signal) in ALL_SIGNALS.iter().enumerate() { - pos += signal.name.len(); - print!("{}", signal.name); + pos += signal.len(); + print!("{}", signal); if idx > 0 && pos > 73 { println!(); pos = 0; diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 3c52a9158..e6d2e7763 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -10,11 +10,6 @@ pub static DEFAULT_SIGNAL: usize = 15; -pub struct Signal<'a> { - pub name: &'a str, - pub value: usize, -} - /* Linux Programmer's Manual @@ -29,135 +24,10 @@ Linux Programmer's Manual */ #[cfg(target_os = "linux")] -pub static ALL_SIGNALS: [Signal<'static>; 32] = [ - Signal { - name: "EXIT", - value: 0, - }, - Signal { - name: "HUP", - value: 1, - }, - Signal { - name: "INT", - value: 2, - }, - Signal { - name: "QUIT", - value: 3, - }, - Signal { - name: "ILL", - value: 4, - }, - Signal { - name: "TRAP", - value: 5, - }, - Signal { - name: "ABRT", - value: 6, - }, - Signal { - name: "BUS", - value: 7, - }, - Signal { - name: "FPE", - value: 8, - }, - Signal { - name: "KILL", - value: 9, - }, - Signal { - name: "USR1", - value: 10, - }, - Signal { - name: "SEGV", - value: 11, - }, - Signal { - name: "USR2", - value: 12, - }, - Signal { - name: "PIPE", - value: 13, - }, - Signal { - name: "ALRM", - value: 14, - }, - Signal { - name: "TERM", - value: 15, - }, - Signal { - name: "STKFLT", - value: 16, - }, - Signal { - name: "CHLD", - value: 17, - }, - Signal { - name: "CONT", - value: 18, - }, - Signal { - name: "STOP", - value: 19, - }, - Signal { - name: "TSTP", - value: 20, - }, - Signal { - name: "TTIN", - value: 21, - }, - Signal { - name: "TTOU", - value: 22, - }, - Signal { - name: "URG", - value: 23, - }, - Signal { - name: "XCPU", - value: 24, - }, - Signal { - name: "XFSZ", - value: 25, - }, - Signal { - name: "VTALRM", - value: 26, - }, - Signal { - name: "PROF", - value: 27, - }, - Signal { - name: "WINCH", - value: 28, - }, - Signal { - name: "POLL", - value: 29, - }, - Signal { - name: "PWR", - value: 30, - }, - Signal { - name: "SYS", - value: 31, - }, +pub static ALL_SIGNALS: [&str; 32] = [ + "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "BUS", "FPE", "KILL", "USR1", "SEGV", + "USR2", "PIPE", "ALRM", "TERM", "STKFLT", "CHLD", "CONT", "STOP", "TSTP", "TTIN", "TTOU", + "URG", "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "POLL", "PWR", "SYS", ]; /* @@ -202,135 +72,10 @@ No Name Default Action Description */ #[cfg(any(target_vendor = "apple", target_os = "freebsd"))] -pub static ALL_SIGNALS: [Signal<'static>; 32] = [ - Signal { - name: "EXIT", - value: 0, - }, - Signal { - name: "HUP", - value: 1, - }, - Signal { - name: "INT", - value: 2, - }, - Signal { - name: "QUIT", - value: 3, - }, - Signal { - name: "ILL", - value: 4, - }, - Signal { - name: "TRAP", - value: 5, - }, - Signal { - name: "ABRT", - value: 6, - }, - Signal { - name: "EMT", - value: 7, - }, - Signal { - name: "FPE", - value: 8, - }, - Signal { - name: "KILL", - value: 9, - }, - Signal { - name: "BUS", - value: 10, - }, - Signal { - name: "SEGV", - value: 11, - }, - Signal { - name: "SYS", - value: 12, - }, - Signal { - name: "PIPE", - value: 13, - }, - Signal { - name: "ALRM", - value: 14, - }, - Signal { - name: "TERM", - value: 15, - }, - Signal { - name: "URG", - value: 16, - }, - Signal { - name: "STOP", - value: 17, - }, - Signal { - name: "TSTP", - value: 18, - }, - Signal { - name: "CONT", - value: 19, - }, - Signal { - name: "CHLD", - value: 20, - }, - Signal { - name: "TTIN", - value: 21, - }, - Signal { - name: "TTOU", - value: 22, - }, - Signal { - name: "IO", - value: 23, - }, - Signal { - name: "XCPU", - value: 24, - }, - Signal { - name: "XFSZ", - value: 25, - }, - Signal { - name: "VTALRM", - value: 26, - }, - Signal { - name: "PROF", - value: 27, - }, - Signal { - name: "WINCH", - value: 28, - }, - Signal { - name: "INFO", - value: 29, - }, - Signal { - name: "USR1", - value: 30, - }, - Signal { - name: "USR2", - value: 31, - }, +pub static ALL_SIGNALS: [&str; 32] = [ + "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "EMT", "FPE", "KILL", "BUS", "SEGV", + "SYS", "PIPE", "ALRM", "TERM", "URG", "STOP", "TSTP", "CONT", "CHLD", "TTIN", "TTOU", "IO", + "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "INFO", "USR1", "USR2", ]; pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { @@ -343,70 +88,45 @@ pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { } let signal_name = signal_name_or_value.trim_start_matches("SIG"); - ALL_SIGNALS - .iter() - .find(|s| s.name == signal_name) - .map(|s| s.value) + ALL_SIGNALS.iter().position(|&s| s == signal_name) +} + +pub fn is_signal(num: usize) -> bool { + num < ALL_SIGNALS.len() } pub fn signal_name_by_value(signal_value: usize) -> Option<&'static str> { - ALL_SIGNALS - .iter() - .find(|signal| signal.value == signal_value) - .map(|signal| signal.name) -} - -#[inline(always)] -pub fn is_signal(num: usize) -> bool { - // Named signals start at 1 - num <= ALL_SIGNALS.len() -} - -#[test] -fn signals_all_contiguous() { - for (i, signal) in ALL_SIGNALS.iter().enumerate() { - assert_eq!(signal.value, i); - } -} - -#[test] -fn signals_all_are_signal() { - for signal in &ALL_SIGNALS { - assert!(is_signal(signal.value)); - } + ALL_SIGNALS.get(signal_value).copied() } #[test] fn signal_by_value() { assert_eq!(signal_by_name_or_value("0"), Some(0)); - for signal in &ALL_SIGNALS { - assert_eq!( - signal_by_name_or_value(&signal.value.to_string()), - Some(signal.value) - ); + for (value, _signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_by_name_or_value(&value.to_string()), Some(value)); } } #[test] fn signal_by_short_name() { - for signal in &ALL_SIGNALS { - assert_eq!(signal_by_name_or_value(signal.name), Some(signal.value)); + for (value, signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_by_name_or_value(signal), Some(value)); } } #[test] fn signal_by_long_name() { - for signal in &ALL_SIGNALS { + for (value, signal) in ALL_SIGNALS.iter().enumerate() { assert_eq!( - signal_by_name_or_value(&format!("SIG{}", signal.name)), - Some(signal.value) + signal_by_name_or_value(&format!("SIG{}", signal)), + Some(value) ); } } #[test] fn name() { - for signal in &ALL_SIGNALS { - assert_eq!(signal_name_by_value(signal.value), Some(signal.name)); + for (value, signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_name_by_value(value), Some(*signal)); } } From d6181ce7d44e523c8400f4081bb1b16d3d97b0aa Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sat, 5 Jun 2021 23:35:15 +0530 Subject: [PATCH 14/25] du: Add threshold argument support - Add --threshold parameter and corresponding logic to skip listing entires that don't adhere to the threshold --- src/uu/du/src/du.rs | 52 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index b7c53eb72..39af3d7e1 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -28,6 +28,7 @@ use std::os::windows::io::AsRawHandle; #[cfg(windows)] use std::path::Path; use std::path::PathBuf; +use std::str::FromStr; use std::time::{Duration, UNIX_EPOCH}; use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::InvalidEncodingHandling; @@ -56,6 +57,7 @@ mod options { pub const BLOCK_SIZE_1M: &str = "m"; pub const SEPARATE_DIRS: &str = "S"; pub const SUMMARIZE: &str = "s"; + pub const THRESHOLD: &str = "threshold"; pub const SI: &str = "si"; pub const TIME: &str = "time"; pub const TIME_STYLE: &str = "time-style"; @@ -510,6 +512,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(options::ONE_FILE_SYSTEM) .help("skip directories on different file systems") ) + .arg( + Arg::with_name(options::THRESHOLD) + .short("t") + .long(options::THRESHOLD) + .alias("th") + .value_name("SIZE") + .number_of_values(1) + .allow_hyphen_values(true) + .help("exclude entries smaller than SIZE if positive, \ + or entries greater than SIZE if negative") + ) // .arg( // Arg::with_name("") // .short("x") @@ -586,6 +599,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let block_size = u64::try_from(read_block_size(matches.value_of(options::BLOCK_SIZE))).unwrap(); + let threshold = match matches.value_of(options::THRESHOLD) { + Some(s) => Threshold::from_str(s) + .unwrap_or_else(|e| crash!(1, "{}", format_error_message(e, s, options::THRESHOLD))), + None => Threshold(None, false), + }; + let multiplier: u64 = if matches.is_present(options::SI) { 1000 } else { @@ -654,6 +673,10 @@ Try '{} --help' for more information.", // See: http://linux.die.net/man/2/stat stat.blocks * 512 }; + if threshold.should_exclude(size) { + continue; + } + if matches.is_present(options::TIME) { let tm = { let secs = { @@ -720,6 +743,35 @@ Try '{} --help' for more information.", 0 } +struct Threshold(Option, bool); + +impl FromStr for Threshold { + type Err = ParseSizeError; + + fn from_str(s: &str) -> std::result::Result { + let offset = if s.starts_with('-') || s.starts_with('+') { + 1 + } else { + 0 + }; + let sz = parse_size(&s[offset..])?; + + Ok(Threshold( + Some(u64::try_from(sz).unwrap()), + s.starts_with('-'), + )) + } +} + +impl Threshold { + fn should_exclude(&self, sz: u64) -> bool { + match *self { + Threshold(Some(th), is_upper) => (is_upper && sz > th) || (!is_upper && sz < th), + Threshold(None, _) => false, + } + } +} + fn format_error_message(error: ParseSizeError, s: &str, option: &str) -> String { // NOTE: // GNU's du echos affected flag, -B or --block-size (-t or --threshold), depending user's selection From fa12b46c51268dd4c4f2db6fcaf93421c3330015 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sat, 12 Jun 2021 19:30:48 +0530 Subject: [PATCH 15/25] tests: Add test for du threshold feature --- tests/by-util/test_du.rs | 50 +++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 8d1267423..93875ae51 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -80,19 +80,22 @@ fn _du_basics_subdir(s: &str) { #[test] fn test_du_invalid_size() { - new_ucmd!() - .arg("--block-size=1fb4t") - .arg("/tmp") - .fails() - .code_is(1) - .stderr_only("du: invalid --block-size argument '1fb4t'"); - #[cfg(not(target_pointer_width = "128"))] - new_ucmd!() - .arg("--block-size=1Y") - .arg("/tmp") - .fails() - .code_is(1) - .stderr_only("du: --block-size argument '1Y' too large"); + let args = &["block-size", "threshold"]; + for s in args { + new_ucmd!() + .arg(format!("--{}=1fb4t", s)) + .arg("/tmp") + .fails() + .code_is(1) + .stderr_only(format!("du: invalid --{} argument '1fb4t'", s)); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .arg(format!("--{}=1Y", s)) + .arg("/tmp") + .fails() + .code_is(1) + .stderr_only(format!("du: --{} argument '1Y' too large", s)); + } } #[test] @@ -351,3 +354,24 @@ fn test_du_one_file_system() { } _du_basics_subdir(result.stdout_str()); } + +#[test] +fn test_du_threshold() { + let scene = TestScenario::new(util_name!()); + + let threshold = if cfg!(windows) { "7K" } else { "10K" }; + + scene + .ucmd() + .arg(format!("--threshold={}", threshold)) + .succeeds() + .stdout_contains("links") + .stdout_does_not_contain("deeper"); + + scene + .ucmd() + .arg(format!("--threshold=-{}", threshold)) + .succeeds() + .stdout_does_not_contain("links") + .stdout_contains("deeper"); +} From 8e7eedebe7d3578289b54dce7f69668840b74a23 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 11 Jun 2021 22:00:26 +0200 Subject: [PATCH 16/25] tests: take slices in `stdout_is_fixture` --- tests/by-util/test_pr.rs | 132 +++++++++------------------------------ tests/common/util.rs | 2 +- 2 files changed, 31 insertions(+), 103 deletions(-) diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs index def361fab..c1dee2a6c 100644 --- a/tests/by-util/test_pr.rs +++ b/tests/by-util/test_pr.rs @@ -33,10 +33,7 @@ fn test_without_any_options() { scenario .args(&[test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -48,10 +45,7 @@ fn test_with_numbering_option_with_number_width() { scenario .args(&["-n", "2", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -66,10 +60,7 @@ fn test_with_long_header_option() { .succeeds() .stdout_is_templated_fixture( expected_test_file_path, - vec![ - (&"{last_modified_time}".to_string(), &value), - (&"{header}".to_string(), &header.to_string()), - ], + &[("{last_modified_time}", &value), ("{header}", header)], ); new_ucmd!() @@ -77,10 +68,7 @@ fn test_with_long_header_option() { .succeeds() .stdout_is_templated_fixture( expected_test_file_path, - vec![ - (&"{last_modified_time}".to_string(), &value), - (&"{header}".to_string(), &header.to_string()), - ], + &[("{last_modified_time}", &value), ("{header}", header)], ); } @@ -93,18 +81,12 @@ fn test_with_double_space_option() { scenario .args(&["-d", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); new_ucmd!() .args(&["--double-space", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -116,10 +98,7 @@ fn test_with_first_line_number_option() { scenario .args(&["-N", "5", "-n", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -131,10 +110,7 @@ fn test_with_first_line_number_long_option() { scenario .args(&["--first-line-number=5", "-n", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -146,10 +122,7 @@ fn test_with_number_option_with_custom_separator_char() { scenario .args(&["-nc", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -161,10 +134,7 @@ fn test_with_number_option_with_custom_separator_char_and_width() { scenario .args(&["-nc1", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -207,25 +177,19 @@ fn test_with_page_range() { scenario .args(&["--pages=15", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); new_ucmd!() .args(&["+15", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); new_ucmd!() .args(&["--pages=15:17", test_file_path]) .succeeds() .stdout_is_templated_fixture( expected_test_file_path1, - vec![(&"{last_modified_time}".to_string(), &value)], + &[("{last_modified_time}", &value)], ); new_ucmd!() @@ -233,7 +197,7 @@ fn test_with_page_range() { .succeeds() .stdout_is_templated_fixture( expected_test_file_path1, - vec![(&"{last_modified_time}".to_string(), &value)], + &[("{last_modified_time}", &value)], ); } @@ -246,10 +210,7 @@ fn test_with_no_header_trailer_option() { scenario .args(&["-t", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -262,10 +223,7 @@ fn test_with_page_length_option() { scenario .args(&["--pages=2:3", "-l", "100", "-n", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); new_ucmd!() .args(&["--pages=2:3", "-l", "5", "-n", test_file_path]) @@ -293,10 +251,7 @@ fn test_with_stdin() { .pipe_in_fixture("stdin.log") .args(&["--pages=1:2", "-n", "-"]) .run() - .stdout_is_templated_fixture( - expected_file_path, - vec![(&"{last_modified_time}".to_string(), &now)], - ); + .stdout_is_templated_fixture(expected_file_path, &[("{last_modified_time}", &now)]); } #[test] @@ -308,18 +263,12 @@ fn test_with_column() { scenario .args(&["--pages=3:5", "--column=3", "-n", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); new_ucmd!() .args(&["--pages=3:5", "-3", "-n", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -331,10 +280,7 @@ fn test_with_column_across_option() { scenario .args(&["--pages=3:5", "--column=3", "-a", "-n", test_file_path]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -354,10 +300,7 @@ fn test_with_column_across_option_and_column_separator() { test_file_path, ]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); new_ucmd!() .args(&[ @@ -371,7 +314,7 @@ fn test_with_column_across_option_and_column_separator() { .succeeds() .stdout_is_templated_fixture( expected_test_file_path1, - vec![(&"{last_modified_time}".to_string(), &value)], + &[("{last_modified_time}", &value)], ); } @@ -386,19 +329,13 @@ fn test_with_mpr() { new_ucmd!() .args(&["--pages=1:2", "-m", "-n", test_file_path, test_file_path1]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &now)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &now)]); let now = now_time(); new_ucmd!() .args(&["--pages=2:4", "-m", "-n", test_file_path, test_file_path1]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path1, - vec![(&"{last_modified_time}".to_string(), &now)], - ); + .stdout_is_templated_fixture(expected_test_file_path1, &[("{last_modified_time}", &now)]); let now = now_time(); new_ucmd!() @@ -413,10 +350,7 @@ fn test_with_mpr() { test_file_path, ]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path2, - vec![(&"{last_modified_time}".to_string(), &now)], - ); + .stdout_is_templated_fixture(expected_test_file_path2, &[("{last_modified_time}", &now)]); } #[test] @@ -452,10 +386,7 @@ fn test_with_offset_space_option() { test_file_path, ]) .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - vec![(&"{last_modified_time}".to_string(), &value)], - ); + .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); } #[test] @@ -497,9 +428,9 @@ fn test_with_pr_core_utils_tests() { scenario_with_expected_status.stdout_is_templated_fixture( test_file_path, - vec![ - (&"{last_modified_time}".to_string(), &value), - (&"{file_name}".to_string(), &input_file_path.to_string()), + &[ + ("{last_modified_time}", &value), + ("{file_name}", input_file_path), ], ); } @@ -515,8 +446,5 @@ fn test_with_join_lines_option() { scenario .args(&["+1:2", "-J", "-m", test_file_1, test_file_2]) .run() - .stdout_is_templated_fixture( - expected_file_path, - vec![(&"{last_modified_time}".to_string(), &now)], - ); + .stdout_is_templated_fixture(expected_file_path, &[("{last_modified_time}", &now)]); } diff --git a/tests/common/util.rs b/tests/common/util.rs index 922d2ba36..52911912e 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -247,7 +247,7 @@ impl CmdResult { pub fn stdout_is_templated_fixture>( &self, file_rel_path: T, - template_vars: Vec<(&String, &String)>, + template_vars: &[(&str, &str)], ) -> &CmdResult { let mut contents = String::from_utf8(read_scenario_fixture(&self.tmpd, file_rel_path)).unwrap(); From 98088db9ff09d40ee9f61aa697dedf41e2ae71ce Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 11 Jun 2021 22:02:10 +0200 Subject: [PATCH 17/25] tests: add `_any` functions This should make it easier to write tests that could have different valid outputs depending on timing. --- tests/common/util.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/common/util.rs b/tests/common/util.rs index 52911912e..f881cff21 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -223,6 +223,18 @@ impl CmdResult { self } + /// like `stdout_is`, but succeeds if any elements of `expected` matches stdout. + pub fn stdout_is_any + std::fmt::Debug>(&self, expected: Vec) -> &CmdResult { + if !expected.iter().any(|msg| self.stdout_str() == msg.as_ref()) { + panic!( + "stdout was {}\nExpected any of {:#?}", + self.stdout_str(), + expected + ) + } + self + } + /// Like `stdout_is` but newlines are normalized to `\n`. pub fn normalized_newlines_stdout_is>(&self, msg: T) -> &CmdResult { let msg = msg.as_ref().replace("\r\n", "\n"); @@ -257,6 +269,23 @@ impl CmdResult { self.stdout_is(contents) } + /// like `stdout_is_templated_fixture`, but succeeds if any replacement by `template_vars` results in the actual stdout. + pub fn stdout_is_templated_fixture_any>( + &self, + file_rel_path: T, + template_vars: &[Vec<(String, String)>], + ) { + let contents = String::from_utf8(read_scenario_fixture(&self.tmpd, file_rel_path)).unwrap(); + let possible_values = template_vars.iter().map(|vars| { + let mut contents = contents.clone(); + for kv in vars.iter() { + contents = contents.replace(&kv.0, &kv.1); + } + contents + }); + self.stdout_is_any(possible_values.collect()); + } + /// asserts that the command resulted in stderr stream output that equals the /// passed in value, when both are trimmed of trailing whitespace /// stderr_only is a better choice unless stdout may or will be non-empty From bb029193e2b281f07f5fae69985a42d028f4f6a6 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 11 Jun 2021 22:29:16 +0200 Subject: [PATCH 18/25] tests/pr: prevent races Allow any timestamp from the start of the command to its end to show up in stdout. --- tests/by-util/test_pr.rs | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs index c1dee2a6c..2391bc37a 100644 --- a/tests/by-util/test_pr.rs +++ b/tests/by-util/test_pr.rs @@ -3,6 +3,7 @@ use crate::common::util::*; use chrono::offset::Local; use chrono::DateTime; +use chrono::Duration; use std::fs::metadata; fn file_last_modified_time(ucmd: &UCommand, path: &str) -> String { @@ -20,8 +21,22 @@ fn file_last_modified_time(ucmd: &UCommand, path: &str) -> String { .unwrap_or_default() } -fn now_time() -> String { - Local::now().format("%b %d %H:%M %Y").to_string() +fn all_minutes(from: DateTime, to: DateTime) -> Vec { + const FORMAT: &str = "%b %d %H:%M %Y"; + let mut vec = vec![]; + let mut current = from; + while current < to { + vec.push(current.format(FORMAT).to_string()); + current = current + Duration::minutes(1); + } + vec +} + +fn valid_last_modified_template_vars(from: DateTime) -> Vec> { + all_minutes(from, Local::now()) + .into_iter() + .map(|time| vec![("{last_modified_time}".to_string(), time)]) + .collect() } #[test] @@ -246,12 +261,12 @@ fn test_with_suppress_error_option() { fn test_with_stdin() { let expected_file_path = "stdin.log.expected"; let mut scenario = new_ucmd!(); - let now = now_time(); + let start = Local::now(); scenario .pipe_in_fixture("stdin.log") .args(&["--pages=1:2", "-n", "-"]) .run() - .stdout_is_templated_fixture(expected_file_path, &[("{last_modified_time}", &now)]); + .stdout_is_templated_fixture_any(expected_file_path, &valid_last_modified_template_vars(start)); } #[test] @@ -325,19 +340,19 @@ fn test_with_mpr() { let expected_test_file_path = "mpr.log.expected"; let expected_test_file_path1 = "mpr1.log.expected"; let expected_test_file_path2 = "mpr2.log.expected"; - let now = now_time(); + let start = Local::now(); new_ucmd!() .args(&["--pages=1:2", "-m", "-n", test_file_path, test_file_path1]) .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &now)]); + .stdout_is_templated_fixture_any(expected_test_file_path, &valid_last_modified_template_vars(start)); - let now = now_time(); + let start = Local::now(); new_ucmd!() .args(&["--pages=2:4", "-m", "-n", test_file_path, test_file_path1]) .succeeds() - .stdout_is_templated_fixture(expected_test_file_path1, &[("{last_modified_time}", &now)]); + .stdout_is_templated_fixture_any(expected_test_file_path1, &valid_last_modified_template_vars(start)); - let now = now_time(); + let start = Local::now(); new_ucmd!() .args(&[ "--pages=1:2", @@ -350,7 +365,7 @@ fn test_with_mpr() { test_file_path, ]) .succeeds() - .stdout_is_templated_fixture(expected_test_file_path2, &[("{last_modified_time}", &now)]); + .stdout_is_templated_fixture_any(expected_test_file_path2, &valid_last_modified_template_vars(start)); } #[test] @@ -442,9 +457,9 @@ fn test_with_join_lines_option() { let test_file_2 = "test.log"; let expected_file_path = "joined.log.expected"; let mut scenario = new_ucmd!(); - let now = now_time(); + let start = Local::now(); scenario .args(&["+1:2", "-J", "-m", test_file_1, test_file_2]) .run() - .stdout_is_templated_fixture(expected_file_path, &[("{last_modified_time}", &now)]); + .stdout_is_templated_fixture_any(expected_file_path, &valid_last_modified_template_vars(start)); } From d8c8e6774ff5c66783ff875635282e7fb5468162 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 12 Jun 2021 12:35:50 +0200 Subject: [PATCH 19/25] tests/pr: formatting --- tests/by-util/test_pr.rs | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs index 2391bc37a..fb6703f28 100644 --- a/tests/by-util/test_pr.rs +++ b/tests/by-util/test_pr.rs @@ -266,7 +266,10 @@ fn test_with_stdin() { .pipe_in_fixture("stdin.log") .args(&["--pages=1:2", "-n", "-"]) .run() - .stdout_is_templated_fixture_any(expected_file_path, &valid_last_modified_template_vars(start)); + .stdout_is_templated_fixture_any( + expected_file_path, + &valid_last_modified_template_vars(start), + ); } #[test] @@ -344,13 +347,19 @@ fn test_with_mpr() { new_ucmd!() .args(&["--pages=1:2", "-m", "-n", test_file_path, test_file_path1]) .succeeds() - .stdout_is_templated_fixture_any(expected_test_file_path, &valid_last_modified_template_vars(start)); + .stdout_is_templated_fixture_any( + expected_test_file_path, + &valid_last_modified_template_vars(start), + ); let start = Local::now(); new_ucmd!() .args(&["--pages=2:4", "-m", "-n", test_file_path, test_file_path1]) .succeeds() - .stdout_is_templated_fixture_any(expected_test_file_path1, &valid_last_modified_template_vars(start)); + .stdout_is_templated_fixture_any( + expected_test_file_path1, + &valid_last_modified_template_vars(start), + ); let start = Local::now(); new_ucmd!() @@ -365,7 +374,10 @@ fn test_with_mpr() { test_file_path, ]) .succeeds() - .stdout_is_templated_fixture_any(expected_test_file_path2, &valid_last_modified_template_vars(start)); + .stdout_is_templated_fixture_any( + expected_test_file_path2, + &valid_last_modified_template_vars(start), + ); } #[test] @@ -461,5 +473,8 @@ fn test_with_join_lines_option() { scenario .args(&["+1:2", "-J", "-m", test_file_1, test_file_2]) .run() - .stdout_is_templated_fixture_any(expected_file_path, &valid_last_modified_template_vars(start)); + .stdout_is_templated_fixture_any( + expected_file_path, + &valid_last_modified_template_vars(start), + ); } From da7b02cf9d8b64d35de957124807f3a3b80c02ac Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sat, 12 Jun 2021 21:31:56 +0530 Subject: [PATCH 20/25] du: Refactor threshold handling --- src/uu/du/src/du.rs | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 39af3d7e1..e466b8afe 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -599,11 +599,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let block_size = u64::try_from(read_block_size(matches.value_of(options::BLOCK_SIZE))).unwrap(); - let threshold = match matches.value_of(options::THRESHOLD) { - Some(s) => Threshold::from_str(s) - .unwrap_or_else(|e| crash!(1, "{}", format_error_message(e, s, options::THRESHOLD))), - None => Threshold(None, false), - }; + let threshold = matches.value_of(options::THRESHOLD).map(|s| { + Threshold::from_str(s) + .unwrap_or_else(|e| crash!(1, "{}", format_error_message(e, s, options::THRESHOLD))) + }); let multiplier: u64 = if matches.is_present(options::SI) { 1000 @@ -673,7 +672,8 @@ Try '{} --help' for more information.", // See: http://linux.die.net/man/2/stat stat.blocks * 512 }; - if threshold.should_exclude(size) { + + if threshold.map_or(false, |threshold| threshold.should_exclude(size)) { continue; } @@ -743,31 +743,33 @@ Try '{} --help' for more information.", 0 } -struct Threshold(Option, bool); +#[derive(Clone, Copy)] +enum Threshold { + Lower(u64), + Upper(u64), +} impl FromStr for Threshold { type Err = ParseSizeError; fn from_str(s: &str) -> std::result::Result { - let offset = if s.starts_with('-') || s.starts_with('+') { - 1 - } else { - 0 - }; - let sz = parse_size(&s[offset..])?; + let offset = if s.starts_with(&['-', '+'][..]) { 1 } else { 0 }; - Ok(Threshold( - Some(u64::try_from(sz).unwrap()), - s.starts_with('-'), - )) + let size = u64::try_from(parse_size(&s[offset..])?).unwrap(); + + if s.starts_with('-') { + Ok(Threshold::Upper(size)) + } else { + Ok(Threshold::Lower(size)) + } } } impl Threshold { - fn should_exclude(&self, sz: u64) -> bool { + fn should_exclude(&self, size: u64) -> bool { match *self { - Threshold(Some(th), is_upper) => (is_upper && sz > th) || (!is_upper && sz < th), - Threshold(None, _) => false, + Threshold::Upper(threshold) => size > threshold, + Threshold::Lower(threshold) => size < threshold, } } } From 7acb9373a62270c5baee091019da88f43ebc16b8 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 14 Jun 2021 11:10:41 +0200 Subject: [PATCH 21/25] kill: fix signal table printing --- src/uu/kill/src/kill.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index 7965c40a9..c48864564 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -129,22 +129,15 @@ fn handle_obsolete(mut args: Vec) -> (Vec, Option) { } fn table() { - let mut name_width = 0; - /* Compute the maximum width of a signal name. */ - for s in &ALL_SIGNALS { - if s.len() > name_width { - name_width = s.len() - } - } + let name_width = ALL_SIGNALS.iter().map(|n| n.len()).max().unwrap(); for (idx, signal) in ALL_SIGNALS.iter().enumerate() { - print!("{0: >#2} {1: <#8}", idx + 1, signal); - //TODO: obtain max signal width here - + print!("{0: >#2} {1: <#2$}", idx, signal, name_width + 2); if (idx + 1) % 7 == 0 { println!(); } } + println!() } fn print_signal(signal_name_or_value: &str) { From 13458b48066c5d8476a58b04685b56ff31925046 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 14 Jun 2021 11:39:26 +0200 Subject: [PATCH 22/25] sort: use values_of --- src/uu/sort/src/sort.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 006664193..bc5048e11 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -1272,11 +1272,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.separator = Some(separator.chars().next().unwrap()) } - if matches.is_present(options::KEY) { - for key in &matches.args[options::KEY].vals { + if let Some(values) = matches.values_of(options::KEY) { + for value in values { settings .selectors - .push(FieldSelector::parse(&key.to_string_lossy(), &settings)); + .push(FieldSelector::parse(value, &settings)); } } From 25240ba61c1e3319840b72ef3be36fc295e13412 Mon Sep 17 00:00:00 2001 From: David Suilea Date: Sat, 12 Jun 2021 13:28:21 +0200 Subject: [PATCH 23/25] touch: change the error message to match the GNU error message #2346 --- src/uu/touch/src/touch.rs | 10 +++++++++- tests/by-util/test_touch.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index be4e51041..efa436c81 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -166,7 +166,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } if let Err(e) = File::create(path) { - show_warning!("cannot touch '{}': {}", path, e); + match e.kind() { + std::io::ErrorKind::NotFound => { + show_error!("cannot touch '{}': {}", path, "No such file or directory") + } + std::io::ErrorKind::PermissionDenied => { + show_error!("cannot touch '{}': {}", path, "Permission denied") + } + _ => show_error!("cannot touch '{}': {}", path, e), + } error_code = 1; continue; }; diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index c861a50dd..5e8114092 100644 --- a/tests/by-util/test_touch.rs +++ b/tests/by-util/test_touch.rs @@ -6,6 +6,7 @@ use self::touch::filetime::{self, FileTime}; extern crate time; use crate::common::util::*; +use std::path::PathBuf; fn get_file_times(at: &AtPath, path: &str) -> (FileTime, FileTime) { let m = at.metadata(path); @@ -466,3 +467,37 @@ fn test_touch_trailing_slash() { let file = "no-file/"; ucmd.args(&[file]).fails(); } + +#[test] +fn test_touch_no_such_file_error_msg() { + let dirname = "nonexistent"; + let filename = "file"; + let path = PathBuf::from(dirname).join(filename); + let path_str = path.to_str().unwrap(); + + new_ucmd!().arg(&path).fails().stderr_only(format!( + "touch: cannot touch '{}': No such file or directory", + path_str + )); +} + +#[test] +#[cfg(unix)] +fn test_touch_permission_denied_error_msg() { + let (at, mut ucmd) = at_and_ucmd!(); + + let dirname = "dir_with_read_only_access"; + let filename = "file"; + let path = PathBuf::from(dirname).join(filename); + let path_str = path.to_str().unwrap(); + + // create dest without write permissions + at.mkdir(dirname); + at.set_readonly(dirname); + + let full_path = at.plus_as_string(path_str); + ucmd.arg(&full_path).fails().stderr_only(format!( + "touch: cannot touch '{}': Permission denied", + &full_path + )); +} From 22fbf16b2c62f7688f7ba1d51de7260ed179c3cb Mon Sep 17 00:00:00 2001 From: Daniel Rocco Date: Fri, 4 Jun 2021 23:16:48 -0400 Subject: [PATCH 24/25] test: implement user, group ownership checks closes #2337 --- src/uu/test/src/test.rs | 36 +++++++++++++++++++++------- tests/by-util/test_test.rs | 48 +++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index acf0f7eca..e30d7cf51 100644 --- a/src/uu/test/src/test.rs +++ b/src/uu/test/src/test.rs @@ -6,7 +6,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (vars) FiletestOp StrlenOp +// spell-checker:ignore (vars) egid euid FiletestOp StrlenOp mod parser; @@ -96,8 +96,10 @@ fn eval(stack: &mut Vec) -> Result { "-e" => path(&f, PathCondition::Exists), "-f" => path(&f, PathCondition::Regular), "-g" => path(&f, PathCondition::GroupIdFlag), + "-G" => path(&f, PathCondition::GroupOwns), "-h" => path(&f, PathCondition::SymLink), "-L" => path(&f, PathCondition::SymLink), + "-O" => path(&f, PathCondition::UserOwns), "-p" => path(&f, PathCondition::Fifo), "-r" => path(&f, PathCondition::Readable), "-S" => path(&f, PathCondition::Socket), @@ -166,7 +168,9 @@ enum PathCondition { Exists, Regular, GroupIdFlag, + GroupOwns, SymLink, + UserOwns, Fifo, Readable, Socket, @@ -190,18 +194,28 @@ fn path(path: &OsStr, condition: PathCondition) -> bool { Execute = 0o1, } - let perm = |metadata: Metadata, p: Permission| { + let geteuid = || { #[cfg(not(target_os = "redox"))] - let (uid, gid) = unsafe { (libc::getuid(), libc::getgid()) }; + let euid = unsafe { libc::geteuid() }; #[cfg(target_os = "redox")] - let (uid, gid) = ( - syscall::getuid().unwrap() as u32, - syscall::getgid().unwrap() as u32, - ); + let euid = syscall::geteuid().unwrap() as u32; - if uid == metadata.uid() { + euid + }; + + let getegid = || { + #[cfg(not(target_os = "redox"))] + let egid = unsafe { libc::getegid() }; + #[cfg(target_os = "redox")] + let egid = syscall::getegid().unwrap() as u32; + + egid + }; + + let perm = |metadata: Metadata, p: Permission| { + if geteuid() == metadata.uid() { metadata.mode() & ((p as u32) << 6) != 0 - } else if gid == metadata.gid() { + } else if getegid() == metadata.gid() { metadata.mode() & ((p as u32) << 3) != 0 } else { metadata.mode() & (p as u32) != 0 @@ -230,7 +244,9 @@ fn path(path: &OsStr, condition: PathCondition) -> bool { PathCondition::Exists => true, PathCondition::Regular => file_type.is_file(), PathCondition::GroupIdFlag => metadata.mode() & S_ISGID != 0, + PathCondition::GroupOwns => metadata.gid() == getegid(), PathCondition::SymLink => metadata.file_type().is_symlink(), + PathCondition::UserOwns => metadata.uid() == geteuid(), PathCondition::Fifo => file_type.is_fifo(), PathCondition::Readable => perm(metadata, Permission::Read), PathCondition::Socket => file_type.is_socket(), @@ -257,7 +273,9 @@ fn path(path: &OsStr, condition: PathCondition) -> bool { PathCondition::Exists => true, PathCondition::Regular => stat.is_file(), PathCondition::GroupIdFlag => false, + PathCondition::GroupOwns => unimplemented!(), PathCondition::SymLink => false, + PathCondition::UserOwns => unimplemented!(), PathCondition::Fifo => false, PathCondition::Readable => false, // TODO PathCondition::Socket => false, diff --git a/tests/by-util/test_test.rs b/tests/by-util/test_test.rs index aaf09d657..8d41c5ead 100644 --- a/tests/by-util/test_test.rs +++ b/tests/by-util/test_test.rs @@ -8,7 +8,7 @@ // file that was distributed with this source code. // -// spell-checker:ignore (words) pseudofloat +// spell-checker:ignore (words) egid euid pseudofloat use crate::common::util::*; @@ -476,6 +476,52 @@ fn test_nonexistent_file_is_not_symlink() { .succeeds(); } +#[test] +#[cfg(not(windows))] +fn test_file_owned_by_euid() { + new_ucmd!().args(&["-O", "regular_file"]).succeeds(); +} + +#[test] +#[cfg(not(windows))] +fn test_nonexistent_file_not_owned_by_euid() { + new_ucmd!() + .args(&["-O", "nonexistent_file"]) + .run() + .status_code(1); +} + +#[test] +#[cfg(all(not(windows), not(target_os = "freebsd")))] +fn test_file_not_owned_by_euid() { + new_ucmd!() + .args(&["-f", "/bin/sh", "-a", "!", "-O", "/bin/sh"]) + .succeeds(); +} + +#[test] +#[cfg(not(windows))] +fn test_file_owned_by_egid() { + new_ucmd!().args(&["-G", "regular_file"]).succeeds(); +} + +#[test] +#[cfg(not(windows))] +fn test_nonexistent_file_not_owned_by_egid() { + new_ucmd!() + .args(&["-G", "nonexistent_file"]) + .run() + .status_code(1); +} + +#[test] +#[cfg(all(not(windows), not(target_os = "freebsd")))] +fn test_file_not_owned_by_egid() { + new_ucmd!() + .args(&["-f", "/bin/sh", "-a", "!", "-G", "/bin/sh"]) + .succeeds(); +} + #[test] fn test_op_precedence_and_or_1() { new_ucmd!().args(&[" ", "-o", "", "-a", ""]).succeeds(); From 996e1b8539456b033b3f96607f2a0a6c1afb19ea Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Tue, 15 Jun 2021 22:13:52 +0200 Subject: [PATCH 25/25] uucore/entries: fix `getgrouplist` wrapper to handle a bug in macOS's `getgrouplist` implementation * add documentation --- src/uucore/src/lib/features/entries.rs | 60 ++++++++++++++++++++------ src/uucore/src/lib/features/process.rs | 4 ++ 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/src/uucore/src/lib/features/entries.rs b/src/uucore/src/lib/features/entries.rs index bc4166346..6b986e616 100644 --- a/src/uucore/src/lib/features/entries.rs +++ b/src/uucore/src/lib/features/entries.rs @@ -47,6 +47,9 @@ use std::io::Result as IOResult; use std::ptr; extern "C" { + /// From: https://man7.org/linux/man-pages/man3/getgrouplist.3.html + /// > The getgrouplist() function scans the group database to obtain + /// > the list of groups that user belongs to. fn getgrouplist( name: *const c_char, gid: gid_t, @@ -55,6 +58,13 @@ extern "C" { ) -> c_int; } +/// From: https://man7.org/linux/man-pages/man2/getgroups.2.html +/// > getgroups() returns the supplementary group IDs of the calling +/// > process in list. +/// > If size is zero, list is not modified, but the total number of +/// > supplementary group IDs for the process is returned. This allows +/// > the caller to determine the size of a dynamically allocated list +/// > to be used in a further call to getgroups(). pub fn get_groups() -> IOResult> { let ngroups = unsafe { getgroups(0, ptr::null_mut()) }; if ngroups == -1 { @@ -83,17 +93,17 @@ pub fn get_groups() -> IOResult> { /// for `id --groups --real` if `gid` and `egid` are not equal. /// /// From: https://www.man7.org/linux/man-pages/man3/getgroups.3p.html -/// As implied by the definition of supplementary groups, the -/// effective group ID may appear in the array returned by -/// getgroups() or it may be returned only by getegid(). Duplication -/// may exist, but the application needs to call getegid() to be sure -/// of getting all of the information. Various implementation -/// variations and administrative sequences cause the set of groups -/// appearing in the result of getgroups() to vary in order and as to -/// whether the effective group ID is included, even when the set of -/// groups is the same (in the mathematical sense of ``set''). (The -/// history of a process and its parents could affect the details of -/// the result.) +/// > As implied by the definition of supplementary groups, the +/// > effective group ID may appear in the array returned by +/// > getgroups() or it may be returned only by getegid(). Duplication +/// > may exist, but the application needs to call getegid() to be sure +/// > of getting all of the information. Various implementation +/// > variations and administrative sequences cause the set of groups +/// > appearing in the result of getgroups() to vary in order and as to +/// > whether the effective group ID is included, even when the set of +/// > groups is the same (in the mathematical sense of ``set''). (The +/// > history of a process and its parents could affect the details of +/// > the result.) #[cfg(all(unix, feature = "process"))] pub fn get_groups_gnu(arg_id: Option) -> IOResult> { let groups = get_groups()?; @@ -184,16 +194,38 @@ impl Passwd { self.inner } + /// This is a wrapper function for `libc::getgrouplist`. + /// + /// From: https://man7.org/linux/man-pages/man3/getgrouplist.3.html + /// > If the number of groups of which user is a member is less than or + /// > equal to *ngroups, then the value *ngroups is returned. + /// > If the user is a member of more than *ngroups groups, then + /// > getgrouplist() returns -1. In this case, the value returned in + /// > *ngroups can be used to resize the buffer passed to a further + /// > call getgrouplist(). + /// + /// However, on macOS/darwin (and maybe others?) `getgrouplist` does + /// not update `ngroups` if `ngroups` is too small. Therefore, if not + /// updated by `getgrouplist`, `ngroups` needs to be increased in a + /// loop until `getgrouplist` stops returning -1. pub fn belongs_to(&self) -> Vec { let mut ngroups: c_int = 8; + let mut ngroups_old: c_int; let mut groups = Vec::with_capacity(ngroups as usize); let gid = self.inner.pw_gid; let name = self.inner.pw_name; - unsafe { - if getgrouplist(name, gid, groups.as_mut_ptr(), &mut ngroups) == -1 { + loop { + ngroups_old = ngroups; + if unsafe { getgrouplist(name, gid, groups.as_mut_ptr(), &mut ngroups) } == -1 { + if ngroups == ngroups_old { + ngroups *= 2; + } groups.resize(ngroups as usize, 0); - getgrouplist(name, gid, groups.as_mut_ptr(), &mut ngroups); + } else { + break; } + } + unsafe { groups.set_len(ngroups as usize); } groups.truncate(ngroups as usize); diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index cda41bb4f..21bfa992c 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -17,18 +17,22 @@ use std::process::ExitStatus as StdExitStatus; use std::thread; use std::time::{Duration, Instant}; +/// `geteuid()` returns the effective user ID of the calling process. pub fn geteuid() -> uid_t { unsafe { libc::geteuid() } } +/// `getegid()` returns the effective group ID of the calling process. pub fn getegid() -> gid_t { unsafe { libc::getegid() } } +/// `getgid()` returns the real group ID of the calling process. pub fn getgid() -> gid_t { unsafe { libc::getgid() } } +/// `getuid()` returns the real user ID of the calling process. pub fn getuid() -> uid_t { unsafe { libc::getuid() } }