From f5f8cf08e0836524ebdafedd9ad9b49a53b7c91f Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 31 Mar 2024 21:08:01 +0200 Subject: [PATCH 1/4] hostid: return correct exit code on error --- src/uu/hostid/src/hostid.rs | 2 +- tests/by-util/test_hostid.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/uu/hostid/src/hostid.rs b/src/uu/hostid/src/hostid.rs index a5c18d075..157cfc420 100644 --- a/src/uu/hostid/src/hostid.rs +++ b/src/uu/hostid/src/hostid.rs @@ -19,7 +19,7 @@ extern "C" { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - uu_app().get_matches_from(args); + uu_app().try_get_matches_from(args)?; hostid(); Ok(()) } diff --git a/tests/by-util/test_hostid.rs b/tests/by-util/test_hostid.rs index e9336116b..7525f5e08 100644 --- a/tests/by-util/test_hostid.rs +++ b/tests/by-util/test_hostid.rs @@ -10,3 +10,12 @@ fn test_normal() { let re = Regex::new(r"^[0-9a-f]{8}").unwrap(); new_ucmd!().succeeds().stdout_matches(&re); } + +#[test] +fn test_invalid_flag() { + new_ucmd!() + .arg("--invalid-argument") + .fails() + .no_stdout() + .code_is(1); +} From 95273417146574fdbe10413f12128c396131445c Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 31 Mar 2024 21:19:24 +0200 Subject: [PATCH 2/4] pr: return correct exit code on error --- src/uu/pr/src/pr.rs | 8 +------- tests/by-util/test_pr.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index 010183d31..aba71c341 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -386,13 +386,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let opt_args = recreate_arguments(&args); let mut command = uu_app(); - let matches = match command.try_get_matches_from_mut(opt_args) { - Ok(m) => m, - Err(e) => { - e.print()?; - return Ok(()); - } - }; + let matches = command.try_get_matches_from_mut(opt_args)?; let mut files = matches .get_many::(options::FILES) diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs index 823f0718f..c886b6452 100644 --- a/tests/by-util/test_pr.rs +++ b/tests/by-util/test_pr.rs @@ -43,6 +43,15 @@ fn valid_last_modified_template_vars(from: DateTime) -> Vec Date: Sun, 31 Mar 2024 23:08:51 +0200 Subject: [PATCH 3/4] tee: ensure that -h and --help are identical --- src/uu/tee/src/tee.rs | 10 ++++++++++ tests/by-util/test_tee.rs | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index b1443dbb9..69d66fd60 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -89,6 +89,16 @@ pub fn uu_app() -> Command { .override_usage(format_usage(USAGE)) .after_help(AFTER_HELP) .infer_long_args(true) + // Since we use value-specific help texts for "--output-error", clap's "short help" and "long help" differ. + // However, this is something that the GNU tests explicitly test for, so we *always* show the long help instead. + .disable_help_flag(true) + .arg( + Arg::new("--help") + .short('h') + .long("help") + .help("Print help") + .action(ArgAction::HelpLong) + ) .arg( Arg::new(options::APPEND) .long(options::APPEND) diff --git a/tests/by-util/test_tee.rs b/tests/by-util/test_tee.rs index 9ff4ea7dc..6058be076 100644 --- a/tests/by-util/test_tee.rs +++ b/tests/by-util/test_tee.rs @@ -18,6 +18,22 @@ fn test_invalid_arg() { new_ucmd!().arg("--definitely-invalid").fails().code_is(1); } +#[test] +fn test_short_help_is_long_help() { + // I can't believe that this test is necessary. + let help_short = new_ucmd!() + .arg("-h") + .succeeds() + .no_stderr() + .stdout_str() + .to_owned(); + new_ucmd!() + .arg("--help") + .succeeds() + .no_stderr() + .stdout_is(help_short); +} + #[test] fn test_tee_processing_multiple_operands() { // POSIX says: "Processing of at least 13 file operands shall be supported." From 120f0312e7c5312878b751f812e4b5a0b39c36ae Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 31 Mar 2024 23:10:54 +0200 Subject: [PATCH 4/4] util: undo custom GNU exit codes, reduce number of special cases --- util/build-gnu.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 6a4b09f89..8bdadddcf 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -279,14 +279,11 @@ sed -i "s|\$PACKAGE_VERSION|[0-9]*|g" tests/rm/fail-2eperm.sh tests/mv/sticky-to # with the option -/ is used, clap is returning a better error than GNU's. Adjust the GNU test sed -i -e "s~ grep \" '\*/'\*\" err || framework_failure_~ grep \" '*-/'*\" err || framework_failure_~" tests/misc/usage_vs_getopt.sh sed -i -e "s~ sed -n \"1s/'\\\/'/'OPT'/p\" < err >> pat || framework_failure_~ sed -n \"1s/'-\\\/'/'OPT'/p\" < err >> pat || framework_failure_~" tests/misc/usage_vs_getopt.sh -# Ignore some binaries (not built) -# And change the default error code to 2 -# see issue #3331 (clap limitation). -# Upstream returns 1 for most of the program. We do for cp, truncate & pr -# So, keep it as it -sed -i -e "s/rcexp=1$/rcexp=2\n case \"\$prg\" in chcon|runcon) return;; esac/" -e "s/rcexp=125 ;;/rcexp=2 ;;\ncp|truncate|pr) rcexp=1;;/" tests/misc/usage_vs_getopt.sh +# Ignore runcon, it needs some extra attention +# For all other tools, we want drop-in compatibility, and that includes the exit code. +sed -i -e "s/rcexp=1$/rcexp=1\n case \"\$prg\" in runcon|stdbuf) return;; esac/" tests/misc/usage_vs_getopt.sh # GNU has option=[SUFFIX], clap is -sed -i -e "s/cat opts/sed -i -e \"s| <.\*>$||g\" opts/" tests/misc/usage_vs_getopt.sh +sed -i -e "s/cat opts/sed -i -e \"s| <.\*$||g\" opts/" tests/misc/usage_vs_getopt.sh # for some reasons, some stuff are duplicated, strip that sed -i -e "s/provoked error./provoked error\ncat pat |sort -u > pat/" tests/misc/usage_vs_getopt.sh