From 44981cab0123dfbb92476f3e1c07aaefcbb81d33 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 14 Aug 2021 13:50:53 +0200 Subject: [PATCH] refactor/uucore ~ correct implementation of executable!() for multicall - Use an atomic bool to track whether the utility name is the second or the first argument. - Add tests --- src/bin/coreutils.rs | 1 + src/uu/base32/src/base32.rs | 6 +-- src/uu/base64/src/base64.rs | 4 +- src/uucore/src/lib/lib.rs | 9 ++++ src/uucore/src/lib/macros.rs | 59 ++++++++++---------------- tests/test_util_name.rs | 81 ++++++++++++++++++++++++++++++++++++ 6 files changed, 117 insertions(+), 43 deletions(-) create mode 100644 tests/test_util_name.rs diff --git a/src/bin/coreutils.rs b/src/bin/coreutils.rs index 3e8df57f7..477931dbd 100644 --- a/src/bin/coreutils.rs +++ b/src/bin/coreutils.rs @@ -70,6 +70,7 @@ fn main() { Some(OsString::from(*util)) } else { // unmatched binary name => regard as multi-binary container and advance argument list + uucore::set_utility_is_second_arg(); args.next() }; diff --git a/src/uu/base32/src/base32.rs b/src/uu/base32/src/base32.rs index a93b4e18b..4d6c634e5 100644 --- a/src/uu/base32/src/base32.rs +++ b/src/uu/base32/src/base32.rs @@ -38,7 +38,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let name = util_name!(); let config_result: Result = - base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); + base_common::parse_base_cmd_args(args, &name, VERSION, ABOUT, &usage); let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); // Create a reference to stdin so we can return a locked stdin from @@ -52,12 +52,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - name, + &name, ); 0 } pub fn uu_app() -> App<'static, 'static> { - base_common::base_app(util_name!(), VERSION, ABOUT) + base_common::base_app(&util_name!(), VERSION, ABOUT) } diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index b53ec32e9..689940a07 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -38,7 +38,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let usage = usage(); let name = util_name!(); let config_result: Result = - base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); + base_common::parse_base_cmd_args(args, &name, VERSION, ABOUT, &usage); let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); // Create a reference to stdin so we can return a locked stdin from @@ -52,7 +52,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - name, + &name, ); 0 diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index aa96a1086..8920b226d 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -77,6 +77,15 @@ pub use crate::features::wide; //## core functions use std::ffi::OsString; +use std::sync::atomic::Ordering; + +pub fn get_utility_is_second_arg() -> bool { + crate::macros::UTILITY_IS_SECOND_ARG.load(Ordering::SeqCst) +} + +pub fn set_utility_is_second_arg() { + crate::macros::UTILITY_IS_SECOND_ARG.store(true, Ordering::SeqCst) +} pub enum InvalidEncodingHandling { Ignore, diff --git a/src/uucore/src/lib/macros.rs b/src/uucore/src/lib/macros.rs index df3834f7e..72826be5f 100644 --- a/src/uucore/src/lib/macros.rs +++ b/src/uucore/src/lib/macros.rs @@ -1,3 +1,5 @@ +use std::sync::atomic::AtomicBool; + // This file is part of the uutils coreutils package. // // (c) Alex Lyon @@ -5,40 +7,22 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +/// Whether we were called as a multicall binary ("coreutils ") +pub static UTILITY_IS_SECOND_ARG: AtomicBool = AtomicBool::new(false); + /// Get the executable path (as `OsString`). #[macro_export] macro_rules! executable_os( () => ({ - &uucore::args_os().next().unwrap() + $crate::args_os().next().unwrap() }) ); -/// Get the executable path (as `String`; lossless). +/// Get the executable path (as `String`). #[macro_export] macro_rules! executable( () => ({ - let exe = match $crate::executable_os!().to_str() { - // * UTF-8 - Some(s) => s.to_string(), - // * "lossless" debug format (if/when `executable_os!()` is not well-formed UTF-8) - None => format!("{:?}", $crate::executable_os!()) - }; - &exe.to_owned() - }) -); - -/// Get the executable name. -#[macro_export] -macro_rules! executable_name( - () => ({ - let stem = &std::path::Path::new($crate::executable_os!()).file_stem().unwrap().to_owned(); - let exe = match stem.to_str() { - // * UTF-8 - Some(s) => s.to_string(), - // * "lossless" debug format (if/when `executable_os!()` is not well-formed UTF-8) - None => format!("{:?}", stem) - }; - &exe.to_owned() + $crate::executable_os!().to_string_lossy().to_string() }) ); @@ -46,13 +30,11 @@ macro_rules! executable_name( #[macro_export] macro_rules! util_name( () => ({ - let crate_name = env!("CARGO_PKG_NAME"); - let name = if crate_name.starts_with("uu_") { - &crate_name[3..] + if $crate::get_utility_is_second_arg() { + $crate::args_os().nth(1).unwrap().to_string_lossy().to_string() } else { - crate_name - }; - &name.to_owned() + $crate::executable!() + } }) ); @@ -62,14 +44,15 @@ macro_rules! util_name( #[macro_export] macro_rules! execution_phrase( () => ({ - let exe = if (executable_name!() == util_name!()) { - executable!().to_string() - } else { - format!("{} {}", executable!(), util_name!()) - .as_str() - .to_owned() - }; - &exe.to_owned() + if $crate::get_utility_is_second_arg() { + $crate::args_os() + .take(2) + .map(|os_str| os_str.to_string_lossy().to_string()) + .collect::>() + .join(" ") + } else { + $crate::executable!() + } }) ); diff --git a/tests/test_util_name.rs b/tests/test_util_name.rs new file mode 100644 index 000000000..640fecd44 --- /dev/null +++ b/tests/test_util_name.rs @@ -0,0 +1,81 @@ +mod common; + +use common::util::TestScenario; + +#[test] +#[cfg(feature = "ls")] +fn execution_phrase_double() { + use std::process::Command; + + let scenario = TestScenario::new("ls"); + let output = Command::new(&scenario.bin_path) + .arg("ls") + .arg("--some-invalid-arg") + .output() + .unwrap(); + assert!(String::from_utf8(output.stderr) + .unwrap() + .contains(&format!("USAGE:\n {} ls", scenario.bin_path.display(),))); +} + +#[test] +#[cfg(feature = "ls")] +fn execution_phrase_single() { + use std::process::Command; + + let scenario = TestScenario::new("ls"); + std::fs::copy(scenario.bin_path, scenario.fixtures.plus("uu-ls")).unwrap(); + let output = Command::new(scenario.fixtures.plus("uu-ls")) + .arg("--some-invalid-arg") + .output() + .unwrap(); + assert!(String::from_utf8(output.stderr).unwrap().contains(&format!( + "USAGE:\n {}", + scenario.fixtures.plus("uu-ls").display() + ))); +} + +#[test] +#[cfg(feature = "sort")] +fn util_name_double() { + use std::{ + io::Write, + process::{Command, Stdio}, + }; + + let scenario = TestScenario::new("sort"); + let mut child = Command::new(&scenario.bin_path) + .arg("sort") + .stdin(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + // input invalid utf8 to cause an error + child.stdin.take().unwrap().write_all(&[255]).unwrap(); + let output = child.wait_with_output().unwrap(); + assert!(String::from_utf8(output.stderr).unwrap().contains("sort: ")); +} + +#[test] +#[cfg(feature = "sort")] +fn util_name_single() { + use std::{ + io::Write, + process::{Command, Stdio}, + }; + + let scenario = TestScenario::new("sort"); + std::fs::copy(scenario.bin_path, scenario.fixtures.plus("uu-sort")).unwrap(); + let mut child = Command::new(scenario.fixtures.plus("uu-sort")) + .stdin(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + // input invalid utf8 to cause an error + child.stdin.take().unwrap().write_all(&[255]).unwrap(); + let output = child.wait_with_output().unwrap(); + assert!(String::from_utf8(output.stderr).unwrap().contains(&format!( + "{}: ", + scenario.fixtures.plus("uu-sort").display() + ))); +}