Merge branch 'main' of github.com:uutils/coreutils into hbina-test_ls_order_time-is-flaky

.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",
+]

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<T: uucore::Args>(
     args: impl Iterator<Item = OsString>,
-    util_map: UtilityMap<T>,
+    util_map: &UtilityMap<T>,
 ) -> ! {
     let all_utilities: Vec<_> = std::iter::once("coreutils")
         .chain(util_map.keys().copied())
@@ -168,9 +168,9 @@ fn gen_completions<T: uucore::Args>(
     process::exit(0);
 }
 
-fn gen_coreutils_app<T: uucore::Args>(util_map: UtilityMap<T>) -> App<'static> {
+fn gen_coreutils_app<T: uucore::Args>(util_map: &UtilityMap<T>) -> 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

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::gid_t>) -> 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::gid_t>) -> 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());
         }

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(),

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..])

src/uu/false/src/false.rs

@@ -4,16 +4,62 @@
 //  *
 //  * For the full copyright and license information, please view the LICENSE
 //  * file that was distributed with this source code.
+use clap::{App, AppSettings, Arg};
+use std::io::Write;
+use uucore::error::{set_exit_code, UResult};
 
-use clap::App;
+static ABOUT: &str = "\
-use uucore::error::UResult;
+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);
+    let mut app = uu_app();
-    Err(1.into())
+
+    // 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 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);
+        }
+    }
+
+    Ok(())
 }
 
 pub fn uu_app<'a>() -> App<'a> {
     App::new(uucore::util_name())
+        .version(clap::crate_version!())
+        .about(ABOUT)
+        // We provide our own help and version options, to ensure maximum compatibility with GNU.
+        .setting(AppSettings::DisableHelpFlag | AppSettings::DisableVersionFlag)
+        .arg(
+            Arg::new("help")
+                .long("help")
+                .help("Print help information")
+                .exclusive(true),
+        )
+        .arg(
+            Arg::new("version")
+                .long("version")
+                .help("Print version information"),
+        )
 }

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)
 }

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<String>) -> UResult<()> {
+fn list(arg: Option<&String>) -> UResult<()> {
     match arg {
-        Some(ref x) => print_signal(x),
+        Some(x) => print_signal(x),
         None => {
             print_signals();
             Ok(())

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)

src/uu/split/src/number.rs

@@ -96,8 +96,8 @@ impl Number {
     #[allow(dead_code)]
     fn digits(&self) -> &Vec<u8> {
         match self {
-            Number::FixedWidth(number) => &number.digits,
+            Self::FixedWidth(number) => &number.digits,
-            Number::DynamicWidth(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(),
+            Self::FixedWidth(number) => number.increment(),
-            Number::DynamicWidth(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),
+            Self::FixedWidth(number) => number.fmt(f),
-            Number::DynamicWidth(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 {
+    pub fn new(radix: u8, width: usize) -> Self {
-        FixedWidthNumber {
+        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 {
+    pub fn new(radix: u8) -> Self {
-        DynamicWidthNumber {
+        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
         }

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<Self> {
+    fn from(matches: &ArgMatches) -> UResult<Self> {
         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()),

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 }
     }

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,
         }

src/uu/true/src/true.rs

@@ -4,16 +4,59 @@
 //  *
 //  * For the full copyright and license information, please view the LICENSE
 //  * file that was distributed with this source code.
+use clap::{App, AppSettings, Arg};
+use std::io::Write;
+use uucore::error::{set_exit_code, UResult};
 
-use clap::{App, AppSettings};
+static ABOUT: &str = "\
-use uucore::error::UResult;
+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 mut app = uu_app();
+
+    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);
+        }
+    }
+
     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)
+        // We provide our own help and version options, to ensure maximum compatibility with GNU.
+        .setting(AppSettings::DisableHelpFlag | AppSettings::DisableVersionFlag)
+        .arg(
+            Arg::new("help")
+                .long("help")
+                .help("Print help information")
+                .exclusive(true),
+        )
+        .arg(
+            Arg::new("version")
+                .long("version")
+                .help("Print version information"),
+        )
 }

src/uu/truncate/src/truncate.rs

@@ -115,7 +115,14 @@ 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)
+        .map_err(|e| {
+            e.print().expect("Error writing clap::Error");
+            match e.kind {
+                clap::ErrorKind::DisplayHelp | clap::ErrorKind::DisplayVersion => 0,
+                _ => 1,
+            }
+        })?;
 
     let files: Vec<String> = matches
         .values_of(options::ARG_FILES)

src/uucore/src/lib/features/fsext.rs

@@ -238,7 +238,7 @@ impl MountInfo {
         }
     }
     #[cfg(windows)]
-    fn new(mut volume_name: String) -> Option<MountInfo> {
+    fn new(mut volume_name: String) -> Option<Self> {
         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<StatFs> 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.

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()

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"),

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]

tests/by-util/test_false.rs

@@ -1,6 +1,53 @@
 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]
+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() {
+    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");
+    }
+}

tests/by-util/test_true.rs

@@ -1,6 +1,53 @@
 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]
+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() {
+    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");
+    }
+}

tests/by-util/test_truncate.rs

@@ -250,13 +250,26 @@ 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(
+    new_ucmd!()
+        .args(&["file"])
+        .fails()
+        .code_is(1)
+        .stderr_contains(
             "error: The following required arguments were not provided:
     --reference <RFILE>
     --size <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]
 fn test_invalid_numbers() {
     new_ucmd!()