From d6a0b3c9206201e865c6f2b3c5b304fb266df104 Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Sun, 30 Jan 2022 17:15:29 -0500 Subject: [PATCH 01/46] Allow echo with escapes to work with \0 Testing with gecho on macos outputs a nul character for a \0 --- src/uu/echo/src/echo.rs | 5 +---- tests/by-util/test_echo.rs | 12 ++++++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 35606be71..1cb63fe93 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -88,10 +88,7 @@ fn print_escaped(input: &str, mut output: impl Write) -> io::Result { start = 0; next }), - '0' => parse_code(&mut iter, 8, 3, 3).unwrap_or_else(|| { - start = 0; - next - }), + '0' => parse_code(&mut iter, 8, 3, 3).unwrap_or('\0'), _ => { start = 0; next diff --git a/tests/by-util/test_echo.rs b/tests/by-util/test_echo.rs index 09ed9658f..401e16706 100644 --- a/tests/by-util/test_echo.rs +++ b/tests/by-util/test_echo.rs @@ -138,11 +138,19 @@ fn test_escape_short_octal() { } #[test] -fn test_escape_no_octal() { +fn test_escape_nul() { new_ucmd!() .args(&["-e", "foo\\0 bar"]) .succeeds() - .stdout_only("foo\\0 bar\n"); + .stdout_only("foo\0 bar\n"); +} + +#[test] +fn test_escape_octal_invalid_digit() { + new_ucmd!() + .args(&["-e", "foo\\08 bar"]) + .succeeds() + .stdout_only("foo\u{0}8 bar\n"); } #[test] From 184b65df206efce4a2f0289814b4a0f34ac83c92 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 31 Jan 2022 12:10:57 +0100 Subject: [PATCH 02/46] uucore: allow backup suffix with hyphen value --- src/uucore/src/lib/mods/backup_control.rs | 10 +++++++++ tests/by-util/test_cp.rs | 18 +++++++++++++++ tests/by-util/test_install.rs | 25 +++++++++++++++++++++ tests/by-util/test_ln.rs | 27 +++++++++++++++++++++++ tests/by-util/test_mv.rs | 21 ++++++++++++++++++ 5 files changed, 101 insertions(+) diff --git a/src/uucore/src/lib/mods/backup_control.rs b/src/uucore/src/lib/mods/backup_control.rs index e14716591..a2753b964 100644 --- a/src/uucore/src/lib/mods/backup_control.rs +++ b/src/uucore/src/lib/mods/backup_control.rs @@ -231,6 +231,7 @@ pub mod arguments { .help("override the usual backup suffix") .takes_value(true) .value_name("SUFFIX") + .allow_hyphen_values(true) } } @@ -618,4 +619,13 @@ mod tests { assert_eq!(result, BackupMode::SimpleBackup); env::remove_var(ENV_VERSION_CONTROL); } + + #[test] + fn test_suffix_takes_hyphen_value() { + let _dummy = TEST_MUTEX.lock().unwrap(); + let matches = make_app().get_matches_from(vec!["app", "-b", "--suffix", "-v"]); + + let result = determine_backup_suffix(&matches); + assert_eq!(result, "-v"); + } } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 92637dfbe..e9b149ede 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -385,6 +385,24 @@ fn test_cp_arg_suffix() { ); } +#[test] +fn test_cp_arg_suffix_hyphen_value() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg(TEST_HELLO_WORLD_SOURCE) + .arg("-b") + .arg("--suffix") + .arg("-v") + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&*format!("{}-v", TEST_HOW_ARE_YOU_SOURCE)), + "How are you?\n" + ); +} + #[test] fn test_cp_custom_backup_suffix_via_env() { let (at, mut ucmd) = at_and_ucmd!(); diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 97169f934..23bebf224 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -815,6 +815,31 @@ fn test_install_backup_short_custom_suffix() { assert!(at.file_exists(&format!("{}{}", file_b, suffix))); } +#[test] +fn test_install_backup_short_custom_suffix_hyphen_value() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_custom_suffix_file_a"; + let file_b = "test_install_backup_custom_suffix_file_b"; + let suffix = "-v"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("-b") + .arg(format!("--suffix={}", suffix)) + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(&format!("{}{}", file_b, suffix))); +} + #[test] fn test_install_backup_custom_suffix_via_env() { let scene = TestScenario::new(util_name!()); diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index 9fa73c0bc..a2a31464f 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -180,6 +180,33 @@ fn test_symlink_custom_backup_suffix() { assert_eq!(at.resolve_link(backup), file); } +#[test] +fn test_symlink_custom_backup_suffix_hyphen_value() { + let (at, mut ucmd) = at_and_ucmd!(); + let file = "test_symlink_custom_backup_suffix"; + let link = "test_symlink_custom_backup_suffix_link"; + let suffix = "-v"; + + at.touch(file); + at.symlink_file(file, link); + assert!(at.file_exists(file)); + assert!(at.is_symlink(link)); + assert_eq!(at.resolve_link(link), file); + + let arg = &format!("--suffix={}", suffix); + ucmd.args(&["-b", arg, "-s", file, link]) + .succeeds() + .no_stderr(); + assert!(at.file_exists(file)); + + assert!(at.is_symlink(link)); + assert_eq!(at.resolve_link(link), file); + + let backup = &format!("{}{}", link, suffix); + assert!(at.is_symlink(backup)); + assert_eq!(at.resolve_link(backup), file); +} + #[test] fn test_symlink_backup_numbering() { let (at, mut ucmd) = at_and_ucmd!(); diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 89f4043f8..a0bd0209d 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -340,6 +340,27 @@ fn test_mv_custom_backup_suffix() { assert!(at.file_exists(&format!("{}{}", file_b, suffix))); } +#[test] +fn test_mv_custom_backup_suffix_hyphen_value() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_custom_backup_suffix_file_a"; + let file_b = "test_mv_custom_backup_suffix_file_b"; + let suffix = "-v"; + + at.touch(file_a); + at.touch(file_b); + ucmd.arg("-b") + .arg(format!("--suffix={}", suffix)) + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(!at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(&format!("{}{}", file_b, suffix))); +} + #[test] fn test_mv_custom_backup_suffix_via_env() { let (at, mut ucmd) = at_and_ucmd!(); From 4f8d1c5fcfe1bff7971047e05895e7f896c675eb Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Sun, 30 Jan 2022 21:25:09 +0100 Subject: [PATCH 03/46] add additional lints --- .cargo/config | 9 +++++++++ src/bin/coreutils.rs | 8 ++++---- src/uu/chroot/src/chroot.rs | 6 +++--- src/uu/du/src/du.rs | 2 +- src/uu/env/src/env.rs | 1 + src/uu/kill/src/kill.rs | 6 +++--- src/uu/split/src/number.rs | 24 ++++++++++++------------ src/uu/split/src/split.rs | 6 +++--- src/uu/tail/src/platform/unix.rs | 1 + src/uu/tail/src/platform/windows.rs | 4 ++-- src/uucore/src/lib/features/fsext.rs | 8 ++++---- src/uucore/src/lib/features/wide.rs | 4 ++-- tests/by-util/test_du.rs | 6 +++--- tests/by-util/test_env.rs | 2 +- 14 files changed, 49 insertions(+), 38 deletions(-) diff --git a/.cargo/config b/.cargo/config index 58e1381b1..0a8fd3d00 100644 --- a/.cargo/config +++ b/.cargo/config @@ -1,2 +1,11 @@ [target.x86_64-unknown-redox] linker = "x86_64-unknown-redox-gcc" + +[target.'cfg(feature = "cargo-clippy")'] +rustflags = [ + "-Wclippy::use_self", + "-Wclippy::needless_pass_by_value", + "-Wclippy::semicolon_if_nothing_returned", + "-Wclippy::single_char_pattern", + "-Wclippy::explicit_iter_loop", +] diff --git a/src/bin/coreutils.rs b/src/bin/coreutils.rs index 901139edc..fcd86c93f 100644 --- a/src/bin/coreutils.rs +++ b/src/bin/coreutils.rs @@ -87,7 +87,7 @@ fn main() { }; if util == "completion" { - gen_completions(args, utils); + gen_completions(args, &utils); } match utils.get(util) { @@ -132,7 +132,7 @@ fn main() { /// Prints completions for the utility in the first parameter for the shell in the second parameter to stdout fn gen_completions( args: impl Iterator, - util_map: UtilityMap, + util_map: &UtilityMap, ) -> ! { let all_utilities: Vec<_> = std::iter::once("coreutils") .chain(util_map.keys().copied()) @@ -168,9 +168,9 @@ fn gen_completions( process::exit(0); } -fn gen_coreutils_app(util_map: UtilityMap) -> App<'static> { +fn gen_coreutils_app(util_map: &UtilityMap) -> App<'static> { let mut app = App::new("coreutils"); - for (_, (_, sub_app)) in &util_map { + for (_, (_, sub_app)) in util_map { app = app.subcommand(sub_app()); } app diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index 179880b14..f656ed77d 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -201,12 +201,12 @@ fn set_main_group(group: &str) -> UResult<()> { } #[cfg(any(target_vendor = "apple", target_os = "freebsd"))] -fn set_groups(groups: Vec) -> libc::c_int { +fn set_groups(groups: &[libc::gid_t]) -> libc::c_int { unsafe { setgroups(groups.len() as libc::c_int, groups.as_ptr()) } } #[cfg(target_os = "linux")] -fn set_groups(groups: Vec) -> libc::c_int { +fn set_groups(groups: &[libc::gid_t]) -> libc::c_int { unsafe { setgroups(groups.len() as libc::size_t, groups.as_ptr()) } } @@ -220,7 +220,7 @@ fn set_groups_from_str(groups: &str) -> UResult<()> { }; groups_vec.push(gid); } - let err = set_groups(groups_vec); + let err = set_groups(&groups_vec); if err != 0 { return Err(ChrootError::SetGroupsFailed(Error::last_os_error()).into()); } diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 7629aba69..e75210ef5 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -144,7 +144,7 @@ impl Stat { #[cfg(windows)] let file_info = get_file_info(&path); #[cfg(windows)] - Ok(Stat { + Ok(Self { path, is_dir: metadata.is_dir(), size: metadata.len(), diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index 677ffe1bf..c0e94a578 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -104,6 +104,7 @@ fn load_config_file(opts: &mut Options) -> UResult<()> { } #[cfg(not(windows))] +#[allow(clippy::ptr_arg)] fn build_command<'a, 'b>(args: &'a mut Vec<&'b str>) -> (Cow<'b, str>, &'a [&'b str]) { let progname = Cow::from(args[0]); (progname, &args[1..]) diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index 02dc91dbf..413a183a8 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -74,7 +74,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { table(); Ok(()) } - Mode::List => list(pids_or_signals.get(0).cloned()), + Mode::List => list(pids_or_signals.get(0)), } } @@ -168,9 +168,9 @@ fn print_signals() { println!(); } -fn list(arg: Option) -> UResult<()> { +fn list(arg: Option<&String>) -> UResult<()> { match arg { - Some(ref x) => print_signal(x), + Some(x) => print_signal(x), None => { print_signals(); Ok(()) diff --git a/src/uu/split/src/number.rs b/src/uu/split/src/number.rs index b2c402716..ef3ccbc4b 100644 --- a/src/uu/split/src/number.rs +++ b/src/uu/split/src/number.rs @@ -96,8 +96,8 @@ impl Number { #[allow(dead_code)] fn digits(&self) -> &Vec { match self { - Number::FixedWidth(number) => &number.digits, - Number::DynamicWidth(number) => &number.digits, + Self::FixedWidth(number) => &number.digits, + Self::DynamicWidth(number) => &number.digits, } } @@ -136,8 +136,8 @@ impl Number { /// ``` pub fn increment(&mut self) -> Result<(), Overflow> { match self { - Number::FixedWidth(number) => number.increment(), - Number::DynamicWidth(number) => number.increment(), + Self::FixedWidth(number) => number.increment(), + Self::DynamicWidth(number) => number.increment(), } } } @@ -145,8 +145,8 @@ impl Number { impl Display for Number { fn fmt(&self, f: &mut Formatter) -> fmt::Result { match self { - Number::FixedWidth(number) => number.fmt(f), - Number::DynamicWidth(number) => number.fmt(f), + Self::FixedWidth(number) => number.fmt(f), + Self::DynamicWidth(number) => number.fmt(f), } } } @@ -183,8 +183,8 @@ pub struct FixedWidthNumber { impl FixedWidthNumber { /// Instantiate a number of the given radix and width. - pub fn new(radix: u8, width: usize) -> FixedWidthNumber { - FixedWidthNumber { + pub fn new(radix: u8, width: usize) -> Self { + Self { radix, digits: vec![0; width], } @@ -286,8 +286,8 @@ impl DynamicWidthNumber { /// /// This associated function returns a new instance of the struct /// with the given radix and a width of two digits, both 0. - pub fn new(radix: u8) -> DynamicWidthNumber { - DynamicWidthNumber { + pub fn new(radix: u8) -> Self { + Self { radix, digits: vec![0, 0], } @@ -404,7 +404,7 @@ mod tests { fn num(n: usize) -> Number { let mut number = Number::DynamicWidth(DynamicWidthNumber::new(26)); for _ in 0..n { - number.increment().unwrap() + number.increment().unwrap(); } number } @@ -428,7 +428,7 @@ mod tests { fn num(n: usize) -> Number { let mut number = Number::DynamicWidth(DynamicWidthNumber::new(10)); for _ in 0..n { - number.increment().unwrap() + number.increment().unwrap(); } number } diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index 23eb24768..a05959810 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -62,7 +62,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .override_usage(&usage[..]) .after_help(&long_usage[..]) .get_matches_from(args); - let settings = Settings::from(matches)?; + let settings = Settings::from(&matches)?; split(&settings) } @@ -232,7 +232,7 @@ struct Settings { impl Settings { /// Parse a strategy from the command-line arguments. - fn from(matches: ArgMatches) -> UResult { + fn from(matches: &ArgMatches) -> UResult { let result = Self { suffix_length: matches .value_of(OPT_SUFFIX_LENGTH) @@ -242,7 +242,7 @@ impl Settings { numeric_suffix: matches.occurrences_of(OPT_NUMERIC_SUFFIXES) > 0, additional_suffix: matches.value_of(OPT_ADDITIONAL_SUFFIX).unwrap().to_owned(), verbose: matches.occurrences_of("verbose") > 0, - strategy: Strategy::from(&matches)?, + strategy: Strategy::from(matches)?, input: matches.value_of(ARG_INPUT).unwrap().to_owned(), prefix: matches.value_of(ARG_PREFIX).unwrap().to_owned(), filter: matches.value_of(OPT_FILTER).map(|s| s.to_owned()), diff --git a/src/uu/tail/src/platform/unix.rs b/src/uu/tail/src/platform/unix.rs index e7f75c31e..e01d5e444 100644 --- a/src/uu/tail/src/platform/unix.rs +++ b/src/uu/tail/src/platform/unix.rs @@ -30,6 +30,7 @@ impl ProcessChecker { } // Borrowing mutably to be aligned with Windows implementation + #[allow(clippy::wrong_self_convention)] pub fn is_dead(&mut self) -> bool { unsafe { libc::kill(self.pid, 0) != 0 && get_errno() != libc::EPERM } } diff --git a/src/uu/tail/src/platform/windows.rs b/src/uu/tail/src/platform/windows.rs index 7faa872e6..c63040a2a 100644 --- a/src/uu/tail/src/platform/windows.rs +++ b/src/uu/tail/src/platform/windows.rs @@ -24,11 +24,11 @@ pub struct ProcessChecker { } impl ProcessChecker { - pub fn new(process_id: self::Pid) -> ProcessChecker { + pub fn new(process_id: self::Pid) -> Self { #[allow(non_snake_case)] let FALSE = 0i32; let h = unsafe { OpenProcess(SYNCHRONIZE, FALSE, process_id as DWORD) }; - ProcessChecker { + Self { dead: h.is_null(), handle: h, } diff --git a/src/uucore/src/lib/features/fsext.rs b/src/uucore/src/lib/features/fsext.rs index d1e623757..a3b05dff8 100644 --- a/src/uucore/src/lib/features/fsext.rs +++ b/src/uucore/src/lib/features/fsext.rs @@ -238,7 +238,7 @@ impl MountInfo { } } #[cfg(windows)] - fn new(mut volume_name: String) -> Option { + fn new(mut volume_name: String) -> Option { let mut dev_name_buf = [0u16; MAX_PATH]; volume_name.pop(); unsafe { @@ -289,7 +289,7 @@ impl MountInfo { } else { None }; - let mut mn_info = MountInfo { + let mut mn_info = Self { dev_id: volume_name, dev_name, fs_type: fs_type.unwrap_or_else(|| "".to_string()), @@ -319,7 +319,7 @@ use std::ffi::CStr; ))] impl From for MountInfo { fn from(statfs: StatFs) -> Self { - let mut info = MountInfo { + let mut info = Self { dev_id: "".to_string(), dev_name: unsafe { // spell-checker:disable-next-line @@ -553,7 +553,7 @@ impl FsUsage { } let bytes_per_cluster = sectors_per_cluster as u64 * bytes_per_sector as u64; - FsUsage { + Self { // f_bsize File system block size. blocksize: bytes_per_cluster as u64, // f_blocks - Total number of blocks on the file system, in units of f_frsize. diff --git a/src/uucore/src/lib/features/wide.rs b/src/uucore/src/lib/features/wide.rs index 49ce575a7..6b9368f50 100644 --- a/src/uucore/src/lib/features/wide.rs +++ b/src/uucore/src/lib/features/wide.rs @@ -27,10 +27,10 @@ pub trait FromWide { fn from_wide_null(wide: &[u16]) -> Self; } impl FromWide for String { - fn from_wide(wide: &[u16]) -> String { + fn from_wide(wide: &[u16]) -> Self { OsString::from_wide(wide).to_string_lossy().into_owned() } - fn from_wide_null(wide: &[u16]) -> String { + fn from_wide_null(wide: &[u16]) -> Self { let len = wide.iter().take_while(|&&c| c != 0).count(); OsString::from_wide(&wide[..len]) .to_string_lossy() diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index efc097773..b0506d071 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -179,15 +179,15 @@ fn test_du_hard_link() { #[cfg(target_vendor = "apple")] fn _du_hard_link(s: &str) { - assert_eq!(s, "12\tsubdir/links\n") + assert_eq!(s, "12\tsubdir/links\n"); } #[cfg(target_os = "windows")] fn _du_hard_link(s: &str) { - assert_eq!(s, "8\tsubdir/links\n") + assert_eq!(s, "8\tsubdir/links\n"); } #[cfg(target_os = "freebsd")] fn _du_hard_link(s: &str) { - assert_eq!(s, "16\tsubdir/links\n") + assert_eq!(s, "16\tsubdir/links\n"); } #[cfg(all( not(target_vendor = "apple"), diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index e1950f0df..3559e74cd 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -219,7 +219,7 @@ fn test_change_directory() { .args(&pwd) .succeeds() .stdout_move_str(); - assert_eq!(out.trim(), temporary_path.as_os_str()) + assert_eq!(out.trim(), temporary_path.as_os_str()); } #[test] From cd1b5c5748ab2613faa5c63c59ed415a9739c27d Mon Sep 17 00:00:00 2001 From: Sam Caldwell Date: Mon, 31 Jan 2022 22:08:59 -0700 Subject: [PATCH 04/46] [truncate] change cli error return code Exit with status code 1 for argument parsing errors in `truncate`. When `clap` encounters an error during argument parsing, it exits with status code 2. This causes some GNU tests to fail since they expect status code 1. --- src/uu/truncate/src/truncate.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 685363f8f..142524aad 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -6,6 +6,7 @@ // * file that was distributed with this source code. // spell-checker:ignore (ToDO) RFILE refsize rfilename fsize tsize +use clap; use clap::{crate_version, App, AppSettings, Arg}; use std::convert::TryFrom; use std::fs::{metadata, OpenOptions}; @@ -115,7 +116,16 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app() .override_usage(&usage[..]) .after_help(&long_usage[..]) - .get_matches_from(args); + .try_get_matches_from(args) + .unwrap_or_else(|e| { + e.print(); + match e.kind { + clap::ErrorKind::DisplayHelp | clap::ErrorKind::DisplayVersion => { + std::process::exit(0) + } + _ => std::process::exit(1), + } + }); let files: Vec = matches .values_of(options::ARG_FILES) From 6f24166c63a3aa001f9c06df8a93fc2f0ab211b7 Mon Sep 17 00:00:00 2001 From: Sam Caldwell Date: Mon, 31 Jan 2022 22:25:59 -0700 Subject: [PATCH 05/46] [truncate] handle unused_must_use warning --- src/uu/truncate/src/truncate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 142524aad..4850e592a 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -118,7 +118,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .after_help(&long_usage[..]) .try_get_matches_from(args) .unwrap_or_else(|e| { - e.print(); + e.print().expect("Error writing clap::Error"); match e.kind { clap::ErrorKind::DisplayHelp | clap::ErrorKind::DisplayVersion => { std::process::exit(0) From e1f7c774d811c4c3da6cd548bdc25c47a3183580 Mon Sep 17 00:00:00 2001 From: Sam Caldwell Date: Mon, 31 Jan 2022 22:59:10 -0700 Subject: [PATCH 06/46] Remove redundant import --- src/uu/truncate/src/truncate.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 4850e592a..12f413247 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -6,7 +6,6 @@ // * file that was distributed with this source code. // spell-checker:ignore (ToDO) RFILE refsize rfilename fsize tsize -use clap; use clap::{crate_version, App, AppSettings, Arg}; use std::convert::TryFrom; use std::fs::{metadata, OpenOptions}; From d762bebc1c1a16c3fbb0d8fef024d4d995f9db51 Mon Sep 17 00:00:00 2001 From: Linux User Date: Tue, 1 Feb 2022 06:55:11 +0000 Subject: [PATCH 07/46] update shell scripts according to shellcheck recommendations and minor cleanup --- .travis/redox-toolchain.sh | 2 +- util/GHA-delete-GNU-workflow-logs.sh | 20 +++++++++--------- util/build-code_coverage.sh | 18 +++++++++------- util/build-gnu.sh | 4 ++-- util/publish.sh | 31 ++++++++++++---------------- util/run-gnu-test.sh | 3 ++- util/show-code_coverage.sh | 7 +++---- util/show-utils.sh | 16 ++++++-------- util/update-version.sh | 7 +++++-- 9 files changed, 52 insertions(+), 56 deletions(-) diff --git a/.travis/redox-toolchain.sh b/.travis/redox-toolchain.sh index 83bc8fc45..d8b43b489 100755 --- a/.travis/redox-toolchain.sh +++ b/.travis/redox-toolchain.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh rustup target add x86_64-unknown-redox sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys AA12E97F0881517F diff --git a/util/GHA-delete-GNU-workflow-logs.sh b/util/GHA-delete-GNU-workflow-logs.sh index 19e3311d4..f7406b831 100755 --- a/util/GHA-delete-GNU-workflow-logs.sh +++ b/util/GHA-delete-GNU-workflow-logs.sh @@ -15,21 +15,21 @@ ME_parent_dir_abs="$(realpath -mP -- "${ME_parent_dir}")" # * `gh` available? unset GH -gh --version 1>/dev/null 2>&1 -if [ $? -eq 0 ]; then export GH="gh"; fi +if gh --version 1>/dev/null 2>&1; then + export GH="gh" +else + echo "ERR!: missing \`gh\` (see install instructions at )" 1>&2 +fi # * `jq` available? unset JQ -jq --version 1>/dev/null 2>&1 -if [ $? -eq 0 ]; then export JQ="jq"; fi +if jq --version 1>/dev/null 2>&1; then + export JQ="jq" +else + echo "ERR!: missing \`jq\` (install with \`sudo apt install jq\`)" 1>&2 +fi if [ -z "${GH}" ] || [ -z "${JQ}" ]; then - if [ -z "${GH}" ]; then - echo 'ERR!: missing `gh` (see install instructions at )' 1>&2 - fi - if [ -z "${JQ}" ]; then - echo 'ERR!: missing `jq` (install with `sudo apt install jq`)' 1>&2 - fi exit 1 fi diff --git a/util/build-code_coverage.sh b/util/build-code_coverage.sh index 7ad3165fe..b92b7eb48 100755 --- a/util/build-code_coverage.sh +++ b/util/build-code_coverage.sh @@ -8,12 +8,13 @@ FEATURES_OPTION="--features feat_os_unix" -ME_dir="$(dirname -- $(readlink -fm -- "$0"))" +ME_dir="$(dirname -- "$(readlink -fm -- "$0")")" REPO_main_dir="$(dirname -- "${ME_dir}")" -cd "${REPO_main_dir}" +cd "${REPO_main_dir}" && echo "[ \"$PWD\" ]" +#shellcheck disable=SC2086 UTIL_LIST=$("${ME_dir}"/show-utils.sh ${FEATURES_OPTION}) CARGO_INDIVIDUAL_PACKAGE_OPTIONS="" for UTIL in ${UTIL_LIST}; do @@ -30,10 +31,12 @@ export RUSTC_WRAPPER="" ## NOTE: RUSTC_WRAPPER=='sccache' breaks code covera export RUSTFLAGS="-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort" export RUSTDOCFLAGS="-Cpanic=abort" export RUSTUP_TOOLCHAIN="nightly-gnu" -cargo build ${FEATURES_OPTION} -cargo test --no-run ${FEATURES_OPTION} -cargo test --quiet ${FEATURES_OPTION} -cargo test --quiet ${FEATURES_OPTION} ${CARGO_INDIVIDUAL_PACKAGE_OPTIONS} +#shellcheck disable=SC2086 +{ cargo build ${FEATURES_OPTION} + cargo test --no-run ${FEATURES_OPTION} + cargo test --quiet ${FEATURES_OPTION} + cargo test --quiet ${FEATURES_OPTION} ${CARGO_INDIVIDUAL_PACKAGE_OPTIONS} +} export COVERAGE_REPORT_DIR if [ -z "${COVERAGE_REPORT_DIR}" ]; then COVERAGE_REPORT_DIR="${REPO_main_dir}/target/debug/coverage-nix"; fi @@ -47,8 +50,7 @@ mkdir -p "${COVERAGE_REPORT_DIR}" grcov . --output-type lcov --output-path "${COVERAGE_REPORT_DIR}/../lcov.info" --branch --ignore build.rs --ignore '/*' --ignore '[A-Za-z]:/*' --ignore 'C:/Users/*' --excl-br-line '^\s*((debug_)?assert(_eq|_ne)?!|#\[derive\()' # * build HTML # -- use `genhtml` if available for display of additional branch coverage information -genhtml --version 2>/dev/null 1>&2 -if [ $? -eq 0 ]; then +if genhtml --version 2>/dev/null 1>&2; then genhtml "${COVERAGE_REPORT_DIR}/../lcov.info" --output-directory "${COVERAGE_REPORT_DIR}" --branch-coverage --function-coverage | grep ": [0-9]" else grcov . --output-type html --output-path "${COVERAGE_REPORT_DIR}" --branch --ignore build.rs --ignore '/*' --ignore '[A-Za-z]:/*' --ignore 'C:/Users/*' --excl-br-line '^\s*((debug_)?assert(_eq|_ne)?!|#\[derive\()' diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 8b1e4925b..a52d42107 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -15,7 +15,7 @@ if test ! -d ../gnulib; then fi -pushd $(pwd) +pushd "$PWD" make PROFILE=release BUILDDIR="$PWD/target/release/" cp "${BUILDDIR}/install" "${BUILDDIR}/ginstall" # The GNU tests rename this script before running, to avoid confusion with the make target @@ -49,7 +49,7 @@ make -j "$(nproc)" # Used to be 36. Reduced to 20 to decrease the log size for i in {00..20} do - make tests/factor/t${i}.sh + make "tests/factor/t${i}.sh" done # strip the long stuff diff --git a/util/publish.sh b/util/publish.sh index ae171e39c..6f4d9f237 100755 --- a/util/publish.sh +++ b/util/publish.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh set -e ARG="" @@ -6,26 +6,21 @@ if test "$1" != "--do-it"; then ARG="--dry-run --allow-dirty" fi -cd src/uucore/ -cargo publish $ARG -cd - -sleep 2s - -cd src/uucore_procs/ -cargo publish $ARG -cd - -sleep 2s - -cd src/uu/stdbuf/src/libstdbuf/ -cargo publish $ARG -cd - -sleep 2s +for dir in src/uucore/ src/uucore_procs/ src/uu/stdbuf/src/libstdbuf/ ; do + ( cd "$dir" + #shellcheck disable=SC2086 + cargo publish $ARG + ) + sleep 2s +done PROGS=$(ls -1d src/uu/*/) for p in $PROGS; do - cd $p - cargo publish $ARG - cd - + ( cd "$p" + #shellcheck disable=SC2086 + cargo publish $ARG + ) done +#shellcheck disable=SC2086 cargo publish $ARG diff --git a/util/run-gnu-test.sh b/util/run-gnu-test.sh index 483fc1be9..ff61e636e 100755 --- a/util/run-gnu-test.sh +++ b/util/run-gnu-test.sh @@ -1,6 +1,6 @@ #!/bin/bash # spell-checker:ignore (env/vars) BUILDDIR GNULIB SUBDIRS -cd "$(dirname "${BASH_SOURCE[0]}")/../.." +cd "$(dirname -- "$(readlink -fm -- "$0")")/../.." set -e BUILDDIR="${PWD}/uutils/target/release" GNULIB_DIR="${PWD}/gnulib" @@ -13,4 +13,5 @@ if test -n "$1"; then export RUN_TEST="TESTS=$1" fi +#shellcheck disable=SC2086 timeout -sKILL 2h make -j "$(nproc)" check $RUN_TEST SUBDIRS=. RUN_EXPENSIVE_TESTS=yes RUN_VERY_EXPENSIVE_TESTS=yes VERBOSE=no || : # Kill after 4 hours in case something gets stuck in make diff --git a/util/show-code_coverage.sh b/util/show-code_coverage.sh index 365041434..6226d856b 100755 --- a/util/show-code_coverage.sh +++ b/util/show-code_coverage.sh @@ -2,15 +2,14 @@ # spell-checker:ignore (vars) OSID -ME_dir="$(dirname -- $(readlink -fm -- "$0"))" +ME_dir="$(dirname -- "$(readlink -fm -- "$0")")" REPO_main_dir="$(dirname -- "${ME_dir}")" export COVERAGE_REPORT_DIR="${REPO_main_dir}/target/debug/coverage-nix" -"${ME_dir}/build-code_coverage.sh" -if [ $? -ne 0 ]; then exit 1 ; fi +if ! "${ME_dir}/build-code_coverage.sh"; then exit 1 ; fi case ";$OSID_tags;" in - *";wsl;"* ) powershell.exe -c $(wslpath -w "${COVERAGE_REPORT_DIR}"/index.html) ;; + *";wsl;"* ) powershell.exe -c "$(wslpath -w "${COVERAGE_REPORT_DIR}"/index.html)" ;; * ) xdg-open --version >/dev/null 2>&1 && xdg-open "${COVERAGE_REPORT_DIR}"/index.html || echo "report available at '\"${COVERAGE_REPORT_DIR}\"/index.html'" ;; esac ; diff --git a/util/show-utils.sh b/util/show-utils.sh index b4a613d9b..f69b42678 100755 --- a/util/show-utils.sh +++ b/util/show-utils.sh @@ -15,17 +15,13 @@ default_utils="base32 base64 basename cat cksum comm cp cut date dircolors dirna project_main_dir="${ME_parent_dir_abs}" # printf 'project_main_dir="%s"\n' "${project_main_dir}" -cd "${project_main_dir}" +cd "${project_main_dir}" && # `jq` available? -unset JQ -jq --version 1>/dev/null 2>&1 -if [ $? -eq 0 ]; then export JQ="jq"; fi - -if [ -z "${JQ}" ]; then - echo 'WARN: missing `jq` (install with `sudo apt install jq`); falling back to default (only fully cross-platform) utility list' 1>&2 - echo $default_utils +if ! jq --version 1>/dev/null 2>&1; then + echo "WARN: missing \`jq\` (install with \`sudo apt install jq\`); falling back to default (only fully cross-platform) utility list" 1>&2 + echo "$default_utils" else - cargo metadata $* --format-version 1 | jq -r "[.resolve.nodes[] | { id: .id, deps: [.deps[] | { name:.name, pkg:.pkg }] }] | .[] | select(.id|startswith(\"coreutils\")) | [.deps[] | select((.name|startswith(\"uu_\")) or (.pkg|startswith(\"uu_\")))] | [.[].pkg | match(\"^\\\w+\";\"g\")] | [.[].string | sub(\"^uu_\"; \"\")] | sort | join(\" \")" - # cargo metadata $* --format-version 1 | jq -r "[.resolve.nodes[] | { id: .id, deps: [.deps[] | { name:.name, pkg:.pkg }] }] | .[] | select(.id|startswith(\"coreutils\")) | [.deps[] | select((.name|startswith(\"uu_\")) or (.pkg|startswith(\"uu_\")))] | [.[].pkg | match(\"^\\\w+\";\"g\")] | [.[].string] | sort | join(\" \")" + cargo metadata "$*" --format-version 1 | jq -r "[.resolve.nodes[] | { id: .id, deps: [.deps[] | { name:.name, pkg:.pkg }] }] | .[] | select(.id|startswith(\"coreutils\")) | [.deps[] | select((.name|startswith(\"uu_\")) or (.pkg|startswith(\"uu_\")))] | [.[].pkg | match(\"^\\\w+\";\"g\")] | [.[].string | sub(\"^uu_\"; \"\")] | sort | join(\" \")" + # cargo metadata "$*" --format-version 1 | jq -r "[.resolve.nodes[] | { id: .id, deps: [.deps[] | { name:.name, pkg:.pkg }] }] | .[] | select(.id|startswith(\"coreutils\")) | [.deps[] | select((.name|startswith(\"uu_\")) or (.pkg|startswith(\"uu_\")))] | [.[].pkg | match(\"^\\\w+\";\"g\")] | [.[].string] | sort | join(\" \")" fi diff --git a/util/update-version.sh b/util/update-version.sh index 503f65e52..62b130bda 100755 --- a/util/update-version.sh +++ b/util/update-version.sh @@ -1,10 +1,10 @@ -#!/bin/bash +#!/bin/sh # This is a stupid helper. I will mass replace all versions (including other crates) # So, it should be triple-checked # How to ship a new release: # 1) update this script -# 2) run it: bash util/update-version.sh +# 2) run it: sh util/update-version.sh # 3) Do a spot check with "git diff" # 4) cargo test --release --features unix # 5) Run util/publish.sh in dry mode (it will fail as packages needs more recent version of uucore) @@ -23,6 +23,7 @@ UUCORE_TO="0.0.11" PROGS=$(ls -1d src/uu/*/Cargo.toml src/uu/stdbuf/src/libstdbuf/Cargo.toml Cargo.toml src/uu/base64/Cargo.toml) # update the version of all programs +#shellcheck disable=SC2086 sed -i -e "s|version = \"$FROM\"|version = \"$TO\"|" $PROGS # Update uucore_procs @@ -35,6 +36,8 @@ sed -i -e "s|= { optional=true, version=\"$FROM\", package=\"uu_|= { optional=tr # Update uucore itself sed -i -e "s|version = \"$UUCORE_FROM\"|version = \"$UUCORE_TO\"|" src/uucore/Cargo.toml # Update crates using uucore +#shellcheck disable=SC2086 sed -i -e "s|uucore = { version=\">=$UUCORE_FROM\",|uucore = { version=\">=$UUCORE_TO\",|" $PROGS # Update crates using uucore_procs +#shellcheck disable=SC2086 sed -i -e "s|uucore_procs = { version=\">=$UUCORE_PROCS_FROM\",|uucore_procs = { version=\">=$UUCORE_PROCS_TO\",|" $PROGS From b29e219e4dc7fe162c83d0e0dec301da4fd9526c Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 1 Feb 2022 00:35:26 +0100 Subject: [PATCH 08/46] true: Rework to return true more often Now treats recognized command line options and ignores unrecognized command line options instead of returning a special exit status for them. There is one point of interest, which is related to an implementation detail in GNU `true`. It may return a non-true exit status (in particular EXIT_FAIL) if writing the diagnostics of a GNU specific option fails. For example `true --version > /dev/full` would fail and have exit status 1. This behavior was acknowledged in gnu in commit <9a6a486e6503520fd2581f2d3356b7149f1b225d>. No further justification provided for keeping this quirk. POSIX knows no such options, and requires an exit status of 0 in all cases. We replicate GNU here which is a consistency improvement over the prior implementation. Adds documentation to clarify the intended behavior more properly. --- src/uu/true/src/true.rs | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/uu/true/src/true.rs b/src/uu/true/src/true.rs index ff5b08e85..21b064e73 100644 --- a/src/uu/true/src/true.rs +++ b/src/uu/true/src/true.rs @@ -4,16 +4,41 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. +use clap::{App, AppSettings, ErrorKind}; +use std::io::Write; +use uucore::error::{set_exit_code, UResult}; -use clap::{App, AppSettings}; -use uucore::error::UResult; +static ABOUT: &str = " + Returns true, a successful exit status. + + Immediately returns with the exit status `0`, except when invoked with one of the recognized + options. In those cases it will try to write the help or version text. Any IO error during this + operation causes the program to return `1` instead. +"; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - uu_app().get_matches_from(args); + let app = uu_app(); + + if let Err(err) = app.try_get_matches_from(args) { + if let ErrorKind::DisplayHelp | ErrorKind::DisplayVersion = err.kind { + if let Err(print_fail) = err.print() { + // Try to display this error. + let _ = writeln!(std::io::stderr(), "{}: {}", uucore::util_name(), print_fail); + // Mirror GNU options. When failing to print warnings or version flags, then we exit + // with FAIL. This avoids allocation some error information which may result in yet + // other types of failure. + set_exit_code(1); + } + } + } + Ok(()) } pub fn uu_app<'a>() -> App<'a> { - App::new(uucore::util_name()).setting(AppSettings::InferLongArgs) + App::new(uucore::util_name()) + .version(clap::crate_version!()) + .about(ABOUT) + .setting(AppSettings::InferLongArgs) } From dcf177f9083b2f676b85049846f78a1ce642d602 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 1 Feb 2022 01:43:59 +0100 Subject: [PATCH 09/46] false: Align behavior to true and GNU --- src/uu/false/src/false.rs | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/uu/false/src/false.rs b/src/uu/false/src/false.rs index 324f90579..b304b4af6 100644 --- a/src/uu/false/src/false.rs +++ b/src/uu/false/src/false.rs @@ -4,16 +4,43 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. +use clap::{App, AppSettings, ErrorKind}; +use std::io::Write; +use uucore::error::{set_exit_code, UResult}; -use clap::App; -use uucore::error::UResult; +static ABOUT: &str = " + Returns false, an unsuccessful exit status. + + Immediately returns with the exit status `1`. When invoked with one of the recognized options it + will try to write the help or version text. Any IO error during this operation is diagnosed, yet + the program will also return `1`. +"; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - uu_app().get_matches_from(args); - Err(1.into()) + let app = uu_app(); + + // Mirror GNU options, always return `1`. In particular even the 'successful' cases of no-op, + // and the interrupted display of help and version should return `1`. Also, we return Ok in all + // paths to avoid the allocation of an error object, an operation that could, in theory, fail + // and unwind through the standard library allocation handling machinery. + set_exit_code(1); + + if let Err(err) = app.try_get_matches_from(args) { + if let ErrorKind::DisplayHelp | ErrorKind::DisplayVersion = err.kind { + if let Err(print_fail) = err.print() { + // Try to display this error. + let _ = writeln!(std::io::stderr(), "{}: {}", uucore::util_name(), print_fail); + } + } + } + + Ok(()) } pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) + .version(clap::crate_version!()) + .about(ABOUT) + .setting(AppSettings::InferLongArgs) } From 149399c1bf54f7c0df5558578e28bd82ad219581 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 1 Feb 2022 12:19:00 +0100 Subject: [PATCH 10/46] false,true: Add by-util tests for options --- tests/by-util/test_false.rs | 39 +++++++++++++++++++++++++++++++++++++ tests/by-util/test_true.rs | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/tests/by-util/test_false.rs b/tests/by-util/test_false.rs index bbabc7a52..366dd277b 100644 --- a/tests/by-util/test_false.rs +++ b/tests/by-util/test_false.rs @@ -1,6 +1,45 @@ use crate::common::util::*; +#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] +use std::fs::OpenOptions; #[test] fn test_exit_code() { new_ucmd!().fails(); } + +#[test] +fn test_version() { + new_ucmd!() + .args(&["--version"]) + .fails() + .stdout_contains("false"); +} + +#[test] +fn test_help() { + new_ucmd!() + .args(&["--help"]) + .fails() + .stdout_contains("false"); +} + +#[test] +fn test_short_options() { + for option in ["-h", "-v"] { + new_ucmd!().arg(option).fails().stdout_is(""); + } +} + +#[test] +#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] +fn test_full() { + for option in ["--version", "--help"] { + let dev_full = OpenOptions::new().write(true).open("/dev/full").unwrap(); + + new_ucmd!() + .arg(option) + .set_stdout(dev_full) + .fails() + .stderr_contains("No space left on device"); + } +} diff --git a/tests/by-util/test_true.rs b/tests/by-util/test_true.rs index 1d8622c96..d8ac2003b 100644 --- a/tests/by-util/test_true.rs +++ b/tests/by-util/test_true.rs @@ -1,6 +1,45 @@ use crate::common::util::*; +#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] +use std::fs::OpenOptions; #[test] fn test_exit_code() { new_ucmd!().succeeds(); } + +#[test] +fn test_version() { + new_ucmd!() + .args(&["--version"]) + .succeeds() + .stdout_contains("true"); +} + +#[test] +fn test_help() { + new_ucmd!() + .args(&["--help"]) + .succeeds() + .stdout_contains("true"); +} + +#[test] +fn test_short_options() { + for option in ["-h", "-v"] { + new_ucmd!().arg(option).succeeds().stdout_is(""); + } +} + +#[test] +#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] +fn test_full() { + for option in ["--version", "--help"] { + let dev_full = OpenOptions::new().write(true).open("/dev/full").unwrap(); + + new_ucmd!() + .arg(option) + .set_stdout(dev_full) + .fails() + .stderr_contains("No space left on device"); + } +} From c1e108933fe66853e0d081f13b657c3acee75cd6 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 1 Feb 2022 12:32:30 +0100 Subject: [PATCH 11/46] false,true: Align behavior of short flags to GNU --- src/uu/false/src/false.rs | 15 +++++++++++++-- src/uu/true/src/true.rs | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/uu/false/src/false.rs b/src/uu/false/src/false.rs index b304b4af6..0ebbb5be2 100644 --- a/src/uu/false/src/false.rs +++ b/src/uu/false/src/false.rs @@ -4,7 +4,7 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -use clap::{App, AppSettings, ErrorKind}; +use clap::{App, Arg, ArgSettings, ErrorKind}; use std::io::Write; use uucore::error::{set_exit_code, UResult}; @@ -42,5 +42,16 @@ pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) .version(clap::crate_version!()) .about(ABOUT) - .setting(AppSettings::InferLongArgs) + // Hide the default -V and -h for version and help. + // This requires us to overwrite short, not short_aliases. + .arg( + Arg::new("dummy-help") + .short('h') + .setting(ArgSettings::Hidden), + ) + .arg( + Arg::new("dummy-version") + .short('V') + .setting(ArgSettings::Hidden), + ) } diff --git a/src/uu/true/src/true.rs b/src/uu/true/src/true.rs index 21b064e73..20ee100b7 100644 --- a/src/uu/true/src/true.rs +++ b/src/uu/true/src/true.rs @@ -4,7 +4,7 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -use clap::{App, AppSettings, ErrorKind}; +use clap::{App, Arg, ArgSettings, ErrorKind}; use std::io::Write; use uucore::error::{set_exit_code, UResult}; @@ -40,5 +40,16 @@ pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) .version(clap::crate_version!()) .about(ABOUT) - .setting(AppSettings::InferLongArgs) + // Hide the default -V and -h for version and help. + // This requires us to overwrite short, not short_aliases. + .arg( + Arg::new("dummy-help") + .short('h') + .setting(ArgSettings::Hidden), + ) + .arg( + Arg::new("dummy-version") + .short('V') + .setting(ArgSettings::Hidden), + ) } From 23a544c485672743b9fa119e0c6614ea12706da3 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 1 Feb 2022 14:29:26 +0100 Subject: [PATCH 12/46] false,true: Implement custom help, version This avoids hacking around the short options of these command line arguments that have been introduced by clap. Additionally, we test and correctly handle the combination of both version and help. The GNU binary will ignore both arguments in this case while clap would perform the first one. A test for this edge case was added. --- src/uu/false/src/false.rs | 40 ++++++++++++++++++------------- src/uu/true/src/true.rs | 47 +++++++++++++++++++++---------------- tests/by-util/test_false.rs | 10 +++++++- tests/by-util/test_true.rs | 10 +++++++- 4 files changed, 69 insertions(+), 38 deletions(-) diff --git a/src/uu/false/src/false.rs b/src/uu/false/src/false.rs index 0ebbb5be2..8b487847d 100644 --- a/src/uu/false/src/false.rs +++ b/src/uu/false/src/false.rs @@ -4,7 +4,7 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -use clap::{App, Arg, ArgSettings, ErrorKind}; +use clap::{App, AppSettings, Arg}; use std::io::Write; use uucore::error::{set_exit_code, UResult}; @@ -18,7 +18,7 @@ static ABOUT: &str = " #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let app = uu_app(); + let mut app = uu_app(); // Mirror GNU options, always return `1`. In particular even the 'successful' cases of no-op, // and the interrupted display of help and version should return `1`. Also, we return Ok in all @@ -26,12 +26,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // and unwind through the standard library allocation handling machinery. set_exit_code(1); - if let Err(err) = app.try_get_matches_from(args) { - if let ErrorKind::DisplayHelp | ErrorKind::DisplayVersion = err.kind { - if let Err(print_fail) = err.print() { - // Try to display this error. - let _ = writeln!(std::io::stderr(), "{}: {}", uucore::util_name(), print_fail); - } + if let Ok(matches) = app.try_get_matches_from_mut(args) { + let error = if matches.index_of("help").is_some() { + app.print_long_help() + } else if matches.index_of("version").is_some() { + writeln!(std::io::stdout(), "{}", app.render_version()) + } else { + Ok(()) + }; + + // Try to display this error. + if let Err(print_fail) = error { + // Completely ignore any error here, no more failover and we will fail in any case. + let _ = writeln!(std::io::stderr(), "{}: {}", uucore::util_name(), print_fail); } } @@ -42,16 +49,17 @@ pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) .version(clap::crate_version!()) .about(ABOUT) - // Hide the default -V and -h for version and help. - // This requires us to overwrite short, not short_aliases. + // We provide our own help and version options, to ensure maximum compatibility with GNU. + .setting(AppSettings::DisableHelpFlag | AppSettings::DisableVersionFlag) .arg( - Arg::new("dummy-help") - .short('h') - .setting(ArgSettings::Hidden), + Arg::new("help") + .long("help") + .help("Print help information") + .exclusive(true), ) .arg( - Arg::new("dummy-version") - .short('V') - .setting(ArgSettings::Hidden), + Arg::new("version") + .long("version") + .help("Print version information"), ) } diff --git a/src/uu/true/src/true.rs b/src/uu/true/src/true.rs index 20ee100b7..c3026e684 100644 --- a/src/uu/true/src/true.rs +++ b/src/uu/true/src/true.rs @@ -4,7 +4,7 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -use clap::{App, Arg, ArgSettings, ErrorKind}; +use clap::{App, AppSettings, Arg}; use std::io::Write; use uucore::error::{set_exit_code, UResult}; @@ -18,18 +18,24 @@ static ABOUT: &str = " #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let app = uu_app(); + let mut app = uu_app(); - if let Err(err) = app.try_get_matches_from(args) { - if let ErrorKind::DisplayHelp | ErrorKind::DisplayVersion = err.kind { - if let Err(print_fail) = err.print() { - // Try to display this error. - let _ = writeln!(std::io::stderr(), "{}: {}", uucore::util_name(), print_fail); - // Mirror GNU options. When failing to print warnings or version flags, then we exit - // with FAIL. This avoids allocation some error information which may result in yet - // other types of failure. - set_exit_code(1); - } + if let Ok(matches) = app.try_get_matches_from_mut(args) { + let error = if matches.index_of("help").is_some() { + app.print_long_help() + } else if matches.index_of("version").is_some() { + writeln!(std::io::stdout(), "{}", app.render_version()) + } else { + Ok(()) + }; + + if let Err(print_fail) = error { + // Try to display this error. + let _ = writeln!(std::io::stderr(), "{}: {}", uucore::util_name(), print_fail); + // Mirror GNU options. When failing to print warnings or version flags, then we exit + // with FAIL. This avoids allocation some error information which may result in yet + // other types of failure. + set_exit_code(1); } } @@ -40,16 +46,17 @@ pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) .version(clap::crate_version!()) .about(ABOUT) - // Hide the default -V and -h for version and help. - // This requires us to overwrite short, not short_aliases. + // We provide our own help and version options, to ensure maximum compatibility with GNU. + .setting(AppSettings::DisableHelpFlag | AppSettings::DisableVersionFlag) .arg( - Arg::new("dummy-help") - .short('h') - .setting(ArgSettings::Hidden), + Arg::new("help") + .long("help") + .help("Print help information") + .exclusive(true), ) .arg( - Arg::new("dummy-version") - .short('V') - .setting(ArgSettings::Hidden), + Arg::new("version") + .long("version") + .help("Print version information"), ) } diff --git a/tests/by-util/test_false.rs b/tests/by-util/test_false.rs index 366dd277b..5ce64e7a8 100644 --- a/tests/by-util/test_false.rs +++ b/tests/by-util/test_false.rs @@ -25,11 +25,19 @@ fn test_help() { #[test] fn test_short_options() { - for option in ["-h", "-v"] { + for option in ["-h", "-V"] { new_ucmd!().arg(option).fails().stdout_is(""); } } +#[test] +fn test_conflict() { + new_ucmd!() + .args(&["--help", "--version"]) + .fails() + .stdout_is(""); +} + #[test] #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] fn test_full() { diff --git a/tests/by-util/test_true.rs b/tests/by-util/test_true.rs index d8ac2003b..aba32578b 100644 --- a/tests/by-util/test_true.rs +++ b/tests/by-util/test_true.rs @@ -25,11 +25,19 @@ fn test_help() { #[test] fn test_short_options() { - for option in ["-h", "-v"] { + for option in ["-h", "-V"] { new_ucmd!().arg(option).succeeds().stdout_is(""); } } +#[test] +fn test_conflict() { + new_ucmd!() + .args(&["--help", "--version"]) + .succeeds() + .stdout_is(""); +} + #[test] #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] fn test_full() { From dbbabedec6decfffe9aba62e58320cff530cbfb4 Mon Sep 17 00:00:00 2001 From: Tillmann Rendel Date: Tue, 1 Feb 2022 14:55:29 +0100 Subject: [PATCH 13/46] Allow comments in VS Code configuration Since JSON formally cannot contain comments, GitHub highlights comments in JSON files as errors, but the configuration files for VS Code in this repository contain comments. This commit configures GitHub to use a different syntax highlighter for the affected files. --- .vscode/.gitattributes | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .vscode/.gitattributes diff --git a/.vscode/.gitattributes b/.vscode/.gitattributes new file mode 100644 index 000000000..026e40131 --- /dev/null +++ b/.vscode/.gitattributes @@ -0,0 +1,2 @@ +# Configure GitHub to not mark comments in configuration files as errors +*.json linguist-language=JSON-with-Comments From 420045210c8d7ea316ebf8de236a81388f0e3698 Mon Sep 17 00:00:00 2001 From: Tillmann Rendel Date: Tue, 1 Feb 2022 15:27:59 +0100 Subject: [PATCH 14/46] Try moving .gitattributes to root --- .vscode/.gitattributes => .gitattributes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename .vscode/.gitattributes => .gitattributes (58%) diff --git a/.vscode/.gitattributes b/.gitattributes similarity index 58% rename from .vscode/.gitattributes rename to .gitattributes index 026e40131..73153b0d3 100644 --- a/.vscode/.gitattributes +++ b/.gitattributes @@ -1,2 +1,2 @@ # Configure GitHub to not mark comments in configuration files as errors -*.json linguist-language=JSON-with-Comments +.vscode/*.json linguist-language=JSON-with-Comments From 864b09c9b8ea419a47b39e42aea1a4c07639a865 Mon Sep 17 00:00:00 2001 From: Tillmann Rendel Date: Tue, 1 Feb 2022 19:47:49 +0100 Subject: [PATCH 15/46] vscode: only recommend rust-analyzer (#3020) Previously, both RLS and Rust-Analyzer were recommended, now we just recommend RA. --- .vscode/extensions.json | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 46b105d37..30b38bfa9 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -1,10 +1,8 @@ +// spell-checker:ignore (misc) matklad +// see for the documentation about the extensions.json format { - // spell-checker:ignore (misc) matklad - // see for the documentation about the extensions.json format "recommendations": [ // Rust language support. - "rust-lang.rust", - // Provides support for rust-analyzer: novel LSP server for the Rust programming language. "matklad.rust-analyzer", // `cspell` spell-checker support "streetsidesoftware.code-spell-checker" From c6d5eccf6c85420dd99fa2bca7501b292f0416f0 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 1 Feb 2022 19:53:25 +0100 Subject: [PATCH 16/46] false,true: Resolve formatting nit in About --- src/uu/false/src/false.rs | 10 +++++----- src/uu/true/src/true.rs | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/uu/false/src/false.rs b/src/uu/false/src/false.rs index 8b487847d..c6661dc35 100644 --- a/src/uu/false/src/false.rs +++ b/src/uu/false/src/false.rs @@ -8,12 +8,12 @@ use clap::{App, AppSettings, Arg}; use std::io::Write; use uucore::error::{set_exit_code, UResult}; -static ABOUT: &str = " - Returns false, an unsuccessful exit status. +static ABOUT: &str = "\ +Returns false, an unsuccessful exit status. - Immediately returns with the exit status `1`. When invoked with one of the recognized options it - will try to write the help or version text. Any IO error during this operation is diagnosed, yet - the program will also return `1`. +Immediately returns with the exit status `1`. When invoked with one of the recognized options it +will try to write the help or version text. Any IO error during this operation is diagnosed, yet +the program will also return `1`. "; #[uucore::main] diff --git a/src/uu/true/src/true.rs b/src/uu/true/src/true.rs index c3026e684..4a8452db6 100644 --- a/src/uu/true/src/true.rs +++ b/src/uu/true/src/true.rs @@ -8,12 +8,12 @@ use clap::{App, AppSettings, Arg}; use std::io::Write; use uucore::error::{set_exit_code, UResult}; -static ABOUT: &str = " - Returns true, a successful exit status. +static ABOUT: &str = "\ +Returns true, a successful exit status. - Immediately returns with the exit status `0`, except when invoked with one of the recognized - options. In those cases it will try to write the help or version text. Any IO error during this - operation causes the program to return `1` instead. +Immediately returns with the exit status `0`, except when invoked with one of the recognized +options. In those cases it will try to write the help or version text. Any IO error during this +operation causes the program to return `1` instead. "; #[uucore::main] From ea5541db5692022df684084bf6b4be2c58da6352 Mon Sep 17 00:00:00 2001 From: Sam Caldwell Date: Tue, 1 Feb 2022 08:29:22 -0700 Subject: [PATCH 17/46] truncate: add test_invalid_option --- tests/by-util/test_truncate.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index 0ef65ec16..c1e44f605 100644 --- a/tests/by-util/test_truncate.rs +++ b/tests/by-util/test_truncate.rs @@ -250,11 +250,24 @@ fn test_size_and_reference() { #[test] fn test_error_filename_only() { // truncate: you must specify either '--size' or '--reference' - new_ucmd!().args(&["file"]).fails().stderr_contains( - "error: The following required arguments were not provided: + new_ucmd!() + .args(&["file"]) + .fails() + .code_is(1) + .stderr_contains( + "error: The following required arguments were not provided: --reference --size ", - ); + ); +} + +#[test] +fn test_invalid_option() { + // truncate: cli parsing error returns 1 + new_ucmd!() + .args(&["--this-arg-does-not-exist"]) + .fails() + .code_is(1); } #[test] From f117fd8dd69cff0b821f68e41fb74bb5b72f8fa4 Mon Sep 17 00:00:00 2001 From: Rishi Kumar Ray Date: Wed, 2 Feb 2022 02:40:59 +0530 Subject: [PATCH 18/46] added correct format to ABOUT --- src/uu/base32/src/base32.rs | 14 +++++++------- src/uu/base64/src/base64.rs | 14 +++++++------- src/uu/basenc/src/basenc.rs | 10 +++++----- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/uu/base32/src/base32.rs b/src/uu/base32/src/base32.rs index 6d9759fa4..006a796f0 100644 --- a/src/uu/base32/src/base32.rs +++ b/src/uu/base32/src/base32.rs @@ -12,14 +12,14 @@ use uucore::{encoding::Format, error::UResult}; pub mod base_common; -static ABOUT: &str = " - With no FILE, or when FILE is -, read standard input. +static ABOUT: &str = "\ +With no FILE, or when FILE is -, read standard input. - The data are encoded as described for the base32 alphabet in RFC - 4648. When decoding, the input may contain newlines in addition - to the bytes of the formal base32 alphabet. Use --ignore-garbage - to attempt to recover from any other non-alphabet bytes in the - encoded stream. +The data are encoded as described for the base32 alphabet in RFC +4648. When decoding, the input may contain newlines in addition +to the bytes of the formal base32 alphabet. Use --ignore-garbage +to attempt to recover from any other non-alphabet bytes in the +encoded stream. "; fn usage() -> String { diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index 6d0192df9..20a9f55a5 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -13,14 +13,14 @@ use uucore::{encoding::Format, error::UResult}; use std::io::{stdin, Read}; -static ABOUT: &str = " - With no FILE, or when FILE is -, read standard input. +static ABOUT: &str = "\ +With no FILE, or when FILE is -, read standard input. - The data are encoded as described for the base64 alphabet in RFC - 3548. When decoding, the input may contain newlines in addition - to the bytes of the formal base64 alphabet. Use --ignore-garbage - to attempt to recover from any other non-alphabet bytes in the - encoded stream. +The data are encoded as described for the base64 alphabet in RFC +3548. When decoding, the input may contain newlines in addition +to the bytes of the formal base64 alphabet. Use --ignore-garbage +to attempt to recover from any other non-alphabet bytes in the +encoded stream. "; fn usage() -> String { diff --git a/src/uu/basenc/src/basenc.rs b/src/uu/basenc/src/basenc.rs index c21e224da..ef133b151 100644 --- a/src/uu/basenc/src/basenc.rs +++ b/src/uu/basenc/src/basenc.rs @@ -19,12 +19,12 @@ use uucore::{ use std::io::{stdin, Read}; -static ABOUT: &str = " - With no FILE, or when FILE is -, read standard input. +static ABOUT: &str = "\ +With no FILE, or when FILE is -, read standard input. - When decoding, the input may contain newlines in addition to the bytes of - the formal alphabet. Use --ignore-garbage to attempt to recover - from any other non-alphabet bytes in the encoded stream. +When decoding, the input may contain newlines in addition to the bytes of +the formal alphabet. Use --ignore-garbage to attempt to recover +from any other non-alphabet bytes in the encoded stream. "; const ENCODINGS: &[(&str, Format)] = &[ From 39f8329222acf64ff9d5a3193cd8608b46c1f4f4 Mon Sep 17 00:00:00 2001 From: Sam Caldwell Date: Tue, 1 Feb 2022 14:13:52 -0700 Subject: [PATCH 19/46] truncate: use `map_err` instead of `unwrap_or_else` --- src/uu/truncate/src/truncate.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 12f413247..242dd416a 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -116,15 +116,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .override_usage(&usage[..]) .after_help(&long_usage[..]) .try_get_matches_from(args) - .unwrap_or_else(|e| { + .map_err(|e| { e.print().expect("Error writing clap::Error"); match e.kind { - clap::ErrorKind::DisplayHelp | clap::ErrorKind::DisplayVersion => { - std::process::exit(0) - } - _ => std::process::exit(1), + clap::ErrorKind::DisplayHelp | clap::ErrorKind::DisplayVersion => 0, + _ => 1, } - }); + })?; let files: Vec = matches .values_of(options::ARG_FILES) From 587e6d2dede880fda71fcb84de53e54e6eb897dc Mon Sep 17 00:00:00 2001 From: Tillmann Rendel Date: Tue, 1 Feb 2022 23:33:24 +0100 Subject: [PATCH 20/46] Move back to .vscode folder --- .gitattributes => .vscode/.gitattributes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename .gitattributes => .vscode/.gitattributes (58%) diff --git a/.gitattributes b/.vscode/.gitattributes similarity index 58% rename from .gitattributes rename to .vscode/.gitattributes index 73153b0d3..c050fbbf3 100644 --- a/.gitattributes +++ b/.vscode/.gitattributes @@ -1,2 +1,2 @@ # Configure GitHub to not mark comments in configuration files as errors -.vscode/*.json linguist-language=JSON-with-Comments +*.json linguist-language=jsonc From d26f6f0f2f7bdac8468778d2de938767de699b59 Mon Sep 17 00:00:00 2001 From: Tillmann Rendel Date: Tue, 1 Feb 2022 23:36:50 +0100 Subject: [PATCH 21/46] Format cspell config and tweak comments Mostly to flush the highlighting cache on GitHub, but hopefully it also improves the file. --- .vscode/cSpell.json | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/.vscode/cSpell.json b/.vscode/cSpell.json index 95b6f0485..2ff4d4b7e 100644 --- a/.vscode/cSpell.json +++ b/.vscode/cSpell.json @@ -1,7 +1,12 @@ // `cspell` settings { - "version": "0.1", // Version of the setting file. Always 0.1 - "language": "en", // language - current active spelling language + // version of the setting file (always 0.1) + "version": "0.1", + + // spelling language + "language": "en", + + // custom dictionaries "dictionaries": ["acronyms+names", "jargon", "people", "shell", "workspace"], "dictionaryDefinitions": [ { "name": "acronyms+names", "path": "./cspell.dictionaries/acronyms+names.wordlist.txt" }, @@ -10,10 +15,19 @@ { "name": "shell", "path": "./cspell.dictionaries/shell.wordlist.txt" }, { "name": "workspace", "path": "./cspell.dictionaries/workspace.wordlist.txt" } ], - // ignorePaths - a list of globs to specify which files are to be ignored - "ignorePaths": ["Cargo.lock", "target/**", "tests/**/fixtures/**", "src/uu/dd/test-resources/**", "vendor/**"], - // ignoreWords - a list of words to be ignored (even if they are in the flagWords) + + // files to ignore (globs supported) + "ignorePaths": [ + "Cargo.lock", + "target/**", + "tests/**/fixtures/**", + "src/uu/dd/test-resources/**", + "vendor/**" + ], + + // words to ignore (even if they are in the flagWords) "ignoreWords": [], - // words - list of words to be always considered correct + + // words to always consider correct "words": [] } From 3bfcf78c03fd30d6e5ddac900da9d044b949cebf Mon Sep 17 00:00:00 2001 From: Tillmann Rendel Date: Tue, 1 Feb 2022 23:38:18 +0100 Subject: [PATCH 22/46] Tweak comment in extensions.json To flush the highlighting cache. --- .vscode/extensions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 30b38bfa9..a02baee69 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -2,7 +2,7 @@ // see for the documentation about the extensions.json format { "recommendations": [ - // Rust language support. + // Rust language support "matklad.rust-analyzer", // `cspell` spell-checker support "streetsidesoftware.code-spell-checker" From f6174dd946bb51338cb45d33aca6320a1c81686c Mon Sep 17 00:00:00 2001 From: Nathan Date: Tue, 1 Feb 2022 17:34:06 -0500 Subject: [PATCH 23/46] printf: add description and version Adds a version number and brief description to the printf utility in the user documentation --- src/uu/printf/src/printf.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/uu/printf/src/printf.rs b/src/uu/printf/src/printf.rs index 1e6c5fbd3..1bd53740c 100644 --- a/src/uu/printf/src/printf.rs +++ b/src/uu/printf/src/printf.rs @@ -9,6 +9,8 @@ use uucore::InvalidEncodingHandling; const VERSION: &str = "version"; const HELP: &str = "help"; +const USAGE: &str = "printf FORMATSTRING [ARGUMENT]..."; +const ABOUT: &str = "Print output based off of the format string and proceeding arguments."; static LONGHELP_LEAD: &str = "printf USAGE: printf FORMATSTRING [ARGUMENT]... @@ -295,7 +297,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) - .arg(Arg::new(VERSION).long(VERSION)) - .arg(Arg::new(HELP).long(HELP)) + .version(crate_version!()) + .about(ABOUT) + .override_usage(USAGE) + .arg(Arg::new(HELP).long(HELP).help("Print help information")) + .arg( + Arg::new(VERSION) + .long(VERSION) + .help("Print version information"), + ) .setting(AppSettings::InferLongArgs) } From 29b613a8523c26c6841edc43f411705c5db78358 Mon Sep 17 00:00:00 2001 From: Nathan Date: Tue, 1 Feb 2022 22:22:12 -0500 Subject: [PATCH 24/46] printf: resolve formatting nit in LONGHELP strings Removed 1 preceeding space for LONGHELP_LEAD and 2 preceeding spaces for LONGHELP_BODY --- src/uu/printf/src/printf.rs | 334 ++++++++++++++++++------------------ 1 file changed, 167 insertions(+), 167 deletions(-) diff --git a/src/uu/printf/src/printf.rs b/src/uu/printf/src/printf.rs index 1bd53740c..f282dc923 100644 --- a/src/uu/printf/src/printf.rs +++ b/src/uu/printf/src/printf.rs @@ -13,13 +13,13 @@ const USAGE: &str = "printf FORMATSTRING [ARGUMENT]..."; const ABOUT: &str = "Print output based off of the format string and proceeding arguments."; static LONGHELP_LEAD: &str = "printf - USAGE: printf FORMATSTRING [ARGUMENT]... +USAGE: printf FORMATSTRING [ARGUMENT]... - basic anonymous string templating: +basic anonymous string templating: - prints format string at least once, repeating as long as there are remaining arguments - output prints escaped literals in the format string as character literals - output replaces anonymous fields with the next unused argument, formatted according to the field. +prints format string at least once, repeating as long as there are remaining arguments +output prints escaped literals in the format string as character literals +output replaces anonymous fields with the next unused argument, formatted according to the field. Options: --help display this help and exit @@ -27,239 +27,239 @@ Options: "; static LONGHELP_BODY: &str = " - Prints the , replacing escaped character sequences with character literals - and substitution field sequences with passed arguments +Prints the , replacing escaped character sequences with character literals + and substitution field sequences with passed arguments - literally, with the exception of the below - escaped character sequences, and the substitution sequences described further down. +literally, with the exception of the below + escaped character sequences, and the substitution sequences described further down. - ESCAPE SEQUENCES +ESCAPE SEQUENCES - The following escape sequences, organized here in alphabetical order, - will print the corresponding character literal: +The following escape sequences, organized here in alphabetical order, +will print the corresponding character literal: - \" double quote +\" double quote - \\\\ backslash +\\\\ backslash - \\a alert (BEL) +\\a alert (BEL) - \\b backspace +\\b backspace - \\c End-of-Input +\\c End-of-Input - \\e escape +\\e escape - \\f form feed +\\f form feed - \\n new line +\\n new line - \\r carriage return +\\r carriage return - \\t horizontal tab +\\t horizontal tab - \\v vertical tab +\\v vertical tab - \\NNN byte with value expressed in octal value NNN (1 to 3 digits) - values greater than 256 will be treated +\\NNN byte with value expressed in octal value NNN (1 to 3 digits) + values greater than 256 will be treated - \\xHH byte with value expressed in hexadecimal value NN (1 to 2 digits) +\\xHH byte with value expressed in hexadecimal value NN (1 to 2 digits) - \\uHHHH Unicode (IEC 10646) character with value expressed in hexadecimal value HHHH (4 digits) +\\uHHHH Unicode (IEC 10646) character with value expressed in hexadecimal value HHHH (4 digits) - \\uHHHH Unicode character with value expressed in hexadecimal value HHHH (8 digits) +\\uHHHH Unicode character with value expressed in hexadecimal value HHHH (8 digits) - %% a single % +%% a single % - SUBSTITUTIONS +SUBSTITUTIONS - SUBSTITUTION QUICK REFERENCE +SUBSTITUTION QUICK REFERENCE - Fields +Fields - %s - string - %b - string parsed for literals - second parameter is max length +%s - string +%b - string parsed for literals + second parameter is max length - %c - char - no second parameter +%c - char + no second parameter - %i or %d - 64-bit integer - %u - 64 bit unsigned integer - %x or %X - 64-bit unsigned integer as hex - %o - 64-bit unsigned integer as octal - second parameter is min-width, integer - output below that width is padded with leading zeroes +%i or %d - 64-bit integer +%u - 64 bit unsigned integer +%x or %X - 64-bit unsigned integer as hex +%o - 64-bit unsigned integer as octal + second parameter is min-width, integer + output below that width is padded with leading zeroes - %f or %F - decimal floating point value - %e or %E - scientific notation floating point value - %g or %G - shorter of specially interpreted decimal or SciNote floating point value. - second parameter is - -max places after decimal point for floating point output - -max number of significant digits for scientific notation output +%f or %F - decimal floating point value +%e or %E - scientific notation floating point value +%g or %G - shorter of specially interpreted decimal or SciNote floating point value. + second parameter is + -max places after decimal point for floating point output + -max number of significant digits for scientific notation output - parameterizing fields +parameterizing fields - examples: +examples: - printf '%4.3i' 7 - has a first parameter of 4 - and a second parameter of 3 - will result in ' 007' +printf '%4.3i' 7 +has a first parameter of 4 + and a second parameter of 3 +will result in ' 007' - printf '%.1s' abcde - has no first parameter - and a second parameter of 1 - will result in 'a' +printf '%.1s' abcde +has no first parameter + and a second parameter of 1 +will result in 'a' - printf '%4c' q - has a first parameter of 4 - and no second parameter - will result in ' q' +printf '%4c' q +has a first parameter of 4 + and no second parameter +will result in ' q' - The first parameter of a field is the minimum width to pad the output to - if the output is less than this absolute value of this width, - it will be padded with leading spaces, or, if the argument is negative, - with trailing spaces. the default is zero. +The first parameter of a field is the minimum width to pad the output to + if the output is less than this absolute value of this width, + it will be padded with leading spaces, or, if the argument is negative, + with trailing spaces. the default is zero. - The second parameter of a field is particular to the output field type. - defaults can be found in the full substitution help below +The second parameter of a field is particular to the output field type. + defaults can be found in the full substitution help below - special prefixes to numeric arguments - 0 (e.g. 010) - interpret argument as octal (integer output fields only) - 0x (e.g. 0xABC) - interpret argument as hex (numeric output fields only) - \' (e.g. \'a) - interpret argument as a character constant +special prefixes to numeric arguments + 0 (e.g. 010) - interpret argument as octal (integer output fields only) + 0x (e.g. 0xABC) - interpret argument as hex (numeric output fields only) + \' (e.g. \'a) - interpret argument as a character constant - HOW TO USE SUBSTITUTIONS +HOW TO USE SUBSTITUTIONS - Substitutions are used to pass additional argument(s) into the FORMAT string, to be formatted a - particular way. E.g. +Substitutions are used to pass additional argument(s) into the FORMAT string, to be formatted a +particular way. E.g. - printf 'the letter %X comes before the letter %X' 10 11 + printf 'the letter %X comes before the letter %X' 10 11 - will print +will print - 'the letter A comes before the letter B' + 'the letter A comes before the letter B' - because the substitution field %X means - 'take an integer argument and write it as a hexadecimal number' +because the substitution field %X means +'take an integer argument and write it as a hexadecimal number' - Passing more arguments than are in the format string will cause the format string to be - repeated for the remaining substitutions +Passing more arguments than are in the format string will cause the format string to be + repeated for the remaining substitutions - printf 'it is %i F in %s \n' 22 Portland 25 Boston 27 New York + printf 'it is %i F in %s \n' 22 Portland 25 Boston 27 New York - will print +will print - 'it is 22 F in Portland - it is 25 F in Boston - it is 27 F in Boston - ' - If a format string is printed but there are less arguments remaining - than there are substitution fields, substitution fields without - an argument will default to empty strings, or for numeric fields - the value 0 + 'it is 22 F in Portland + it is 25 F in Boston + it is 27 F in Boston + ' +If a format string is printed but there are less arguments remaining + than there are substitution fields, substitution fields without + an argument will default to empty strings, or for numeric fields + the value 0 - AVAILABLE SUBSTITUTIONS +AVAILABLE SUBSTITUTIONS - This program, like GNU coreutils printf, - interprets a modified subset of the POSIX C printf spec, - a quick reference to substitutions is below. +This program, like GNU coreutils printf, +interprets a modified subset of the POSIX C printf spec, +a quick reference to substitutions is below. - STRING SUBSTITUTIONS - All string fields have a 'max width' parameter - %.3s means 'print no more than three characters of the original input' + STRING SUBSTITUTIONS + All string fields have a 'max width' parameter + %.3s means 'print no more than three characters of the original input' - %s - string + %s - string - %b - escaped string - the string will be checked for any escaped literals from - the escaped literal list above, and translate them to literal characters. - e.g. \\n will be transformed into a newline character. + %b - escaped string - the string will be checked for any escaped literals from + the escaped literal list above, and translate them to literal characters. + e.g. \\n will be transformed into a newline character. - One special rule about %b mode is that octal literals are interpreted differently - In arguments passed by %b, pass octal-interpreted literals must be in the form of \\0NNN - instead of \\NNN. (Although, for legacy reasons, octal literals in the form of \\NNN will - still be interpreted and not throw a warning, you will have problems if you use this for a - literal whose code begins with zero, as it will be viewed as in \\0NNN form.) + One special rule about %b mode is that octal literals are interpreted differently + In arguments passed by %b, pass octal-interpreted literals must be in the form of \\0NNN + instead of \\NNN. (Although, for legacy reasons, octal literals in the form of \\NNN will + still be interpreted and not throw a warning, you will have problems if you use this for a + literal whose code begins with zero, as it will be viewed as in \\0NNN form.) - CHAR SUBSTITUTIONS - The character field does not have a secondary parameter. + CHAR SUBSTITUTIONS + The character field does not have a secondary parameter. - %c - a single character + %c - a single character - INTEGER SUBSTITUTIONS - All integer fields have a 'pad with zero' parameter - %.4i means an integer which if it is less than 4 digits in length, - is padded with leading zeros until it is 4 digits in length. + INTEGER SUBSTITUTIONS + All integer fields have a 'pad with zero' parameter + %.4i means an integer which if it is less than 4 digits in length, + is padded with leading zeros until it is 4 digits in length. - %d or %i - 64-bit integer + %d or %i - 64-bit integer - %u - 64 bit unsigned integer + %u - 64 bit unsigned integer - %x or %X - 64 bit unsigned integer printed in Hexadecimal (base 16) - %X instead of %x means to use uppercase letters for 'a' through 'f' + %x or %X - 64 bit unsigned integer printed in Hexadecimal (base 16) + %X instead of %x means to use uppercase letters for 'a' through 'f' - %o - 64 bit unsigned integer printed in octal (base 8) + %o - 64 bit unsigned integer printed in octal (base 8) - FLOATING POINT SUBSTITUTIONS + FLOATING POINT SUBSTITUTIONS - All floating point fields have a 'max decimal places / max significant digits' parameter - %.10f means a decimal floating point with 7 decimal places past 0 - %.10e means a scientific notation number with 10 significant digits - %.10g means the same behavior for decimal and Sci. Note, respectively, and provides the shorter - of each's output. + All floating point fields have a 'max decimal places / max significant digits' parameter + %.10f means a decimal floating point with 7 decimal places past 0 + %.10e means a scientific notation number with 10 significant digits + %.10g means the same behavior for decimal and Sci. Note, respectively, and provides the shorter + of each's output. - Like with GNU coreutils, the value after the decimal point is these outputs is parsed as a - double first before being rendered to text. For both implementations do not expect meaningful - precision past the 18th decimal place. When using a number of decimal places that is 18 or - higher, you can expect variation in output between GNU coreutils printf and this printf at the - 18th decimal place of +/- 1 + Like with GNU coreutils, the value after the decimal point is these outputs is parsed as a + double first before being rendered to text. For both implementations do not expect meaningful + precision past the 18th decimal place. When using a number of decimal places that is 18 or + higher, you can expect variation in output between GNU coreutils printf and this printf at the + 18th decimal place of +/- 1 - %f - floating point value presented in decimal, truncated and displayed to 6 decimal places by - default. There is not past-double behavior parity with Coreutils printf, values are not - estimated or adjusted beyond input values. + %f - floating point value presented in decimal, truncated and displayed to 6 decimal places by + default. There is not past-double behavior parity with Coreutils printf, values are not + estimated or adjusted beyond input values. - %e or %E - floating point value presented in scientific notation - 7 significant digits by default - %E means use to use uppercase E for the mantissa. + %e or %E - floating point value presented in scientific notation + 7 significant digits by default + %E means use to use uppercase E for the mantissa. - %g or %G - floating point value presented in the shorter of decimal and scientific notation - behaves differently from %f and %E, please see posix printf spec for full details, - some examples of different behavior: + %g or %G - floating point value presented in the shorter of decimal and scientific notation + behaves differently from %f and %E, please see posix printf spec for full details, + some examples of different behavior: - Sci Note has 6 significant digits by default - Trailing zeroes are removed - Instead of being truncated, digit after last is rounded + Sci Note has 6 significant digits by default + Trailing zeroes are removed + Instead of being truncated, digit after last is rounded - Like other behavior in this utility, the design choices of floating point - behavior in this utility is selected to reproduce in exact - the behavior of GNU coreutils' printf from an inputs and outputs standpoint. + Like other behavior in this utility, the design choices of floating point + behavior in this utility is selected to reproduce in exact + the behavior of GNU coreutils' printf from an inputs and outputs standpoint. - USING PARAMETERS - Most substitution fields can be parameterized using up to 2 numbers that can - be passed to the field, between the % sign and the field letter. +USING PARAMETERS + Most substitution fields can be parameterized using up to 2 numbers that can + be passed to the field, between the % sign and the field letter. - The 1st parameter always indicates the minimum width of output, it is useful for creating - columnar output. Any output that would be less than this minimum width is padded with - leading spaces - The 2nd parameter is proceeded by a dot. - You do not have to use parameters + The 1st parameter always indicates the minimum width of output, it is useful for creating + columnar output. Any output that would be less than this minimum width is padded with + leading spaces + The 2nd parameter is proceeded by a dot. + You do not have to use parameters - SPECIAL FORMS OF INPUT - For numeric input, the following additional forms of input are accepted besides decimal: +SPECIAL FORMS OF INPUT + For numeric input, the following additional forms of input are accepted besides decimal: - Octal (only with integer): if the argument begins with a 0 the proceeding characters - will be interpreted as octal (base 8) for integer fields + Octal (only with integer): if the argument begins with a 0 the proceeding characters + will be interpreted as octal (base 8) for integer fields - Hexadecimal: if the argument begins with 0x the proceeding characters will be interpreted - will be interpreted as hex (base 16) for any numeric fields - for float fields, hexadecimal input results in a precision - limit (in converting input past the decimal point) of 10^-15 + Hexadecimal: if the argument begins with 0x the proceeding characters will be interpreted + will be interpreted as hex (base 16) for any numeric fields + for float fields, hexadecimal input results in a precision + limit (in converting input past the decimal point) of 10^-15 - Character Constant: if the argument begins with a single quote character, the first byte - of the next character will be interpreted as an 8-bit unsigned integer. If there are - additional bytes, they will throw an error (unless the environment variable POSIXLY_CORRECT - is set) + Character Constant: if the argument begins with a single quote character, the first byte + of the next character will be interpreted as an 8-bit unsigned integer. If there are + additional bytes, they will throw an error (unless the environment variable POSIXLY_CORRECT + is set) WRITTEN BY : Nathan E. Ross, et al. for the uutils project From 560cd74a639157e85b256542ce9a867d30daf377 Mon Sep 17 00:00:00 2001 From: Hanif Bin Ariffin Date: Wed, 2 Feb 2022 12:49:40 +0800 Subject: [PATCH 25/46] test_ls: Do not rely on the system time of metadata'access time Signed-off-by: Hanif Bin Ariffin --- tests/by-util/test_ls.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index e3fd99e00..f61611390 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -1327,17 +1327,14 @@ fn test_ls_order_time() { // So the order should be 2 3 4 1 for arg in &["-u", "--time=atime", "--time=access", "--time=use"] { let result = scene.ucmd().arg("-t").arg(arg).succeeds(); - let file3_access = at.open("test-3").metadata().unwrap().accessed().unwrap(); - let file4_access = at.open("test-4").metadata().unwrap().accessed().unwrap(); + at.open("test-3").metadata().unwrap().accessed().unwrap(); + at.open("test-4").metadata().unwrap().accessed().unwrap(); // It seems to be dependent on the platform whether the access time is actually set - if file3_access > file4_access { - result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n"); - } else { - // Access time does not seem to be set on Windows and some other - // systems so the order is 4 3 2 1 - result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n"); - } + #[cfg(unix)] + result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n"); + #[cfg(windows)] + result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n"); } // test-2 had the last ctime change when the permissions were set From 7e32b6ba17a482d970e78966abffed7a4a2d4ee2 Mon Sep 17 00:00:00 2001 From: Rahul Kadukar Date: Tue, 1 Feb 2022 23:51:48 -0500 Subject: [PATCH 26/46] Added description for hostid --- src/uu/hostid/src/hostid.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/hostid/src/hostid.rs b/src/uu/hostid/src/hostid.rs index 764b7d279..99b800f14 100644 --- a/src/uu/hostid/src/hostid.rs +++ b/src/uu/hostid/src/hostid.rs @@ -12,6 +12,7 @@ use libc::c_long; use uucore::error::UResult; static SYNTAX: &str = "[options]"; +const SUMMARY: &str = "Print the numeric identifier (in hexadecimal) for the current host"; // currently rust libc interface doesn't include gethostid extern "C" { @@ -28,6 +29,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) .version(crate_version!()) + .about(SUMMARY) .override_usage(SYNTAX) .setting(AppSettings::InferLongArgs) } From 773ceb5534a0c85ff2e7ca86222b4b037767bad9 Mon Sep 17 00:00:00 2001 From: DevSaab Date: Wed, 2 Feb 2022 10:08:48 -0500 Subject: [PATCH 27/46] Include ABOUT for shuf --- src/uu/shuf/src/shuf.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/shuf/src/shuf.rs b/src/uu/shuf/src/shuf.rs index a7dcd48e9..da2dfff1b 100644 --- a/src/uu/shuf/src/shuf.rs +++ b/src/uu/shuf/src/shuf.rs @@ -31,6 +31,7 @@ Write a random permutation of the input lines to standard output. With no FILE, or when FILE is -, read standard input. "#; +static ABOUT: &str = "Shuffle the input by outputting a random permutation of input lines. Each output permutation is equally likely."; static TEMPLATE: &str = "Usage: {usage}\nMandatory arguments to long options are mandatory for short options too.\n{options}"; struct Options { @@ -121,6 +122,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) .name(NAME) + .about(ABOUT) .version(crate_version!()) .help_template(TEMPLATE) .override_usage(USAGE) From d50c9c3e776b273569b9bc18bce8271ce789a95f Mon Sep 17 00:00:00 2001 From: Eli Youngs Date: Wed, 2 Feb 2022 23:40:26 -0800 Subject: [PATCH 28/46] Fail when copying a directory to a file --- src/uu/cp/src/cp.rs | 5 ++++- tests/by-util/test_cp.rs | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index f8ce6f241..2ffa1e3ca 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1025,7 +1025,10 @@ fn copy_directory( if is_symlink && !options.dereference { copy_link(&path, &local_to_target, symlinked_files)?; } else if path.is_dir() && !local_to_target.exists() { - or_continue!(fs::create_dir_all(local_to_target)); + if target.is_file() { + return Err("cannot overwrite non-directory with directory".into()); + } + fs::create_dir_all(local_to_target)?; } else if !path.is_dir() { if preserve_hard_links { let mut found_hard_link = false; diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 92637dfbe..e194b59ff 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1444,3 +1444,12 @@ fn test_cp_archive_on_nonexistent_file() { "cp: cannot stat 'nonexistent_file.txt': No such file or directory (os error 2)", ); } +#[test] +fn test_cp_dir_vs_file() { + new_ucmd!() + .arg("-R") + .arg(TEST_COPY_FROM_FOLDER) + .arg(TEST_EXISTING_FILE) + .fails() + .stderr_only("cp: cannot overwrite non-directory with directory"); +} From ff8a83b256b1983c56d85933fa0d869046a503d9 Mon Sep 17 00:00:00 2001 From: Hanif Ariffin Date: Thu, 3 Feb 2022 21:10:39 +0800 Subject: [PATCH 29/46] touch: Better error message when no args is given Matches the behavior of GNU touch ```shell hbina@akarin ~/g/uutils (hbina-realpath-absolute-symlinks)> touch > /dev/null touch: missing file operand Try 'touch --help' for more information. hbina@akarin ~/g/uutils (hbina-realpath-absolute-symlinks) [1]> cargo run --quiet -- touch > /dev/null touch: missing file operand Try 'touch --help' for more information. hbina@akarin ~/g/uutils (hbina-realpath-absolute-symlinks) [1]> cargo run --quiet -- touch 2> /dev/null hbina@akarin ~/g/uutils (hbina-realpath-absolute-symlinks) [1]> touch 2> /dev/null ``` Signed-off-by: Hanif Ariffin --- src/uu/touch/src/touch.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index b1df1aca4..32dd4817d 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -58,7 +58,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().override_usage(&usage[..]).get_matches_from(args); - let files = matches.values_of_os(ARG_FILES).unwrap(); + let files = matches.values_of_os(ARG_FILES).ok_or(USimpleError::new( + 1, + r##"missing file operand +Try 'touch --help' for more information."##, + ))?; let (mut atime, mut mtime) = if let Some(reference) = matches.value_of_os(options::sources::REFERENCE) { From 9cd65c766af5d9996926b8a429d2e442b6a5b2e9 Mon Sep 17 00:00:00 2001 From: Hanif Ariffin Date: Thu, 3 Feb 2022 21:14:56 +0800 Subject: [PATCH 30/46] Add tests Signed-off-by: Hanif Ariffin --- tests/by-util/test_touch.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index e661907cc..dd4a0b6cc 100644 --- a/tests/by-util/test_touch.rs +++ b/tests/by-util/test_touch.rs @@ -530,3 +530,12 @@ fn test_touch_permission_denied_error_msg() { &full_path )); } + +#[test] +fn test_touch_no_args() { + let mut ucmd = new_ucmd!(); + ucmd.fails().stderr_only( + r##"touch: missing file operand +Try 'touch --help' for more information."##, + ); +} From ee721ebf4e4288fc83a3fab3c26bd5a8d2bc6d0e Mon Sep 17 00:00:00 2001 From: snobee Date: Wed, 2 Feb 2022 21:22:28 -0800 Subject: [PATCH 31/46] head: handle multibyte numeric utf-8 chars --- src/uu/head/src/parse.rs | 2 +- tests/by-util/test_head.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/uu/head/src/parse.rs b/src/uu/head/src/parse.rs index 3f1d8ef42..b44a8b69d 100644 --- a/src/uu/head/src/parse.rs +++ b/src/uu/head/src/parse.rs @@ -20,7 +20,7 @@ pub fn parse_obsolete(src: &str) -> Option let mut has_num = false; let mut last_char = 0 as char; for (n, c) in &mut chars { - if c.is_numeric() { + if c.is_digit(10) { has_num = true; num_end = n; } else { diff --git a/tests/by-util/test_head.rs b/tests/by-util/test_head.rs index 246f5b62a..25410d76f 100644 --- a/tests/by-util/test_head.rs +++ b/tests/by-util/test_head.rs @@ -306,6 +306,10 @@ fn test_head_invalid_num() { )); } } + new_ucmd!() + .args(&["-c", "-³"]) + .fails() + .stderr_is("head: invalid number of bytes: '³'"); } #[test] From 3586465917372aa3a95f677a9c387749cd5a4a85 Mon Sep 17 00:00:00 2001 From: Hanif Bin Ariffin Date: Thu, 3 Feb 2022 20:17:53 +0800 Subject: [PATCH 32/46] dont use is_numeric to check for digits Signed-off-by: Hanif Bin Ariffin --- src/uu/tail/src/parse.rs | 2 +- tests/by-util/test_stat.rs | 6 ++++++ tests/by-util/test_tail.rs | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/uu/tail/src/parse.rs b/src/uu/tail/src/parse.rs index 1c4f36bbd..ea9df9a02 100644 --- a/src/uu/tail/src/parse.rs +++ b/src/uu/tail/src/parse.rs @@ -19,7 +19,7 @@ pub fn parse_obsolete(src: &str) -> Option let mut has_num = false; let mut last_char = 0 as char; for (n, c) in &mut chars { - if c.is_numeric() { + if c.is_digit(10) { has_num = true; num_end = n; } else { diff --git a/tests/by-util/test_stat.rs b/tests/by-util/test_stat.rs index 9bbb1c1ca..8c1255a88 100644 --- a/tests/by-util/test_stat.rs +++ b/tests/by-util/test_stat.rs @@ -36,6 +36,12 @@ fn test_group_num() { assert_eq!("", group_num("")); } +#[test] +#[should_panic] +fn test_group_num_panic_if_invalid_numeric_characters() { + group_num("³³³³³"); +} + #[cfg(test)] mod test_generate_tokens { use super::*; diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index dcdb2e9dc..ebcd29cf5 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -491,6 +491,10 @@ fn test_tail_invalid_num() { )); } } + new_ucmd!() + .args(&["-c", "-³"]) + .fails() + .stderr_is("tail: invalid number of bytes: '³'"); } #[test] From 861437addf4dee8cb3ab932cf0b23eb62e548296 Mon Sep 17 00:00:00 2001 From: Hanif Ariffin Date: Thu, 3 Feb 2022 21:45:02 +0800 Subject: [PATCH 33/46] Fix small clippy issue Signed-off-by: Hanif Ariffin --- src/uu/touch/src/touch.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index 32dd4817d..e27dbfc18 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -58,11 +58,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().override_usage(&usage[..]).get_matches_from(args); - let files = matches.values_of_os(ARG_FILES).ok_or(USimpleError::new( - 1, - r##"missing file operand + let files = matches.values_of_os(ARG_FILES).ok_or_else(|| { + USimpleError::new( + 1, + r##"missing file operand Try 'touch --help' for more information."##, - ))?; + ) + })?; let (mut atime, mut mtime) = if let Some(reference) = matches.value_of_os(options::sources::REFERENCE) { From e60b581c38eaf195fa3264b84693427fe9645cb7 Mon Sep 17 00:00:00 2001 From: Hanif Ariffin Date: Thu, 3 Feb 2022 23:02:50 +0800 Subject: [PATCH 34/46] test_sort: Preserve the environment variable when executing tests (#3031) * test_sort: Output sorted files to a file with different name Signed-off-by: Hanif Bin Ariffin * Fix the test by saving the environment variable Signed-off-by: Hanif Bin Ariffin Co-authored-by: Hanif Bin Ariffin --- tests/by-util/test_sort.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 5cf622fb8..5ae56b29e 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1066,10 +1066,13 @@ fn test_separator_null() { #[test] fn test_output_is_input() { let input = "a\nb\nc\n"; - let (at, mut cmd) = at_and_ucmd!(); + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; at.touch("file"); at.append("file", input); - cmd.args(&["-m", "-u", "-o", "file", "file", "file", "file"]) + scene + .ucmd_keepenv() + .args(&["-m", "-u", "-o", "file", "file", "file", "file"]) .succeeds(); assert_eq!(at.read("file"), input); } From caad4db712193804f04179bd921140994989c6b1 Mon Sep 17 00:00:00 2001 From: Roy Ivy III Date: Tue, 1 Feb 2022 23:01:34 -0600 Subject: [PATCH 35/46] maint/CICD ~ add MSRV check for '.clippy.toml' --- .github/workflows/CICD.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index aec424312..5fd51f852 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -340,6 +340,13 @@ jobs: ## Confirm MinSRV compatible 'Cargo.lock' # * 'Cargo.lock' is required to be in a format that `cargo` of MinSRV can interpret (eg, v1-format for MinSRV < v1.38) cargo fetch --locked --quiet || { echo "::error file=Cargo.lock::Incompatible (or out-of-date) 'Cargo.lock' file; update using \`cargo +${{ env.RUST_MIN_SRV }} update\`" ; exit 1 ; } + - name: Confirm MinSRV equivalence for '.clippy.toml' + shell: bash + run: | + ## Confirm MinSRV equivalence for '.clippy.toml' + # * ensure '.clippy.toml' MSRV configuration setting is equal to ${{ env.RUST_MIN_SRV }} + CLIPPY_MSRV=$(grep -P "(?i)^\s*msrv\s*=\s*" .clippy.toml | grep -oP "\d+([.]\d+)+") + if [ "${CLIPPY_MSRV}" != "${{ env.RUST_MIN_SRV }}" ]; then { echo "::error file=.clippy.toml::Incorrect MSRV configuration for clippy (found '${CLIPPY_MSRV}'; should be '${{ env.RUST_MIN_SRV }}'); update '.clippy.toml' with 'msrv = \"${{ env.RUST_MIN_SRV }}\"'" ; exit 1 ; } ; fi - name: Info shell: bash run: | From f01c3ef46a55a2a3a12d728a06344ebcabb0b59f Mon Sep 17 00:00:00 2001 From: Roy Ivy III Date: Tue, 1 Feb 2022 23:01:56 -0600 Subject: [PATCH 36/46] maint/polish ~ whitespace normalization --- .github/workflows/GnuTests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml index b36bbcb33..8303ee403 100644 --- a/.github/workflows/GnuTests.yml +++ b/.github/workflows/GnuTests.yml @@ -63,8 +63,8 @@ jobs: XPASS=$(sed -n "s/.*# XPASS: \(.*\)/\1/p" "$LOG_FILE"|tr -d '\r'|head -n1) ERROR=$(sed -n "s/.*# ERROR: \(.*\)/\1/p" "$LOG_FILE"|tr -d '\r'|head -n1) if [[ "$TOTAL" -eq 0 || "$TOTAL" -eq 1 ]]; then - echo "Error in the execution, failing early" - exit 1 + echo "Error in the execution, failing early" + exit 1 fi output="GNU tests summary = TOTAL: $TOTAL / PASS: $PASS / FAIL: $FAIL / ERROR: $ERROR" echo "${output}" From 3fbaa79359f1712489f13f5d4273caed36bce40e Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Tue, 1 Feb 2022 22:53:19 -0500 Subject: [PATCH 37/46] dd: add support for 'b' and 'x' multipliers Support the suffix 'b' (multiply by 512) and 'x' (multiply by an arbitrary amount) when specifying numeric arguments to dd. --- src/uu/dd/src/parseargs.rs | 124 ++++++++++++++++++++++++++++++++----- tests/by-util/test_dd.rs | 22 ++++++- 2 files changed, 128 insertions(+), 18 deletions(-) diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index 492ab70cb..915a99344 100644 --- a/src/uu/dd/src/parseargs.rs +++ b/src/uu/dd/src/parseargs.rs @@ -4,7 +4,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore ctty, ctable, iconvflags, oconvflags +// spell-checker:ignore ctty, ctable, iconvflags, oconvflags parseargs #[cfg(test)] mod unit_tests; @@ -12,6 +12,7 @@ mod unit_tests; use super::*; use std::error::Error; use uucore::error::UError; +use uucore::parse_size::ParseSizeError; pub type Matches = ArgMatches; @@ -31,6 +32,25 @@ pub enum ParseError { Unimplemented(String), } +impl ParseError { + /// Replace the argument, if any, with the given string, consuming self. + fn with_arg(self, s: String) -> Self { + match self { + Self::MultipleFmtTable => Self::MultipleFmtTable, + Self::MultipleUCaseLCase => Self::MultipleUCaseLCase, + Self::MultipleBlockUnblock => Self::MultipleBlockUnblock, + Self::MultipleExclNoCreate => Self::MultipleExclNoCreate, + Self::FlagNoMatch(_) => Self::FlagNoMatch(s), + Self::ConvFlagNoMatch(_) => Self::ConvFlagNoMatch(s), + Self::MultiplierStringParseFailure(_) => Self::MultiplierStringParseFailure(s), + Self::MultiplierStringOverflow(_) => Self::MultiplierStringOverflow(s), + Self::BlockUnblockWithoutCBS => Self::BlockUnblockWithoutCBS, + Self::StatusLevelNotRecognized(_) => Self::StatusLevelNotRecognized(s), + Self::Unimplemented(_) => Self::Unimplemented(s), + } + } +} + impl std::fmt::Display for ParseError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -310,28 +330,76 @@ fn parse_bytes_only(s: &str) -> Result { .map_err(|_| ParseError::MultiplierStringParseFailure(s.to_string())) } +/// Parse a number of bytes from the given string, assuming no `'x'` characters. +/// +/// The `'x'` character means "multiply the number before the `'x'` by +/// the number after the `'x'`". In order to compute the numbers +/// before and after the `'x'`, use this function, which assumes there +/// are no `'x'` characters in the string. +/// +/// A suffix `'c'` means multiply by 1, `'w'` by 2, and `'b'` by +/// 512. You can also use standard block size suffixes like `'k'` for +/// 1024. +/// +/// # Errors +/// +/// If a number cannot be parsed or if the multiplication would cause +/// an overflow. +/// +/// # Examples +/// +/// ```rust,ignore +/// assert_eq!(parse_bytes_no_x("123").unwrap(), 123); +/// assert_eq!(parse_bytes_no_x("2c").unwrap(), 2 * 1); +/// assert_eq!(parse_bytes_no_x("3w").unwrap(), 3 * 2); +/// assert_eq!(parse_bytes_no_x("2b").unwrap(), 2 * 512); +/// assert_eq!(parse_bytes_no_x("2k").unwrap(), 2 * 1024); +/// ``` +fn parse_bytes_no_x(s: &str) -> Result { + let (num, multiplier) = match (s.find('c'), s.rfind('w'), s.rfind('b')) { + (None, None, None) => match uucore::parse_size::parse_size(s) { + Ok(n) => (n, 1), + Err(ParseSizeError::ParseFailure(s)) => { + return Err(ParseError::MultiplierStringParseFailure(s)) + } + Err(ParseSizeError::SizeTooBig(s)) => { + return Err(ParseError::MultiplierStringOverflow(s)) + } + }, + (Some(i), None, None) => (parse_bytes_only(&s[..i])?, 1), + (None, Some(i), None) => (parse_bytes_only(&s[..i])?, 2), + (None, None, Some(i)) => (parse_bytes_only(&s[..i])?, 512), + _ => return Err(ParseError::MultiplierStringParseFailure(s.to_string())), + }; + num.checked_mul(multiplier) + .ok_or_else(|| ParseError::MultiplierStringOverflow(s.to_string())) +} + /// Parse byte and multiplier like 512, 5KiB, or 1G. /// Uses uucore::parse_size, and adds the 'w' and 'c' suffixes which are mentioned /// in dd's info page. fn parse_bytes_with_opt_multiplier(s: &str) -> Result { - if let Some(idx) = s.rfind('c') { - parse_bytes_only(&s[..idx]) - } else if let Some(idx) = s.rfind('w') { - let partial = parse_bytes_only(&s[..idx])?; + // TODO On my Linux system, there seems to be a maximum block size of 4096 bytes: + // + // $ printf "%0.sa" {1..10000} | dd bs=4095 count=1 status=none | wc -c + // 4095 + // $ printf "%0.sa" {1..10000} | dd bs=4k count=1 status=none | wc -c + // 4096 + // $ printf "%0.sa" {1..10000} | dd bs=4097 count=1 status=none | wc -c + // 4096 + // $ printf "%0.sa" {1..10000} | dd bs=5k count=1 status=none | wc -c + // 4096 + // - partial - .checked_mul(2) - .ok_or_else(|| ParseError::MultiplierStringOverflow(s.to_string())) - } else { - uucore::parse_size::parse_size(s).map_err(|e| match e { - uucore::parse_size::ParseSizeError::ParseFailure(s) => { - ParseError::MultiplierStringParseFailure(s) - } - uucore::parse_size::ParseSizeError::SizeTooBig(s) => { - ParseError::MultiplierStringOverflow(s) - } - }) + // Split on the 'x' characters. Each component will be parsed + // individually, then multiplied together. + let mut total = 1; + for part in s.split('x') { + let num = parse_bytes_no_x(part).map_err(|e| e.with_arg(s.to_string()))?; + total *= num; } + + Ok(total) } pub fn parse_ibs(matches: &Matches) -> Result { @@ -689,3 +757,25 @@ pub fn parse_input_non_ascii(matches: &Matches) -> Result { Ok(false) } } + +#[cfg(test)] +mod tests { + + use crate::parseargs::parse_bytes_with_opt_multiplier; + + #[test] + fn test_parse_bytes_with_opt_multiplier() { + assert_eq!(parse_bytes_with_opt_multiplier("123").unwrap(), 123); + assert_eq!(parse_bytes_with_opt_multiplier("123c").unwrap(), 123 * 1); + assert_eq!(parse_bytes_with_opt_multiplier("123w").unwrap(), 123 * 2); + assert_eq!(parse_bytes_with_opt_multiplier("123b").unwrap(), 123 * 512); + assert_eq!(parse_bytes_with_opt_multiplier("123x3").unwrap(), 123 * 3); + assert_eq!(parse_bytes_with_opt_multiplier("123k").unwrap(), 123 * 1024); + assert_eq!(parse_bytes_with_opt_multiplier("1x2x3").unwrap(), 1 * 2 * 3); + assert_eq!( + parse_bytes_with_opt_multiplier("1wx2cx3w").unwrap(), + (1 * 2) * (2 * 1) * (3 * 2) + ); + assert!(parse_bytes_with_opt_multiplier("123asdf").is_err()); + } +} diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index e73fe0673..e9a1f9468 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -1,4 +1,4 @@ -// spell-checker:ignore fname, tname, fpath, specfile, testfile, unspec, ifile, ofile, outfile, fullblock, urand, fileio, atoe, atoibm, availible, behaviour, bmax, bremain, btotal, cflags, creat, ctable, ctty, datastructures, doesnt, etoa, fileout, fname, gnudd, iconvflags, nocache, noctty, noerror, nofollow, nolinks, nonblock, oconvflags, outfile, parseargs, rlen, rmax, rposition, rremain, rsofar, rstat, sigusr, sigval, wlen, wstat abcdefghijklm +// spell-checker:ignore fname, tname, fpath, specfile, testfile, unspec, ifile, ofile, outfile, fullblock, urand, fileio, atoe, atoibm, availible, behaviour, bmax, bremain, btotal, cflags, creat, ctable, ctty, datastructures, doesnt, etoa, fileout, fname, gnudd, iconvflags, nocache, noctty, noerror, nofollow, nolinks, nonblock, oconvflags, outfile, parseargs, rlen, rmax, rposition, rremain, rsofar, rstat, sigusr, sigval, wlen, wstat abcdefghijklm abcdefghi use crate::common::util::*; @@ -178,6 +178,26 @@ fn test_stdin_stdout_count_w_multiplier() { .success(); } +#[test] +fn test_b_multiplier() { + // "2b" means 2 * 512, which is 1024. + new_ucmd!() + .args(&["bs=2b", "count=1"]) + .pipe_in("a".repeat(1025)) + .succeeds() + .stdout_is("a".repeat(1024)); +} + +#[test] +fn test_x_multiplier() { + // "2x3" means 2 * 3, which is 6. + new_ucmd!() + .args(&["bs=2x3", "count=1"]) + .pipe_in("abcdefghi") + .succeeds() + .stdout_is("abcdef"); +} + #[test] fn test_final_stats_noxfer() { new_ucmd!() From 639971e5200ea213c6f9f8dacec10b19b08a61e3 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Thu, 3 Feb 2022 23:19:14 -0500 Subject: [PATCH 38/46] df: refactor function for parsing CLI args Add a `Options::from()` function to collect the code for parsing an `Options` object from the `clap::ArgMatches` object. --- src/uu/df/src/df.rs | 97 +++++++++++++++++---------------------------- 1 file changed, 37 insertions(+), 60 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 77deeb6df..07aa82dc1 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -12,17 +12,17 @@ use uucore::error::UResult; use uucore::fsext::statfs_fn; use uucore::fsext::{read_fs_list, FsUsage, MountInfo}; -use clap::{crate_version, App, AppSettings, Arg}; +use clap::{crate_version, App, AppSettings, Arg, ArgMatches}; use number_prefix::NumberPrefix; use std::cell::Cell; use std::collections::HashMap; use std::collections::HashSet; - use std::error::Error; #[cfg(unix)] use std::ffi::CString; use std::fmt::Display; +use std::iter::FromIterator; #[cfg(unix)] use std::mem; @@ -69,6 +69,27 @@ struct Options { fs_selector: FsSelector, } +impl Options { + /// Convert command-line arguments into [`Options`]. + fn from(matches: &ArgMatches) -> Self { + Self { + show_local_fs: matches.is_present(OPT_LOCAL), + show_all_fs: matches.is_present(OPT_ALL), + show_listed_fs: false, + show_fs_type: matches.is_present(OPT_PRINT_TYPE), + show_inode_instead: matches.is_present(OPT_INODES), + human_readable_base: if matches.is_present(OPT_HUMAN_READABLE) { + 1024 + } else if matches.is_present(OPT_HUMAN_READABLE_2) { + 1000 + } else { + -1 + }, + fs_selector: FsSelector::from(matches), + } + } +} + #[derive(Debug, Clone)] struct Filesystem { mount_info: MountInfo, @@ -80,18 +101,19 @@ fn usage() -> String { } impl FsSelector { - fn new() -> Self { - Self::default() - } - - #[inline(always)] - fn include(&mut self, fs_type: String) { - self.include.insert(fs_type); - } - - #[inline(always)] - fn exclude(&mut self, fs_type: String) { - self.exclude.insert(fs_type); + /// Convert command-line arguments into a [`FsSelector`]. + /// + /// This function reads the include and exclude sets from + /// [`ArgMatches`] and returns the corresponding [`FsSelector`] + /// instance. + fn from(matches: &ArgMatches) -> Self { + let include = HashSet::from_iter(matches.values_of_lossy(OPT_TYPE).unwrap_or_default()); + let exclude = HashSet::from_iter( + matches + .values_of_lossy(OPT_EXCLUDE_TYPE) + .unwrap_or_default(), + ); + Self { include, exclude } } fn should_select(&self, fs_type: &str) -> bool { @@ -102,24 +124,6 @@ impl FsSelector { } } -impl Options { - fn new() -> Self { - Self { - show_local_fs: false, - show_all_fs: false, - show_listed_fs: false, - show_fs_type: false, - show_inode_instead: false, - // block_size: match env::var("BLOCKSIZE") { - // Ok(size) => size.parse().unwrap(), - // Err(_) => 512, - // }, - human_readable_base: -1, - fs_selector: FsSelector::new(), - } - } -} - impl Filesystem { // TODO: resolve uuid in `mount_info.dev_name` if exists fn new(mount_info: MountInfo) -> Option { @@ -293,34 +297,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } - let mut opt = Options::new(); - if matches.is_present(OPT_LOCAL) { - opt.show_local_fs = true; - } - if matches.is_present(OPT_ALL) { - opt.show_all_fs = true; - } - if matches.is_present(OPT_INODES) { - opt.show_inode_instead = true; - } - if matches.is_present(OPT_PRINT_TYPE) { - opt.show_fs_type = true; - } - if matches.is_present(OPT_HUMAN_READABLE) { - opt.human_readable_base = 1024; - } - if matches.is_present(OPT_HUMAN_READABLE_2) { - opt.human_readable_base = 1000; - } - for fs_type in matches.values_of_lossy(OPT_TYPE).unwrap_or_default() { - opt.fs_selector.include(fs_type.to_owned()); - } - for fs_type in matches - .values_of_lossy(OPT_EXCLUDE_TYPE) - .unwrap_or_default() - { - opt.fs_selector.exclude(fs_type.to_owned()); - } + let opt = Options::from(&matches); let fs_list = filter_mount_list(read_fs_list(), &paths, &opt) .into_iter() From ae755bb9bdcf63709a74ff0b66aa1aa30881cca0 Mon Sep 17 00:00:00 2001 From: Guilherme Augusto de Souza <98732503+lguist@users.noreply.github.com> Date: Fri, 4 Feb 2022 06:28:15 -0300 Subject: [PATCH 39/46] test: add version and about (#3011) --- src/uu/test/src/test.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index 1a3f4139e..cc7437bff 100644 --- a/src/uu/test/src/test.rs +++ b/src/uu/test/src/test.rs @@ -10,7 +10,7 @@ mod parser; -use clap::{crate_version, App, AppSettings}; +use clap::{crate_version, App}; use parser::{parse, Operator, Symbol, UnaryOperator}; use std::ffi::{OsStr, OsString}; use uucore::display::Quotable; @@ -86,10 +86,14 @@ NOTE: your shell may have its own version of test and/or [, which usually supers the version described here. Please refer to your shell's documentation for details about the options it supports."; +const ABOUT: &str = "Check file types and compare values."; + pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) - .setting(AppSettings::DisableHelpFlag) - .setting(AppSettings::DisableVersionFlag) + .version(crate_version!()) + .about(ABOUT) + .override_usage(USAGE) + .after_help(AFTER_HELP) } #[uucore::main] @@ -104,6 +108,7 @@ pub fn uumain(mut args: impl uucore::Args) -> UResult<()> { // Let clap pretty-print help and version App::new(binary_name) .version(crate_version!()) + .about(ABOUT) .override_usage(USAGE) .after_help(AFTER_HELP) // Disable printing of -h and -v as valid alternatives for --help and --version, From 793d3dd97c37c5394b152fdace5fdf66d972247c Mon Sep 17 00:00:00 2001 From: Hanif Ariffin Date: Fri, 4 Feb 2022 18:47:19 +0800 Subject: [PATCH 40/46] test_printf: add test for additional escape (\c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using this escaped character will cause `printf` to stop generating characters. For instance, ```rust hbina@akarin ~/g/uutils (hbina-add-test-for-additional-escape)> cargo run --quiet -- printf "%s\c%s" a b a⏎ ``` Signed-off-by: Hanif Ariffin --- tests/by-util/test_printf.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/by-util/test_printf.rs b/tests/by-util/test_printf.rs index b5c9dc3ed..f8f941ad8 100644 --- a/tests/by-util/test_printf.rs +++ b/tests/by-util/test_printf.rs @@ -429,3 +429,11 @@ fn sub_any_specifiers_after_second_param() { .succeeds() .stdout_only("3"); } + +#[test] +fn stop_after_additional_escape() { + new_ucmd!() + .args(&["A%sC\\cD%sF", "B", "E"]) //spell-checker:disable-line + .succeeds() + .stdout_only("ABC"); +} From 5e790918ef556d5afcdef324e2416294d3a29ce2 Mon Sep 17 00:00:00 2001 From: ndd7xv Date: Fri, 4 Feb 2022 21:43:21 -0500 Subject: [PATCH 41/46] printf: use clap default help and version --- src/uu/printf/src/printf.rs | 42 +++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/uu/printf/src/printf.rs b/src/uu/printf/src/printf.rs index f282dc923..a30d18c53 100644 --- a/src/uu/printf/src/printf.rs +++ b/src/uu/printf/src/printf.rs @@ -11,22 +11,13 @@ const VERSION: &str = "version"; const HELP: &str = "help"; const USAGE: &str = "printf FORMATSTRING [ARGUMENT]..."; const ABOUT: &str = "Print output based off of the format string and proceeding arguments."; -static LONGHELP_LEAD: &str = "printf - -USAGE: printf FORMATSTRING [ARGUMENT]... - +const AFTER_HELP: &str = " basic anonymous string templating: prints format string at least once, repeating as long as there are remaining arguments output prints escaped literals in the format string as character literals output replaces anonymous fields with the next unused argument, formatted according to the field. -Options: - --help display this help and exit - --version output version information and exit - -"; -static LONGHELP_BODY: &str = " Prints the , replacing escaped character sequences with character literals and substitution field sequences with passed arguments @@ -273,32 +264,36 @@ COPYRIGHT : "; +mod options { + pub const FORMATSTRING: &str = "FORMATSTRING"; + pub const ARGUMENT: &str = "ARGUMENT"; +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args .collect_str(InvalidEncodingHandling::Ignore) .accept_any(); + let matches = uu_app().get_matches_from(args); - if args.len() <= 1 { - return Err(UUsageError::new(1, "missing operand")); - } - let formatstr = &args[1]; + let format_string = matches + .value_of(options::FORMATSTRING) + .ok_or_else(|| UUsageError::new(1, "missing operand"))?; + let values: Vec = match matches.values_of(options::ARGUMENT) { + Some(s) => s.map(|s| s.to_string()).collect(), + None => vec![], + }; - if formatstr == "--help" { - print!("{} {}", LONGHELP_LEAD, LONGHELP_BODY); - } else if formatstr == "--version" { - println!("{} {}", uucore::util_name(), crate_version!()); - } else { - let printf_args = &args[2..]; - memo::Memo::run_all(formatstr, printf_args); - } + memo::Memo::run_all(format_string, &values[..]); Ok(()) } pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) + .setting(AppSettings::AllowHyphenValues) .version(crate_version!()) .about(ABOUT) + .after_help(AFTER_HELP) .override_usage(USAGE) .arg(Arg::new(HELP).long(HELP).help("Print help information")) .arg( @@ -306,5 +301,6 @@ pub fn uu_app<'a>() -> App<'a> { .long(VERSION) .help("Print version information"), ) - .setting(AppSettings::InferLongArgs) + .arg(Arg::new(options::FORMATSTRING)) + .arg(Arg::new(options::ARGUMENT).multiple_occurrences(true)) } From 162f85773e11b0b0b016d3fdf9379c8fdd6f8530 Mon Sep 17 00:00:00 2001 From: Eli Youngs Date: Sat, 5 Feb 2022 00:43:09 -0800 Subject: [PATCH 42/46] printf: Support leading zeroes with %0n formatting --- src/uucore/src/lib/features/tokenize/sub.rs | 11 ++++++++++- tests/by-util/test_printf.rs | 8 ++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/tokenize/sub.rs b/src/uucore/src/lib/features/tokenize/sub.rs index 6f9196d93..ac471ae3e 100644 --- a/src/uucore/src/lib/features/tokenize/sub.rs +++ b/src/uucore/src/lib/features/tokenize/sub.rs @@ -60,6 +60,7 @@ pub struct Sub { field_char: char, field_type: FieldType, orig: String, + prefix_char: char, } impl Sub { pub fn new( @@ -67,6 +68,7 @@ impl Sub { second_field: CanAsterisk>, field_char: char, orig: String, + prefix_char: char, ) -> Self { // for more dry printing, field characters are grouped // in initialization of token. @@ -90,6 +92,7 @@ impl Sub { field_char, field_type, orig, + prefix_char, } } } @@ -126,6 +129,11 @@ impl SubParser { fn build_token(parser: Self) -> Box { // not a self method so as to allow move of sub-parser vals. // return new Sub struct as token + let prefix_char = match &parser.min_width_tmp { + Some(width) if width.starts_with('0') => '0', + _ => ' ', + }; + let t: Box = Box::new(Sub::new( if parser.min_width_is_asterisk { CanAsterisk::Asterisk @@ -139,6 +147,7 @@ impl SubParser { }, parser.field_char.unwrap(), parser.text_so_far, + prefix_char, )); t } @@ -394,7 +403,7 @@ impl token::Token for Sub { final_str.push_str(&pre_min_width); } for _ in 0..diff { - final_str.push(' '); + final_str.push(self.prefix_char); } if pad_before { final_str.push_str(&pre_min_width); diff --git a/tests/by-util/test_printf.rs b/tests/by-util/test_printf.rs index f8f941ad8..b3e608dc9 100644 --- a/tests/by-util/test_printf.rs +++ b/tests/by-util/test_printf.rs @@ -437,3 +437,11 @@ fn stop_after_additional_escape() { .succeeds() .stdout_only("ABC"); } + +#[test] +fn sub_float_leading_zeroes() { + new_ucmd!() + .args(&["%010f", "1"]) + .succeeds() + .stdout_only("001.000000"); +} From cc61ea807ed0fe1750fa7b2f08b1e83521de3557 Mon Sep 17 00:00:00 2001 From: Roy Ivy III Date: Fri, 4 Feb 2022 17:39:57 -0600 Subject: [PATCH 43/46] docs/CICD ~ add spell-checker exceptions --- .github/workflows/CICD.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 5fd51f852..81147c8dc 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -1,7 +1,7 @@ name: CICD # spell-checker:ignore (acronyms) CICD MSVC musl -# spell-checker:ignore (env/flags) Awarnings Ccodegen Coverflow Cpanic RUSTDOCFLAGS RUSTFLAGS Zpanic +# spell-checker:ignore (env/flags) Awarnings Ccodegen Coverflow Cpanic Dwarnings RUSTDOCFLAGS RUSTFLAGS Zpanic # spell-checker:ignore (jargon) SHAs deps dequote softprops subshell toolchain # spell-checker:ignore (names) CodeCOV MacOS MinGW Peltoche rivy # spell-checker:ignore (shell/tools) choco clippy dmake dpkg esac fakeroot gmake grcov halium lcov libssl mkdir popd printf pushd rustc rustfmt rustup shopt xargs From 578e5c8aba68f43a996a4b9330060ed0560fa1d4 Mon Sep 17 00:00:00 2001 From: Roy Ivy III Date: Thu, 3 Feb 2022 13:42:13 -0600 Subject: [PATCH 44/46] maint/CICD ~ implement 'GnuTests' workflow fixes/refactor - consolidate configuration - DRY improvements - improve flexibility/robustness in the face of missing reference test info - add reference test info IDs and additional logging to help diagnose testing failures - includes parallel refactor of 'util/run-gnu-test.sh' --- .github/workflows/GnuTests.yml | 169 +++++++++++++++++++++------------ util/run-gnu-test.sh | 26 ++++- 2 files changed, 129 insertions(+), 66 deletions(-) diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml index 8303ee403..69a26608c 100644 --- a/.github/workflows/GnuTests.yml +++ b/.github/workflows/GnuTests.yml @@ -1,6 +1,6 @@ name: GnuTests -# spell-checker:ignore (names) gnulib ; (utils) autopoint gperf pyinotify texinfo ; (vars) XPASS +# spell-checker:ignore (names) gnulib ; (people) Dawid Dziurla * dawidd6 ; (utils) autopoint chksum gperf pyinotify shopt texinfo ; (vars) FILESET XPASS on: [push, pull_request] @@ -9,23 +9,55 @@ jobs: name: Run GNU tests runs-on: ubuntu-latest steps: + - name: Initialize workflow variables + id: vars + shell: bash + run: | + ## VARs setup + outputs() { step_id="vars"; for var in "$@" ; do echo steps.${step_id}.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } + # * config + path_GNU="gnu" + path_GNULIB="gnulib" + path_GNU_tests="gnu/tests" + path_UUTILS="uutils" + path_reference="reference" + outputs path_GNU path_GNU_tests path_GNULIB path_reference path_UUTILS + # + repo_GNU_ref="v9.0" + repo_GNULIB_ref="8e99f24c0931a38880c6ee9b8287c7da80b0036b" + repo_reference_branch="${{ github.event.repository.default_branch }}" + outputs repo_GNU_ref repo_GNULIB_ref repo_reference_branch + # + SUITE_LOG_FILE="${path_GNU_tests}/test-suite.log" + TEST_LOGS_GLOB="${path_GNU_tests}/**/*.log" ## note: not usable at bash CLI; [why] double globstar not enabled by default b/c MacOS includes only bash v3 which doesn't have double globstar support + TEST_FILESET_PREFIX='test-fileset-IDs.sha1#' + TEST_FILESET_SUFFIX='.txt' + TEST_SUMMARY_FILE='gnu-result.json' + outputs SUITE_LOG_FILE TEST_FILESET_PREFIX TEST_FILESET_SUFFIX TEST_LOGS_GLOB TEST_SUMMARY_FILE - name: Checkout code uutil uses: actions/checkout@v2 with: - path: 'uutils' + path: '${{ steps.vars.outputs.path_UUTILS }}' - name: Checkout GNU coreutils uses: actions/checkout@v2 with: repository: 'coreutils/coreutils' - path: 'gnu' - ref: v9.0 + path: '${{ steps.vars.outputs.path_GNU }}' + ref: ${{ steps.vars.outputs.repo_GNU_ref }} - name: Checkout GNU coreutils library (gnulib) uses: actions/checkout@v2 with: repository: 'coreutils/gnulib' - path: 'gnulib' - ref: 8e99f24c0931a38880c6ee9b8287c7da80b0036b - fetch-depth: 0 # gnu gets upset if gnulib is a shallow checkout + path: '${{ steps.vars.outputs.path_GNULIB }}' + ref: ${{ steps.vars.outputs.repo_GNULIB_ref }} + fetch-depth: 0 # full depth checkout (o/w gnu gets upset if gnulib is a shallow checkout) + - name: Retrieve reference artifacts + uses: dawidd6/action-download-artifact@v2 + continue-on-error: true ## don't break the build for missing reference artifacts (may be expired or just not generated yet) + with: + workflow: GnuTests.yml + branch: "${{ steps.vars.outputs.repo_reference_branch }}" + path: "${{ steps.vars.outputs.path_reference }}" - name: Install `rust` toolchain uses: actions-rs/toolchain@v1 with: @@ -43,27 +75,33 @@ jobs: shell: bash run: | ## Build binaries - cd uutils + cd '${{ steps.vars.outputs.path_UUTILS }}' bash util/build-gnu.sh - name: Run GNU tests shell: bash run: | - bash uutils/util/run-gnu-test.sh - - name: Extract testing info + path_GNU='${{ steps.vars.outputs.path_GNU }}' + path_GNULIB='${{ steps.vars.outputs.path_GNULIB }}' + path_UUTILS='${{ steps.vars.outputs.path_UUTILS }}' + bash "${path_UUTILS}/util/run-gnu-test.sh" + - name: Extract/summarize testing info + id: summary shell: bash run: | - ## Extract testing info - LOG_FILE=gnu/tests/test-suite.log - if test -f "$LOG_FILE" + ## Extract/summarize testing info + outputs() { step_id="summary"; for var in "$@" ; do echo steps.${step_id}.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } + # + SUITE_LOG_FILE='${{ steps.vars.outputs.SUITE_LOG_FILE }}' + if test -f "${SUITE_LOG_FILE}" then - TOTAL=$(sed -n "s/.*# TOTAL: \(.*\)/\1/p" "$LOG_FILE"|tr -d '\r'|head -n1) - PASS=$(sed -n "s/.*# PASS: \(.*\)/\1/p" "$LOG_FILE"|tr -d '\r'|head -n1) - SKIP=$(sed -n "s/.*# SKIP: \(.*\)/\1/p" "$LOG_FILE"|tr -d '\r'|head -n1) - FAIL=$(sed -n "s/.*# FAIL: \(.*\)/\1/p" "$LOG_FILE"|tr -d '\r'|head -n1) - XPASS=$(sed -n "s/.*# XPASS: \(.*\)/\1/p" "$LOG_FILE"|tr -d '\r'|head -n1) - ERROR=$(sed -n "s/.*# ERROR: \(.*\)/\1/p" "$LOG_FILE"|tr -d '\r'|head -n1) + TOTAL=$(sed -n "s/.*# TOTAL: \(.*\)/\1/p" "${SUITE_LOG_FILE}" | tr -d '\r' | head -n1) + PASS=$(sed -n "s/.*# PASS: \(.*\)/\1/p" "${SUITE_LOG_FILE}" | tr -d '\r' | head -n1) + SKIP=$(sed -n "s/.*# SKIP: \(.*\)/\1/p" "${SUITE_LOG_FILE}" | tr -d '\r' | head -n1) + FAIL=$(sed -n "s/.*# FAIL: \(.*\)/\1/p" "${SUITE_LOG_FILE}" | tr -d '\r' | head -n1) + XPASS=$(sed -n "s/.*# XPASS: \(.*\)/\1/p" "${SUITE_LOG_FILE}" | tr -d '\r' | head -n1) + ERROR=$(sed -n "s/.*# ERROR: \(.*\)/\1/p" "${SUITE_LOG_FILE}" | tr -d '\r' | head -n1) if [[ "$TOTAL" -eq 0 || "$TOTAL" -eq 1 ]]; then - echo "Error in the execution, failing early" + echo "::error ::Failed to parse test results from '${SUITE_LOG_FILE}'; failing early" exit 1 fi output="GNU tests summary = TOTAL: $TOTAL / PASS: $PASS / FAIL: $FAIL / ERROR: $ERROR" @@ -78,54 +116,61 @@ jobs: --arg fail "$FAIL" \ --arg xpass "$XPASS" \ --arg error "$ERROR" \ - '{($date): { sha: $sha, total: $total, pass: $pass, skip: $skip, fail: $fail, xpass: $xpass, error: $error, }}' > gnu-result.json + '{($date): { sha: $sha, total: $total, pass: $pass, skip: $skip, fail: $fail, xpass: $xpass, error: $error, }}' > '${{ steps.vars.outputs.TEST_SUMMARY_FILE }}' + HASH=$(sha1sum '${{ steps.vars.outputs.TEST_SUMMARY_FILE }}' | cut --delim=" " -f 1) + outputs HASH else - echo "::error ::Failed to get summary of test results" + echo "::error ::Failed to find summary of test results (missing '${SUITE_LOG_FILE}'); failing early" + exit 1 fi - - uses: actions/upload-artifact@v2 + - name: Reserve SHA1/ID of 'test-summary' + uses: actions/upload-artifact@v2 with: - name: test-report - path: gnu/tests/**/*.log - - uses: actions/upload-artifact@v2 + name: "${{ steps.summary.outputs.HASH }}" + path: "${{ steps.vars.outputs.TEST_SUMMARY_FILE }}" + - name: Reserve test results summary + uses: actions/upload-artifact@v2 with: - name: gnu-result - path: gnu-result.json - - name: Download the result - uses: dawidd6/action-download-artifact@v2 + name: test-summary + path: "${{ steps.vars.outputs.TEST_SUMMARY_FILE }}" + - name: Reserve test logs + uses: actions/upload-artifact@v2 with: - workflow: GnuTests.yml - name: gnu-result - repo: uutils/coreutils - branch: main - path: dl - - name: Download the log - uses: dawidd6/action-download-artifact@v2 - with: - workflow: GnuTests.yml - name: test-report - repo: uutils/coreutils - branch: main - path: dl - - name: Compare failing tests against main + name: test-logs + path: "${{ steps.vars.outputs.TEST_LOGS_GLOB }}" + - name: Compare test failures VS reference shell: bash run: | - OLD_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" dl/test-suite.log | sort) - NEW_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" gnu/tests/test-suite.log | sort) - for LINE in $OLD_FAILING - do - if ! grep -Fxq $LINE<<<"$NEW_FAILING"; then - echo "::warning ::Congrats! The gnu test $LINE is now passing!" - fi - done - for LINE in $NEW_FAILING - do - if ! grep -Fxq $LINE<<<"$OLD_FAILING" - then - echo "::error ::GNU test failed: $LINE. $LINE is passing on 'main'. Maybe you have to rebase?" - fi - done - - name: Compare against main results + REF_LOG_FILE='${{ steps.vars.outputs.path_reference }}/test-logs/test-suite.log' + REF_SUMMARY_FILE='${{ steps.vars.outputs.path_reference }}/test-summary/gnu-result.json' + if test -f "${REF_LOG_FILE}"; then + echo "Reference SHA1/ID (of '${REF_SUMMARY_FILE}'): $(sha1sum -- "${REF_SUMMARY_FILE}")" + REF_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" "${REF_LOG_FILE}" | sort) + NEW_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" '${{ steps.vars.outputs.path_GNU_tests }}/test-suite.log' | sort) + for LINE in $REF_FAILING + do + if ! grep -Fxq $LINE<<<"$NEW_FAILING"; then + echo "::warning ::Congrats! The gnu test $LINE is now passing!" + fi + done + for LINE in $NEW_FAILING + do + if ! grep -Fxq $LINE<<<"$REF_FAILING" + then + echo "::error ::GNU test failed: $LINE. $LINE is passing on 'main'. Maybe you have to rebase?" + fi + done + else + echo "::warning ::Skipping test failure comparison; no prior reference test logs are available." + fi + - name: Compare test summary VS reference shell: bash run: | - mv dl/gnu-result.json main-gnu-result.json - python uutils/util/compare_gnu_result.py + REF_SUMMARY_FILE='${{ steps.vars.outputs.path_reference }}/test-summary/gnu-result.json' + if test -f "${REF_SUMMARY_FILE}"; then + echo "Reference SHA1/ID (of '${REF_SUMMARY_FILE}'): $(sha1sum -- "${REF_SUMMARY_FILE}")" + mv "${REF_SUMMARY_FILE}" main-gnu-result.json + python uutils/util/compare_gnu_result.py + else + echo "::warning ::Skipping test summary comparison; no prior reference summary is available." + fi diff --git a/util/run-gnu-test.sh b/util/run-gnu-test.sh index ff61e636e..123c4dab2 100755 --- a/util/run-gnu-test.sh +++ b/util/run-gnu-test.sh @@ -1,10 +1,28 @@ #!/bin/bash +# `$0 [TEST]` +# run GNU test (or all tests if TEST is missing/null) # spell-checker:ignore (env/vars) BUILDDIR GNULIB SUBDIRS -cd "$(dirname -- "$(readlink -fm -- "$0")")/../.." + +ME_dir="$(dirname -- "$(readlink -fm -- "$0")")" +REPO_main_dir="$(dirname -- "${ME_dir}")" + set -e -BUILDDIR="${PWD}/uutils/target/release" -GNULIB_DIR="${PWD}/gnulib" -pushd gnu + +### * config (from environment with fallback defaults) + +path_UUTILS=${path_UUTILS:-${REPO_main_dir}} +path_GNU=${path_GNU:-${path_UUTILS}/../gnu} +path_GNULIB=${path_GNULIB:-${path_UUTILS}/../gnulib} + +### + +BUILD_DIR="$(realpath -- "${path_UUTILS}/target/release")" +GNULIB_DIR="$(realpath -- "${path_GNULIB}")" + +export BUILD_DIR +export GNULIB_DIR + +pushd "$(realpath -- "${path_GNU}")" export RUST_BACKTRACE=1 From fec662a6237c243a5d54d2ab3c3b63729d8aefff Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Tue, 1 Feb 2022 23:22:01 -0500 Subject: [PATCH 45/46] dd: show warning when using 0x size multiplier Show a warning when a block size includes "0x" since this is ambiguous: the user may have meant "multiply the next number by zero" or they may have meant "the following characters should be interpreted as a hexadecimal number". --- src/uu/dd/src/parseargs.rs | 8 ++++++++ tests/by-util/test_dd.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index 915a99344..6bc7bcfd9 100644 --- a/src/uu/dd/src/parseargs.rs +++ b/src/uu/dd/src/parseargs.rs @@ -13,6 +13,7 @@ use super::*; use std::error::Error; use uucore::error::UError; use uucore::parse_size::ParseSizeError; +use uucore::show_warning; pub type Matches = ArgMatches; @@ -356,6 +357,13 @@ fn parse_bytes_only(s: &str) -> Result { /// assert_eq!(parse_bytes_no_x("2k").unwrap(), 2 * 1024); /// ``` fn parse_bytes_no_x(s: &str) -> Result { + if s == "0" { + show_warning!( + "{} is a zero multiplier; use {} if that is intended", + "0x".quote(), + "00x".quote() + ); + } let (num, multiplier) = match (s.find('c'), s.rfind('w'), s.rfind('b')) { (None, None, None) => match uucore::parse_size::parse_size(s) { Ok(n) => (n, 1), diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index e9a1f9468..d27122a75 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -198,6 +198,39 @@ fn test_x_multiplier() { .stdout_is("abcdef"); } +#[test] +fn test_zero_multiplier_warning() { + for arg in ["count", "seek", "skip"] { + new_ucmd!() + .args(&[format!("{}=00x1", arg).as_str(), "status=none"]) + .pipe_in("") + .succeeds() + .no_stdout() + .no_stderr(); + + new_ucmd!() + .args(&[format!("{}=0x1", arg).as_str(), "status=none"]) + .pipe_in("") + .succeeds() + .no_stdout() + .stderr_contains("warning: '0x' is a zero multiplier; use '00x' if that is intended"); + + new_ucmd!() + .args(&[format!("{}=0x0x1", arg).as_str(), "status=none"]) + .pipe_in("") + .succeeds() + .no_stdout() + .stderr_is("dd: warning: '0x' is a zero multiplier; use '00x' if that is intended\ndd: warning: '0x' is a zero multiplier; use '00x' if that is intended\n"); + + new_ucmd!() + .args(&[format!("{}=1x0x1", arg).as_str(), "status=none"]) + .pipe_in("") + .succeeds() + .no_stdout() + .stderr_contains("warning: '0x' is a zero multiplier; use '00x' if that is intended"); + } +} + #[test] fn test_final_stats_noxfer() { new_ucmd!() From 572b2e032c509e0deabddbefe2aa7bf29ca86479 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 6 Feb 2022 11:07:19 -0500 Subject: [PATCH 46/46] df: refactor filter_mount_list() to be more flat Use a `for` loop in the `filter_mount_list()` function to make the filtering logic easier to read. --- src/uu/df/src/df.rs | 129 ++++++++++++++++++++++++-------------------- 1 file changed, 72 insertions(+), 57 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 07aa82dc1..e856a6b1e 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -161,64 +161,79 @@ impl Filesystem { } } +/// Keep only the specified subset of [`MountInfo`] instances. +/// +/// If `paths` is non-empty, this function excludes any [`MountInfo`] +/// that is not mounted at the specified path. +/// +/// The `opt` argument specifies a variety of ways of excluding +/// [`MountInfo`] instances; see [`Options`] for more information. +/// +/// Finally, if there are duplicate entries, the one with the shorter +/// path is kept. fn filter_mount_list(vmi: Vec, paths: &[String], opt: &Options) -> Vec { - vmi.into_iter() - .filter_map(|mi| { - if (mi.remote && opt.show_local_fs) - || (mi.dummy && !opt.show_all_fs && !opt.show_listed_fs) - || !opt.fs_selector.should_select(&mi.fs_type) - { - None - } else { - if paths.is_empty() { - // No path specified - return Some((mi.dev_id.clone(), mi)); - } - if paths.contains(&mi.mount_dir) { - // One or more paths have been provided - Some((mi.dev_id.clone(), mi)) - } else { - // Not a path we want to see - None - } - } - }) - .fold( - HashMap::>::new(), - |mut acc, (id, mi)| { - #[allow(clippy::map_entry)] - { - if acc.contains_key(&id) { - let seen = acc[&id].replace(mi.clone()); - let target_nearer_root = seen.mount_dir.len() > mi.mount_dir.len(); - // With bind mounts, prefer items nearer the root of the source - let source_below_root = !seen.mount_root.is_empty() - && !mi.mount_root.is_empty() - && seen.mount_root.len() < mi.mount_root.len(); - // let "real" devices with '/' in the name win. - if (!mi.dev_name.starts_with('/') || seen.dev_name.starts_with('/')) - // let points towards the root of the device win. - && (!target_nearer_root || source_below_root) - // let an entry over-mounted on a new device win... - && (seen.dev_name == mi.dev_name - /* ... but only when matching an existing mnt point, - to avoid problematic replacement when given - inaccurate mount lists, seen with some chroot - environments for example. */ - || seen.mount_dir != mi.mount_dir) - { - acc[&id].replace(seen); - } - } else { - acc.insert(id, Cell::new(mi)); - } - acc - } - }, - ) - .into_iter() - .map(|ent| ent.1.into_inner()) - .collect::>() + let mut mount_info_by_id = HashMap::>::new(); + for mi in vmi { + // Don't show remote filesystems if `--local` has been given. + if mi.remote && opt.show_local_fs { + continue; + } + + // Don't show pseudo filesystems unless `--all` has been given. + if mi.dummy && !opt.show_all_fs && !opt.show_listed_fs { + continue; + } + + // Don't show filesystems if they have been explicitly excluded. + if !opt.fs_selector.should_select(&mi.fs_type) { + continue; + } + + // Don't show filesystems other than the ones specified on the + // command line, if any. + if !paths.is_empty() && !paths.contains(&mi.mount_dir) { + continue; + } + + // If the device ID has not been encountered yet, just store it. + let id = mi.dev_id.clone(); + #[allow(clippy::map_entry)] + if !mount_info_by_id.contains_key(&id) { + mount_info_by_id.insert(id, Cell::new(mi)); + continue; + } + + // Otherwise, if we have seen the current device ID before, + // then check if we need to update it or keep the previously + // seen one. + let seen = mount_info_by_id[&id].replace(mi.clone()); + let target_nearer_root = seen.mount_dir.len() > mi.mount_dir.len(); + // With bind mounts, prefer items nearer the root of the source + let source_below_root = !seen.mount_root.is_empty() + && !mi.mount_root.is_empty() + && seen.mount_root.len() < mi.mount_root.len(); + // let "real" devices with '/' in the name win. + if (!mi.dev_name.starts_with('/') || seen.dev_name.starts_with('/')) + // let points towards the root of the device win. + && (!target_nearer_root || source_below_root) + // let an entry over-mounted on a new device win... + && (seen.dev_name == mi.dev_name + /* ... but only when matching an existing mnt point, + to avoid problematic replacement when given + inaccurate mount lists, seen with some chroot + environments for example. */ + || seen.mount_dir != mi.mount_dir) + { + mount_info_by_id[&id].replace(seen); + } + } + + // Take ownership of the `MountInfo` instances and collect them + // into a `Vec`. + mount_info_by_id + .into_values() + .map(|m| m.into_inner()) + .collect() } /// Convert `value` to a human readable string based on `base`.