Merge branch 'master' into ls/fix_backslash_escape

Cargo.lock

@@ -1,7 +1,5 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
-version = 3
-
 [[package]]
 name = "advapi32-sys"
 version = "0.2.0"

README.md

@@ -93,7 +93,7 @@ $ cargo build --features "base32 cat echo rm" --no-default-features
 
 If you don't want to build the multicall binary and would prefer to build
 the utilities as individual binaries, that is also possible. Each utility
-is contained in it's own package within the main repository, named
+is contained in its own package within the main repository, named
 "uu_UTILNAME". To build individual utilities, use cargo to build just the
 specific packages (using the `--package` [aka `-p`] option). For example:
 

src/uu/basename/src/basename.rs

@@ -111,6 +111,7 @@ fn basename(fullname: &str, suffix: &str) -> String {
     }
 }
 
+#[allow(clippy::manual_strip)] // can be replaced with strip_suffix once the minimum rust version is 1.45
 fn strip_suffix(name: &str, suffix: &str) -> String {
     if name == suffix {
         return name.to_owned();

src/uu/chgrp/src/chgrp.rs

@@ -286,7 +286,7 @@ impl Chgrper {
 
             ret = match wrap_chgrp(path, &meta, self.dest_gid, follow, self.verbosity.clone()) {
                 Ok(n) => {
-                    if n != "" {
+                    if !n.is_empty() {
                         show_info!("{}", n);
                     }
                     0

src/uu/chmod/src/chmod.rs

@@ -171,13 +171,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 // of a prefix '-' if it's associated with MODE
 // e.g. "chmod -v -xw -R FILE" -> "chmod -v xw -R FILE"
 pub fn strip_minus_from_mode(args: &mut Vec<String>) -> bool {
-    for i in 0..args.len() {
+    for arg in args {
-        if args[i].starts_with("-") {
+        if arg.starts_with('-') {
-            if let Some(second) = args[i].chars().nth(1) {
+            if let Some(second) = arg.chars().nth(1) {
                 match second {
                     'r' | 'w' | 'x' | 'X' | 's' | 't' | 'u' | 'g' | 'o' | '0'..='7' => {
                         // TODO: use strip_prefix() once minimum rust version reaches 1.45.0
-                        args[i] = args[i][1..args[i].len()].to_string();
+                        *arg = arg[1..arg.len()].to_string();
                         return true;
                     }
                     _ => {}

src/uu/chown/src/chown.rs

@@ -272,16 +272,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 }
 
 fn parse_spec(spec: &str) -> Result<(Option<u32>, Option<u32>), String> {
-    let args = spec.split(':').collect::<Vec<_>>();
+    let args = spec.split_terminator(':').collect::<Vec<_>>();
-    let usr_only = args.len() == 1;
+    let usr_only = args.len() == 1 && !args[0].is_empty();
-    let grp_only = args.len() == 2 && args[0].is_empty() && !args[1].is_empty();
+    let grp_only = args.len() == 2 && args[0].is_empty();
     let usr_grp = args.len() == 2 && !args[0].is_empty() && !args[1].is_empty();
 
     if usr_only {
         Ok((
             Some(match Passwd::locate(args[0]) {
                 Ok(v) => v.uid(),
-                _ => return Err(format!("invalid user: '{}'", spec)),
+                _ => return Err(format!("invalid user: ‘{}’", spec)),
             }),
             None,
         ))
@@ -290,18 +290,18 @@ fn parse_spec(spec: &str) -> Result<(Option<u32>, Option<u32>), String> {
             None,
             Some(match Group::locate(args[1]) {
                 Ok(v) => v.gid(),
-                _ => return Err(format!("invalid group: '{}'", spec)),
+                _ => return Err(format!("invalid group: ‘{}’", spec)),
             }),
         ))
     } else if usr_grp {
         Ok((
             Some(match Passwd::locate(args[0]) {
                 Ok(v) => v.uid(),
-                _ => return Err(format!("invalid user: '{}'", spec)),
+                _ => return Err(format!("invalid user: ‘{}’", spec)),
             }),
             Some(match Group::locate(args[1]) {
                 Ok(v) => v.gid(),
-                _ => return Err(format!("invalid group: '{}'", spec)),
+                _ => return Err(format!("invalid group: ‘{}’", spec)),
             }),
         ))
     } else {
@@ -391,7 +391,7 @@ impl Chowner {
                 self.verbosity.clone(),
             ) {
                 Ok(n) => {
-                    if n != "" {
+                    if !n.is_empty() {
                         show_info!("{}", n);
                     }
                     0
@@ -446,7 +446,7 @@ impl Chowner {
                 self.verbosity.clone(),
             ) {
                 Ok(n) => {
-                    if n != "" {
+                    if !n.is_empty() {
                         show_info!("{}", n);
                     }
                     0

src/uu/chroot/src/chroot.rs

@@ -104,7 +104,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         _ => {
             let mut vector: Vec<&str> = Vec::new();
             for (&k, v) in matches.args.iter() {
-                vector.push(k.clone());
+                vector.push(k);
                 vector.push(&v.vals[0].to_str().unwrap());
             }
             vector
@@ -133,7 +133,7 @@ fn set_context(root: &Path, options: &clap::ArgMatches) {
     let userspec = match userspec_str {
         Some(ref u) => {
             let s: Vec<&str> = u.split(':').collect();
-            if s.len() != 2 || s.iter().any(|&spec| spec == "") {
+            if s.len() != 2 || s.iter().any(|&spec| spec.is_empty()) {
                 crash!(1, "invalid userspec: `{}`", u)
             };
             s
@@ -142,16 +142,16 @@ fn set_context(root: &Path, options: &clap::ArgMatches) {
     };
 
     let (user, group) = if userspec.is_empty() {
-        (&user_str[..], &group_str[..])
+        (user_str, group_str)
     } else {
-        (&userspec[0][..], &userspec[1][..])
+        (userspec[0], userspec[1])
     };
 
     enter_chroot(root);
 
-    set_groups_from_str(&groups_str[..]);
+    set_groups_from_str(groups_str);
-    set_main_group(&group[..]);
+    set_main_group(group);
-    set_user(&user[..]);
+    set_user(user);
 }
 
 fn enter_chroot(root: &Path) {

src/uu/cp/src/cp.rs

@@ -132,7 +132,9 @@ macro_rules! prompt_yes(
 
 pub type CopyResult<T> = Result<T, Error>;
 pub type Source = PathBuf;
+pub type SourceSlice = Path;
 pub type Target = PathBuf;
+pub type TargetSlice = Path;
 
 /// Specifies whether when overwrite files
 #[derive(Clone, Eq, PartialEq)]
@@ -547,14 +549,13 @@ impl FromStr for Attribute {
 }
 
 fn add_all_attributes() -> Vec<Attribute> {
-    let mut attr = Vec::new();
+    use Attribute::*;
+
+    let mut attr = vec![Ownership, Timestamps, Context, Xattr, Links];
+
     #[cfg(unix)]
-    attr.push(Attribute::Mode);
+    attr.insert(0, Mode);
-    attr.push(Attribute::Ownership);
+
-    attr.push(Attribute::Timestamps);
-    attr.push(Attribute::Context);
-    attr.push(Attribute::Xattr);
-    attr.push(Attribute::Links);
     attr
 }
 
@@ -665,7 +666,7 @@ impl TargetType {
     ///
     /// Treat target as a dir if we have multiple sources or the target
     /// exists and already is a directory
-    fn determine(sources: &[Source], target: &Target) -> TargetType {
+    fn determine(sources: &[Source], target: &TargetSlice) -> TargetType {
         if sources.len() > 1 || target.is_dir() {
             TargetType::Directory
         } else {
@@ -714,7 +715,7 @@ fn parse_path_args(path_args: &[String], options: &Options) -> CopyResult<(Vec<S
 
 fn preserve_hardlinks(
     hard_links: &mut Vec<(String, u64)>,
-    source: &std::path::PathBuf,
+    source: &std::path::Path,
     dest: std::path::PathBuf,
     found_hard_link: &mut bool,
 ) -> CopyResult<()> {
@@ -788,7 +789,7 @@ fn preserve_hardlinks(
 /// Behavior depends on `options`, see [`Options`] for details.
 ///
 /// [`Options`]: ./struct.Options.html
-fn copy(sources: &[Source], target: &Target, options: &Options) -> CopyResult<()> {
+fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResult<()> {
     let target_type = TargetType::determine(sources, target);
     verify_target_type(target, &target_type)?;
 
@@ -840,7 +841,7 @@ fn copy(sources: &[Source], target: &Target, options: &Options) -> CopyResult<()
 
 fn construct_dest_path(
     source_path: &Path,
-    target: &Target,
+    target: &TargetSlice,
     target_type: &TargetType,
     options: &Options,
 ) -> CopyResult<PathBuf> {
@@ -870,8 +871,8 @@ fn construct_dest_path(
 }
 
 fn copy_source(
-    source: &Source,
+    source: &SourceSlice,
-    target: &Target,
+    target: &TargetSlice,
     target_type: &TargetType,
     options: &Options,
 ) -> CopyResult<()> {
@@ -912,7 +913,7 @@ fn adjust_canonicalization(p: &Path) -> Cow<Path> {
 ///
 /// Any errors encountered copying files in the tree will be logged but
 /// will not cause a short-circuit.
-fn copy_directory(root: &Path, target: &Target, options: &Options) -> CopyResult<()> {
+fn copy_directory(root: &Path, target: &TargetSlice, options: &Options) -> CopyResult<()> {
     if !options.recursive {
         return Err(format!("omitting directory '{}'", root.display()).into());
     }
@@ -1068,6 +1069,7 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu
 }
 
 #[cfg(not(windows))]
+#[allow(clippy::unnecessary_wraps)] // needed for windows version
 fn symlink_file(source: &Path, dest: &Path, context: &str) -> CopyResult<()> {
     match std::os::unix::fs::symlink(source, dest).context(context) {
         Ok(_) => Ok(()),

src/uu/cut/src/cut.rs

@@ -406,7 +406,7 @@ fn cut_files(mut filenames: Vec<String>, mode: Mode) -> i32 {
                 continue;
             }
 
-            if !path.metadata().is_ok() {
+            if path.metadata().is_err() {
                 show_error!("{}: No such file or directory", filename);
                 continue;
             }
@@ -487,7 +487,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 .help("filter field columns from the input source")
                 .takes_value(true)
                 .allow_hyphen_values(true)
-                .value_name("LIST")                
+                .value_name("LIST")
                 .display_order(4),
         )
         .arg(
@@ -535,40 +535,36 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         matches.value_of(options::CHARACTERS),
         matches.value_of(options::FIELDS),
     ) {
-        (Some(byte_ranges), None, None) => {
+        (Some(byte_ranges), None, None) => list_to_ranges(byte_ranges, complement).map(|ranges| {
-            list_to_ranges(&byte_ranges[..], complement).map(|ranges| {
+            Mode::Bytes(
-                Mode::Bytes(
+                ranges,
-                    ranges,
+                Options {
-                    Options {
+                    out_delim: Some(
-                        out_delim: Some(
+                        matches
-                            matches
+                            .value_of(options::OUTPUT_DELIMITER)
-                                .value_of(options::OUTPUT_DELIMITER)
+                            .unwrap_or_default()
-                                .unwrap_or_default()
+                            .to_owned(),
-                                .to_owned(),
+                    ),
-                        ),
+                    zero_terminated: matches.is_present(options::ZERO_TERMINATED),
-                        zero_terminated: matches.is_present(options::ZERO_TERMINATED),
+                },
-                    },
+            )
-                )
+        }),
-            })
+        (None, Some(char_ranges), None) => list_to_ranges(char_ranges, complement).map(|ranges| {
-        }
+            Mode::Characters(
-        (None, Some(char_ranges), None) => {
+                ranges,
-            list_to_ranges(&char_ranges[..], complement).map(|ranges| {
+                Options {
-                Mode::Characters(
+                    out_delim: Some(
-                    ranges,
+                        matches
-                    Options {
+                            .value_of(options::OUTPUT_DELIMITER)
-                        out_delim: Some(
+                            .unwrap_or_default()
-                            matches
+                            .to_owned(),
-                                .value_of(options::OUTPUT_DELIMITER)
+                    ),
-                                .unwrap_or_default()
+                    zero_terminated: matches.is_present(options::ZERO_TERMINATED),
-                                .to_owned(),
+                },
-                        ),
+            )
-                        zero_terminated: matches.is_present(options::ZERO_TERMINATED),
+        }),
-                    },
-                )
-            })
-        }
         (None, None, Some(field_ranges)) => {
-            list_to_ranges(&field_ranges[..], complement).and_then(|ranges| {
+            list_to_ranges(field_ranges, complement).and_then(|ranges| {
                 let out_delim = match matches.value_of(options::OUTPUT_DELIMITER) {
                     Some(s) => {
                         if s.is_empty() {

src/uu/df/src/df.rs

@@ -116,7 +116,6 @@ struct Options {
     show_listed_fs: bool,
     show_fs_type: bool,
     show_inode_instead: bool,
-    print_grand_total: bool,
     // block_size: usize,
     human_readable_base: i64,
     fs_selector: FsSelector,
@@ -286,7 +285,6 @@ impl Options {
             show_listed_fs: false,
             show_fs_type: false,
             show_inode_instead: false,
-            print_grand_total: false,
             // block_size: match env::var("BLOCKSIZE") {
             //     Ok(size) => size.parse().unwrap(),
             //     Err(_) => 512,
@@ -871,9 +869,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     if matches.is_present(OPT_ALL) {
         opt.show_all_fs = true;
     }
-    if matches.is_present(OPT_TOTAL) {
-        opt.print_grand_total = true;
-    }
     if matches.is_present(OPT_INODES) {
         opt.show_inode_instead = true;
     }

src/uu/du/src/du.rs

@@ -15,7 +15,7 @@ use chrono::Local;
 use std::collections::HashSet;
 use std::env;
 use std::fs;
-use std::io::{stderr, Result, Write};
+use std::io::{stderr, ErrorKind, Result, Write};
 use std::iter;
 #[cfg(not(windows))]
 use std::os::unix::fs::MetadataExt;
@@ -296,7 +296,21 @@ fn du(
                             }
                         }
                     }
-                    Err(error) => show_error!("{}", error),
+                    Err(error) => match error.kind() {
+                        ErrorKind::PermissionDenied => {
+                            let description = format!(
+                                "cannot access '{}'",
+                                entry
+                                    .path()
+                                    .as_os_str()
+                                    .to_str()
+                                    .unwrap_or("<Un-printable path>")
+                            );
+                            let error_message = "Permission denied";
+                            show_error_custom_description!(description, "{}", error_message)
+                        }
+                        _ => show_error!("{}", error),
+                    },
                 },
                 Err(error) => show_error!("{}", error),
             }
@@ -322,7 +336,7 @@ fn convert_size_human(size: u64, multiplier: u64, _block_size: u64) -> String {
         }
     }
     if size == 0 {
-        return format!("0");
+        return "0".to_string();
     }
     format!("{}B", size)
 }
@@ -486,7 +500,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     };
 
     let strs = if matches.free.is_empty() {
-        vec!["./".to_owned()]
+        vec!["./".to_owned()] // TODO: gnu `du` doesn't use trailing "/" here
     } else {
         matches.free.clone()
     };

src/uu/expr/src/expr.rs

@@ -51,7 +51,7 @@ fn print_expr_error(expr_error: &str) -> ! {
     crash!(2, "{}", expr_error)
 }
 
-fn evaluate_ast(maybe_ast: Result<Box<syntax_tree::ASTNode>, String>) -> Result<String, String> {
+fn evaluate_ast(maybe_ast: Result<Box<syntax_tree::AstNode>, String>) -> Result<String, String> {
     if maybe_ast.is_err() {
         Err(maybe_ast.err().unwrap())
     } else {

src/uu/expr/src/syntax_tree.rs

@@ -17,10 +17,10 @@ use onig::{Regex, RegexOptions, Syntax};
 use crate::tokens::Token;
 
 type TokenStack = Vec<(usize, Token)>;
-pub type OperandsList = Vec<Box<ASTNode>>;
+pub type OperandsList = Vec<Box<AstNode>>;
 
 #[derive(Debug)]
-pub enum ASTNode {
+pub enum AstNode {
     Leaf {
         token_idx: usize,
         value: String,
@@ -31,7 +31,7 @@ pub enum ASTNode {
         operands: OperandsList,
     },
 }
-impl ASTNode {
+impl AstNode {
     fn debug_dump(&self) {
         self.debug_dump_impl(1);
     }
@@ -40,7 +40,7 @@ impl ASTNode {
             print!("\t",);
         }
         match *self {
-            ASTNode::Leaf {
+            AstNode::Leaf {
                 ref token_idx,
                 ref value,
             } => println!(
@@ -49,7 +49,7 @@ impl ASTNode {
                 token_idx,
                 self.evaluate()
             ),
-            ASTNode::Node {
+            AstNode::Node {
                 ref token_idx,
                 ref op_type,
                 ref operands,
@@ -67,23 +67,23 @@ impl ASTNode {
         }
     }
 
-    fn new_node(token_idx: usize, op_type: &str, operands: OperandsList) -> Box<ASTNode> {
+    fn new_node(token_idx: usize, op_type: &str, operands: OperandsList) -> Box<AstNode> {
-        Box::new(ASTNode::Node {
+        Box::new(AstNode::Node {
             token_idx,
             op_type: op_type.into(),
             operands,
         })
     }
-    fn new_leaf(token_idx: usize, value: &str) -> Box<ASTNode> {
+    fn new_leaf(token_idx: usize, value: &str) -> Box<AstNode> {
-        Box::new(ASTNode::Leaf {
+        Box::new(AstNode::Leaf {
             token_idx,
             value: value.into(),
         })
     }
     pub fn evaluate(&self) -> Result<String, String> {
         match *self {
-            ASTNode::Leaf { ref value, .. } => Ok(value.clone()),
+            AstNode::Leaf { ref value, .. } => Ok(value.clone()),
-            ASTNode::Node { ref op_type, .. } => match self.operand_values() {
+            AstNode::Node { ref op_type, .. } => match self.operand_values() {
                 Err(reason) => Err(reason),
                 Ok(operand_values) => match op_type.as_ref() {
                     "+" => infix_operator_two_ints(
@@ -161,7 +161,7 @@ impl ASTNode {
         }
     }
     pub fn operand_values(&self) -> Result<Vec<String>, String> {
-        if let ASTNode::Node { ref operands, .. } = *self {
+        if let AstNode::Node { ref operands, .. } = *self {
             let mut out = Vec::with_capacity(operands.len());
             for operand in operands {
                 match operand.evaluate() {
@@ -178,7 +178,7 @@ impl ASTNode {
 
 pub fn tokens_to_ast(
     maybe_tokens: Result<Vec<(usize, Token)>, String>,
-) -> Result<Box<ASTNode>, String> {
+) -> Result<Box<AstNode>, String> {
     if maybe_tokens.is_err() {
         Err(maybe_tokens.err().unwrap())
     } else {
@@ -212,7 +212,7 @@ pub fn tokens_to_ast(
     }
 }
 
-fn maybe_dump_ast(result: &Result<Box<ASTNode>, String>) {
+fn maybe_dump_ast(result: &Result<Box<AstNode>, String>) {
     use std::env;
     if let Ok(debug_var) = env::var("EXPR_DEBUG_AST") {
         if debug_var == "1" {
@@ -238,11 +238,11 @@ fn maybe_dump_rpn(rpn: &TokenStack) {
     }
 }
 
-fn ast_from_rpn(rpn: &mut TokenStack) -> Result<Box<ASTNode>, String> {
+fn ast_from_rpn(rpn: &mut TokenStack) -> Result<Box<AstNode>, String> {
     match rpn.pop() {
         None => Err("syntax error (premature end of expression)".to_owned()),
 
-        Some((token_idx, Token::Value { value })) => Ok(ASTNode::new_leaf(token_idx, &value)),
+        Some((token_idx, Token::Value { value })) => Ok(AstNode::new_leaf(token_idx, &value)),
 
         Some((token_idx, Token::InfixOp { value, .. })) => {
             maybe_ast_node(token_idx, &value, 2, rpn)
@@ -262,7 +262,7 @@ fn maybe_ast_node(
     op_type: &str,
     arity: usize,
     rpn: &mut TokenStack,
-) -> Result<Box<ASTNode>, String> {
+) -> Result<Box<AstNode>, String> {
     let mut operands = Vec::with_capacity(arity);
     for _ in 0..arity {
         match ast_from_rpn(rpn) {
@@ -271,7 +271,7 @@ fn maybe_ast_node(
         }
     }
     operands.reverse();
-    Ok(ASTNode::new_node(token_idx, op_type, operands))
+    Ok(AstNode::new_node(token_idx, op_type, operands))
 }
 
 fn move_rest_of_ops_to_out(

src/uu/fmt/src/parasplit.rs

@@ -267,7 +267,7 @@ impl<'a> ParagraphStream<'a> {
                 #[allow(clippy::match_like_matches_macro)]
                 // `matches!(...)` macro not stabilized until rust v1.42
                 l_slice[..colon_posn].chars().all(|x| match x as usize {
-                    y if y < 33 || y > 126 => false,
+                    y if !(33..=126).contains(&y) => false,
                     _ => true,
                 })
             }

src/uu/fold/src/fold.rs

@@ -66,7 +66,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 .takes_value(true),
         )
         .arg(Arg::with_name(options::FILE).hidden(true).multiple(true))
-        .get_matches_from(args.clone());
+        .get_matches_from(args);
 
     let bytes = matches.is_present(options::BYTES);
     let spaces = matches.is_present(options::SPACES);

src/uu/hashsum/src/hashsum.rs

@@ -78,7 +78,7 @@ fn detect_algo<'a>(
         "sha512sum" => ("SHA512", Box::new(Sha512::new()) as Box<dyn Digest>, 512),
         "b2sum" => ("BLAKE2", Box::new(Blake2b::new(64)) as Box<dyn Digest>, 512),
         "sha3sum" => match matches.value_of("bits") {
-            Some(bits_str) => match usize::from_str_radix(&bits_str, 10) {
+            Some(bits_str) => match (&bits_str).parse::<usize>() {
                 Ok(224) => (
                     "SHA3-224",
                     Box::new(Sha3_224::new()) as Box<dyn Digest>,
@@ -128,7 +128,7 @@ fn detect_algo<'a>(
             512,
         ),
         "shake128sum" => match matches.value_of("bits") {
-            Some(bits_str) => match usize::from_str_radix(&bits_str, 10) {
+            Some(bits_str) => match (&bits_str).parse::<usize>() {
                 Ok(bits) => (
                     "SHAKE128",
                     Box::new(Shake128::new()) as Box<dyn Digest>,
@@ -139,7 +139,7 @@ fn detect_algo<'a>(
             None => crash!(1, "--bits required for SHAKE-128"),
         },
         "shake256sum" => match matches.value_of("bits") {
-            Some(bits_str) => match usize::from_str_radix(&bits_str, 10) {
+            Some(bits_str) => match (&bits_str).parse::<usize>() {
                 Ok(bits) => (
                     "SHAKE256",
                     Box::new(Shake256::new()) as Box<dyn Digest>,
@@ -182,7 +182,7 @@ fn detect_algo<'a>(
                 }
                 if matches.is_present("sha3") {
                     match matches.value_of("bits") {
-                        Some(bits_str) => match usize::from_str_radix(&bits_str, 10) {
+                        Some(bits_str) => match (&bits_str).parse::<usize>() {
                             Ok(224) => set_or_crash(
                                 "SHA3-224",
                                 Box::new(Sha3_224::new()) as Box<dyn Digest>,
@@ -226,7 +226,7 @@ fn detect_algo<'a>(
                 }
                 if matches.is_present("shake128") {
                     match matches.value_of("bits") {
-                        Some(bits_str) => match usize::from_str_radix(&bits_str, 10) {
+                        Some(bits_str) => match (&bits_str).parse::<usize>() {
                             Ok(bits) => set_or_crash("SHAKE128", Box::new(Shake128::new()), bits),
                             Err(err) => crash!(1, "{}", err),
                         },
@@ -235,7 +235,7 @@ fn detect_algo<'a>(
                 }
                 if matches.is_present("shake256") {
                     match matches.value_of("bits") {
-                        Some(bits_str) => match usize::from_str_radix(&bits_str, 10) {
+                        Some(bits_str) => match (&bits_str).parse::<usize>() {
                             Ok(bits) => set_or_crash("SHAKE256", Box::new(Shake256::new()), bits),
                             Err(err) => crash!(1, "{}", err),
                         },
@@ -253,7 +253,7 @@ fn detect_algo<'a>(
 
 // TODO: return custom error type
 fn parse_bit_num(arg: &str) -> Result<usize, ParseIntError> {
-    usize::from_str_radix(arg, 10)
+    arg.parse()
 }
 
 fn is_valid_bit_num(arg: String) -> Result<(), String> {

src/uu/head/src/head.rs

@@ -625,7 +625,7 @@ mod tests {
         assert_eq!(arg_outputs("head"), Ok("head".to_owned()));
     }
     #[test]
-    #[cfg(linux)]
+    #[cfg(target_os = "linux")]
     fn test_arg_iterate_bad_encoding() {
         let invalid = unsafe { std::str::from_utf8_unchecked(b"\x80\x81") };
         // this arises from a conversion from OsString to &str

src/uu/install/src/install.rs

@@ -41,6 +41,7 @@ pub struct Behavior {
     compare: bool,
     strip: bool,
     strip_program: String,
+    create_leading: bool,
 }
 
 #[derive(Clone, Eq, PartialEq)]
@@ -70,7 +71,7 @@ static OPT_BACKUP: &str = "backup";
 static OPT_BACKUP_2: &str = "backup2";
 static OPT_DIRECTORY: &str = "directory";
 static OPT_IGNORED: &str = "ignored";
-static OPT_CREATED: &str = "created";
+static OPT_CREATE_LEADING: &str = "create-leading";
 static OPT_GROUP: &str = "group";
 static OPT_MODE: &str = "mode";
 static OPT_OWNER: &str = "owner";
@@ -133,9 +134,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
         .arg(
             // TODO implement flag
-            Arg::with_name(OPT_CREATED)
+            Arg::with_name(OPT_CREATE_LEADING)
                 .short("D")
-                .help("(unimplemented) create all leading components of DEST except the last, then copy SOURCE to DEST")
+                .help("create all leading components of DEST except the last, then copy SOURCE to DEST")
         )
         .arg(
             Arg::with_name(OPT_GROUP)
@@ -266,8 +267,6 @@ fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> {
         Err("--backup")
     } else if matches.is_present(OPT_BACKUP_2) {
         Err("-b")
-    } else if matches.is_present(OPT_CREATED) {
-        Err("-D")
     } else if matches.is_present(OPT_SUFFIX) {
         Err("--suffix, -S")
     } else if matches.is_present(OPT_TARGET_DIRECTORY) {
@@ -302,7 +301,7 @@ fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
 
     let specified_mode: Option<u32> = if matches.is_present(OPT_MODE) {
         match matches.value_of(OPT_MODE) {
-            Some(x) => match mode::parse(&x[..], considering_dir) {
+            Some(x) => match mode::parse(x, considering_dir) {
                 Ok(y) => Some(y),
                 Err(err) => {
                     show_error!("Invalid mode string: {}", err);
@@ -343,6 +342,7 @@ fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
                 .value_of(OPT_STRIP_PROGRAM)
                 .unwrap_or(DEFAULT_STRIP_PROGRAM),
         ),
+        create_leading: matches.is_present(OPT_CREATE_LEADING),
     })
 }
 
@@ -410,12 +410,35 @@ fn standard(paths: Vec<String>, b: Behavior) -> i32 {
         .iter()
         .map(PathBuf::from)
         .collect::<Vec<_>>();
+
     let target = Path::new(paths.last().unwrap());
 
-    if (target.is_file() || is_new_file_path(target)) && sources.len() == 1 {
+    if sources.len() > 1 || (target.exists() && target.is_dir()) {
-        copy_file_to_file(&sources[0], &target.to_path_buf(), &b)
-    } else {
         copy_files_into_dir(sources, &target.to_path_buf(), &b)
+    } else {
+        if let Some(parent) = target.parent() {
+            if !parent.exists() && b.create_leading {
+                if let Err(e) = fs::create_dir_all(parent) {
+                    show_error!("failed to create {}: {}", parent.display(), e);
+                    return 1;
+                }
+
+                if mode::chmod(&parent, b.mode()).is_err() {
+                    show_error!("failed to chmod {}", parent.display());
+                    return 1;
+                }
+            }
+        }
+
+        if target.is_file() || is_new_file_path(target) {
+            copy_file_to_file(&sources[0], &target.to_path_buf(), &b)
+        } else {
+            show_error!(
+                "invalid target {}: No such file or directory",
+                target.display()
+            );
+            1
+        }
     }
 }
 
@@ -429,7 +452,7 @@ fn standard(paths: Vec<String>, b: Behavior) -> i32 {
 /// _files_ must all exist as non-directories.
 /// _target_dir_ must be a directory.
 ///
-fn copy_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) -> i32 {
+fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i32 {
     if !target_dir.is_dir() {
         show_error!("target '{}' is not a directory", target_dir.display());
         return 1;
@@ -453,7 +476,7 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) ->
             continue;
         }
 
-        let mut targetpath = target_dir.clone().to_path_buf();
+        let mut targetpath = target_dir.to_path_buf();
         let filename = sourcepath.components().last().unwrap();
         targetpath.push(filename);
 
@@ -478,7 +501,7 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) ->
 /// _file_ must exist as a non-directory.
 /// _target_ must be a non-directory
 ///
-fn copy_file_to_file(file: &PathBuf, target: &PathBuf, b: &Behavior) -> i32 {
+fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 {
     if copy(file, &target, b).is_err() {
         1
     } else {
@@ -497,7 +520,7 @@ fn copy_file_to_file(file: &PathBuf, target: &PathBuf, b: &Behavior) -> i32 {
 ///
 /// If the copy system call fails, we print a verbose error and return an empty error value.
 ///
-fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> {
+fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
     if b.compare && !need_copy(from, to, b) {
         return Ok(());
     }
@@ -556,7 +579,7 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> {
         };
         let gid = meta.gid();
         match wrap_chown(
-            to.as_path(),
+            to,
             &meta,
             Some(owner_id),
             Some(gid),
@@ -582,7 +605,7 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> {
             Ok(g) => g,
             _ => crash!(1, "no such group: {}", b.group),
         };
-        match wrap_chgrp(to.as_path(), &meta, group_id, false, Verbosity::Normal) {
+        match wrap_chgrp(to, &meta, group_id, false, Verbosity::Normal) {
             Ok(n) => {
                 if !n.is_empty() {
                     show_info!("{}", n);
@@ -601,7 +624,7 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> {
         let modified_time = FileTime::from_last_modification_time(&meta);
         let accessed_time = FileTime::from_last_access_time(&meta);
 
-        match set_file_times(to.as_path(), accessed_time, modified_time) {
+        match set_file_times(to, accessed_time, modified_time) {
             Ok(_) => {}
             Err(e) => show_info!("{}", e),
         }
@@ -630,7 +653,7 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> {
 ///
 /// Crashes the program if a nonexistent owner or group is specified in _b_.
 ///
-fn need_copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> bool {
+fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool {
     let from_meta = match fs::metadata(from) {
         Ok(meta) => meta,
         Err(_) => return true,

src/uu/ln/src/ln.rs

@@ -303,7 +303,7 @@ fn exec(files: &[PathBuf], settings: &Settings) -> i32 {
     }
 }
 
-fn link_files_in_dir(files: &[PathBuf], target_dir: &PathBuf, settings: &Settings) -> i32 {
+fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) -> i32 {
     if !target_dir.is_dir() {
         show_error!("target '{}' is not a directory", target_dir.display());
         return 1;
@@ -329,7 +329,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &PathBuf, settings: &Setting
                     };
                 }
             }
-            target_dir.clone()
+            target_dir.to_path_buf()
         } else {
             match srcpath.as_os_str().to_str() {
                 Some(name) => {
@@ -370,7 +370,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &PathBuf, settings: &Setting
     }
 }
 
-fn relative_path<'a>(src: &PathBuf, dst: &PathBuf) -> Result<Cow<'a, Path>> {
+fn relative_path<'a>(src: &Path, dst: &Path) -> Result<Cow<'a, Path>> {
     let abssrc = canonicalize(src, CanonicalizeMode::Normal)?;
     let absdst = canonicalize(dst, CanonicalizeMode::Normal)?;
     let suffix_pos = abssrc
@@ -390,7 +390,7 @@ fn relative_path<'a>(src: &PathBuf, dst: &PathBuf) -> Result<Cow<'a, Path>> {
     Ok(result.into())
 }
 
-fn link(src: &PathBuf, dst: &PathBuf, settings: &Settings) -> Result<()> {
+fn link(src: &Path, dst: &Path, settings: &Settings) -> Result<()> {
     let mut backup_path = None;
     let source: Cow<'_, Path> = if settings.relative {
         relative_path(&src, dst)?
@@ -453,13 +453,13 @@ fn read_yes() -> bool {
     }
 }
 
-fn simple_backup_path(path: &PathBuf, suffix: &str) -> PathBuf {
+fn simple_backup_path(path: &Path, suffix: &str) -> PathBuf {
     let mut p = path.as_os_str().to_str().unwrap().to_owned();
     p.push_str(suffix);
     PathBuf::from(p)
 }
 
-fn numbered_backup_path(path: &PathBuf) -> PathBuf {
+fn numbered_backup_path(path: &Path) -> PathBuf {
     let mut i: u64 = 1;
     loop {
         let new_path = simple_backup_path(path, &format!(".~{}~", i));
@@ -470,7 +470,7 @@ fn numbered_backup_path(path: &PathBuf) -> PathBuf {
     }
 }
 
-fn existing_backup_path(path: &PathBuf, suffix: &str) -> PathBuf {
+fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf {
     let test_path = simple_backup_path(path, &".~1~".to_owned());
     if test_path.exists() {
         return numbered_backup_path(path);

src/uu/ls/src/ls.rs

@@ -120,6 +120,11 @@ pub mod options {
         pub static FILE_TYPE: &str = "file-type";
         pub static CLASSIFY: &str = "classify";
     }
+    pub mod dereference {
+        pub static ALL: &str = "dereference";
+        pub static ARGS: &str = "dereference-command-line";
+        pub static DIR_ARGS: &str = "dereference-command-line-symlink-to-dir";
+    }
     pub static HIDE_CONTROL_CHARS: &str = "hide-control-chars";
     pub static SHOW_CONTROL_CHARS: &str = "show-control-chars";
     pub static WIDTH: &str = "width";
@@ -134,7 +139,6 @@ pub mod options {
     pub static FILE_TYPE: &str = "file-type";
     pub static SLASH: &str = "p";
     pub static INODE: &str = "inode";
-    pub static DEREFERENCE: &str = "dereference";
     pub static REVERSE: &str = "reverse";
     pub static RECURSIVE: &str = "recursive";
     pub static COLOR: &str = "color";
@@ -180,6 +184,13 @@ enum Time {
     Change,
 }
 
+enum Dereference {
+    None,
+    DirArgs,
+    Args,
+    All,
+}
+
 #[derive(PartialEq, Eq)]
 enum IndicatorStyle {
     None,
@@ -194,7 +205,7 @@ struct Config {
     sort: Sort,
     recursive: bool,
     reverse: bool,
-    dereference: bool,
+    dereference: Dereference,
     ignore_patterns: GlobSet,
     size_format: SizeFormat,
     directory: bool,
@@ -370,6 +381,7 @@ impl Config {
             })
             .or_else(|| termsize::get().map(|s| s.cols));
 
+        #[allow(clippy::needless_bool)]
         let show_control = if options.is_present(options::HIDE_CONTROL_CHARS) {
             false
         } else if options.is_present(options::SHOW_CONTROL_CHARS) {
@@ -482,13 +494,28 @@ impl Config {
 
         let ignore_patterns = ignore_patterns.build().unwrap();
 
+        let dereference = if options.is_present(options::dereference::ALL) {
+            Dereference::All
+        } else if options.is_present(options::dereference::ARGS) {
+            Dereference::Args
+        } else if options.is_present(options::dereference::DIR_ARGS) {
+            Dereference::DirArgs
+        } else if options.is_present(options::DIRECTORY)
+            || indicator_style == IndicatorStyle::Classify
+            || format == Format::Long
+        {
+            Dereference::None
+        } else {
+            Dereference::DirArgs
+        };
+
         Config {
             format,
             files,
             sort,
             recursive: options.is_present(options::RECURSIVE),
             reverse: options.is_present(options::REVERSE),
-            dereference: options.is_present(options::DEREFERENCE),
+            dereference,
             ignore_patterns,
             size_format,
             directory: options.is_present(options::DIRECTORY),
@@ -819,6 +846,48 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 ])
         )
 
+        // Dereferencing
+        .arg(
+            Arg::with_name(options::dereference::ALL)
+                .short("L")
+                .long(options::dereference::ALL)
+                .help(
+                    "When showing file information for a symbolic link, show information for the \
+                file the link references rather than the link itself.",
+                )
+                .overrides_with_all(&[
+                    options::dereference::ALL,
+                    options::dereference::DIR_ARGS,
+                    options::dereference::ARGS,
+                ])
+        )
+        .arg(
+            Arg::with_name(options::dereference::DIR_ARGS)
+                .long(options::dereference::DIR_ARGS)
+                .help(
+                    "Do not dereference symlinks except when they link to directories and are \
+                    given as command line arguments.",
+                )
+                .overrides_with_all(&[
+                    options::dereference::ALL,
+                    options::dereference::DIR_ARGS,
+                    options::dereference::ARGS,
+                ])
+        )
+        .arg(
+            Arg::with_name(options::dereference::ARGS)
+                .short("H")
+                .long(options::dereference::ARGS)
+                .help(
+                    "Do not dereference symlinks except when given as command line arguments.",
+                )
+                .overrides_with_all(&[
+                    options::dereference::ALL,
+                    options::dereference::DIR_ARGS,
+                    options::dereference::ARGS,
+                ])
+        )
+
         // Long format options
         .arg(
             Arg::with_name(options::NO_GROUP)
@@ -877,15 +946,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 .long(options::INODE)
                 .help("print the index number of each file"),
         )
-        .arg(
-            Arg::with_name(options::DEREFERENCE)
-                .short("L")
-                .long(options::DEREFERENCE)
-                .help(
-                    "When showing file information for a symbolic link, show information for the \
-                file the link references rather than the link itself.",
-                ),
-        )
         .arg(
             Arg::with_name(options::REVERSE)
                 .short("r")
@@ -993,26 +1053,32 @@ fn list(locs: Vec<String>, config: Config) -> i32 {
             has_failed = true;
             continue;
         }
-        let mut dir = false;
 
-        if p.is_dir() && !config.directory {
+        let show_dir_contents = if !config.directory {
-            dir = true;
+            match config.dereference {
-            if config.format == Format::Long && !config.dereference {
+                Dereference::None => {
-                if let Ok(md) = p.symlink_metadata() {
+                    if let Ok(md) = p.symlink_metadata() {
-                    if md.file_type().is_symlink() && !p.ends_with("/") {
+                        md.is_dir()
-                        dir = false;
+                    } else {
+                        show_error!("'{}': {}", &loc, "No such file or directory");
+                        has_failed = true;
+                        continue;
                     }
                 }
+                _ => p.is_dir(),
             }
-        }
+        } else {
-        if dir {
+            false
+        };
+
+        if show_dir_contents {
             dirs.push(p);
         } else {
             files.push(p);
         }
     }
     sort_entries(&mut files, &config);
-    display_items(&files, None, &config);
+    display_items(&files, None, &config, true);
 
     sort_entries(&mut dirs, &config);
     for dir in dirs {
@@ -1032,17 +1098,18 @@ fn sort_entries(entries: &mut Vec<PathBuf>, config: &Config) {
     match config.sort {
         Sort::Time => entries.sort_by_key(|k| {
             Reverse(
-                get_metadata(k, config)
+                get_metadata(k, false)
                     .ok()
                     .and_then(|md| get_system_time(&md, config))
                     .unwrap_or(UNIX_EPOCH),
             )
         }),
-        Sort::Size => entries
+        Sort::Size => {
-            .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.len()).unwrap_or(0))),
+            entries.sort_by_key(|k| Reverse(get_metadata(k, false).map(|md| md.len()).unwrap_or(0)))
+        }
         // The default sort in GNU ls is case insensitive
         Sort::Name => entries.sort_by_key(|k| k.to_string_lossy().to_lowercase()),
-        Sort::Version => entries.sort_by(version_cmp::version_cmp),
+        Sort::Version => entries.sort_by(|a, b| version_cmp::version_cmp(a, b)),
         Sort::None => {}
     }
 
@@ -1076,7 +1143,7 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool {
     true
 }
 
-fn enter_directory(dir: &PathBuf, config: &Config) {
+fn enter_directory(dir: &Path, config: &Config) {
     let mut entries: Vec<_> = safe_unwrap!(fs::read_dir(dir).and_then(Iterator::collect));
 
     entries.retain(|e| should_display(e, config));
@@ -1088,9 +1155,9 @@ fn enter_directory(dir: &PathBuf, config: &Config) {
         let mut display_entries = entries.clone();
         display_entries.insert(0, dir.join(".."));
         display_entries.insert(0, dir.join("."));
-        display_items(&display_entries, Some(dir), config);
+        display_items(&display_entries, Some(dir), config, false);
     } else {
-        display_items(&entries, Some(dir), config);
+        display_items(&entries, Some(dir), config, false);
     }
 
     if config.recursive {
@@ -1101,16 +1168,16 @@ fn enter_directory(dir: &PathBuf, config: &Config) {
     }
 }
 
-fn get_metadata(entry: &PathBuf, config: &Config) -> std::io::Result<Metadata> {
+fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result<Metadata> {
-    if config.dereference {
+    if dereference {
         entry.metadata().or_else(|_| entry.symlink_metadata())
     } else {
         entry.symlink_metadata()
     }
 }
 
-fn display_dir_entry_size(entry: &PathBuf, config: &Config) -> (usize, usize) {
+fn display_dir_entry_size(entry: &Path, config: &Config) -> (usize, usize) {
-    if let Ok(md) = get_metadata(entry, config) {
+    if let Ok(md) = get_metadata(entry, false) {
         (
             display_symlink_count(&md).len(),
             display_file_size(&md, config).len(),
@@ -1124,7 +1191,7 @@ fn pad_left(string: String, count: usize) -> String {
     format!("{:>width$}", string, width = count)
 }
 
-fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config) {
+fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config, command_line: bool) {
     if config.format == Format::Long {
         let (mut max_links, mut max_size) = (1, 1);
         for item in items {
@@ -1133,11 +1200,11 @@ fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config) {
             max_size = size.max(max_size);
         }
         for item in items {
-            display_item_long(item, strip, max_links, max_size, config);
+            display_item_long(item, strip, max_links, max_size, config, command_line);
         }
     } else {
         let names = items.iter().filter_map(|i| {
-            let md = get_metadata(i, config);
+            let md = get_metadata(i, false);
             match md {
                 Err(e) => {
                     let filename = get_file_name(i, strip);
@@ -1204,13 +1271,31 @@ fn display_grid(names: impl Iterator<Item = Cell>, width: u16, direction: Direct
 use uucore::fs::display_permissions;
 
 fn display_item_long(
-    item: &PathBuf,
+    item: &Path,
     strip: Option<&Path>,
     max_links: usize,
     max_size: usize,
     config: &Config,
+    command_line: bool,
 ) {
-    let md = match get_metadata(item, config) {
+    let dereference = match &config.dereference {
+        Dereference::All => true,
+        Dereference::Args => command_line,
+        Dereference::DirArgs => {
+            if command_line {
+                if let Ok(md) = item.metadata() {
+                    md.is_dir()
+                } else {
+                    false
+                }
+            } else {
+                false
+            }
+        }
+        Dereference::None => false,
+    };
+
+    let md = match get_metadata(item, dereference) {
         Err(e) => {
             let filename = get_file_name(&item, strip);
             show_error!("{}: {}", filename, e);

src/uu/ls/src/version_cmp.rs

@@ -1,8 +1,9 @@
-use std::{cmp::Ordering, path::PathBuf};
+use std::cmp::Ordering;
+use std::path::Path;
 
-/// Compare pathbufs in a way that matches the GNU version sort, meaning that
+/// Compare paths in a way that matches the GNU version sort, meaning that
 /// numbers get sorted in a natural way.
-pub(crate) fn version_cmp(a: &PathBuf, b: &PathBuf) -> Ordering {
+pub(crate) fn version_cmp(a: &Path, b: &Path) -> Ordering {
     let a_string = a.to_string_lossy();
     let b_string = b.to_string_lossy();
     let mut a = a_string.chars().peekable();

src/uu/mv/src/mv.rs

@@ -335,7 +335,7 @@ fn exec(files: &[PathBuf], b: Behavior) -> i32 {
     0
 }
 
-fn move_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) -> i32 {
+fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i32 {
     if !target_dir.is_dir() {
         show_error!("target ‘{}’ is not a directory", target_dir.display());
         return 1;
@@ -373,7 +373,7 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) ->
     }
 }
 
-fn rename(from: &PathBuf, to: &PathBuf, b: &Behavior) -> io::Result<()> {
+fn rename(from: &Path, to: &Path, b: &Behavior) -> io::Result<()> {
     let mut backup_path = None;
 
     if to.exists() {
@@ -429,7 +429,7 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behavior) -> io::Result<()> {
 
 /// A wrapper around `fs::rename`, so that if it fails, we try falling back on
 /// copying and removing.
-fn rename_with_fallback(from: &PathBuf, to: &PathBuf) -> io::Result<()> {
+fn rename_with_fallback(from: &Path, to: &Path) -> io::Result<()> {
     if fs::rename(from, to).is_err() {
         // Get metadata without following symlinks
         let metadata = from.symlink_metadata()?;
@@ -464,7 +464,7 @@ fn rename_with_fallback(from: &PathBuf, to: &PathBuf) -> io::Result<()> {
 /// Move the given symlink to the given destination. On Windows, dangling
 /// symlinks return an error.
 #[inline]
-fn rename_symlink_fallback(from: &PathBuf, to: &PathBuf) -> io::Result<()> {
+fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
     let path_symlink_points_to = fs::read_link(from)?;
     #[cfg(unix)]
     {
@@ -507,20 +507,20 @@ fn read_yes() -> bool {
     }
 }
 
-fn simple_backup_path(path: &PathBuf, suffix: &str) -> PathBuf {
+fn simple_backup_path(path: &Path, suffix: &str) -> PathBuf {
     let mut p = path.to_string_lossy().into_owned();
     p.push_str(suffix);
     PathBuf::from(p)
 }
 
-fn numbered_backup_path(path: &PathBuf) -> PathBuf {
+fn numbered_backup_path(path: &Path) -> PathBuf {
     (1_u64..)
         .map(|i| path.with_extension(format!("~{}~", i)))
         .find(|p| !p.exists())
         .expect("cannot create backup")
 }
 
-fn existing_backup_path(path: &PathBuf, suffix: &str) -> PathBuf {
+fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf {
     let test_path = path.with_extension("~1~");
     if test_path.exists() {
         numbered_backup_path(path)
@@ -529,7 +529,7 @@ fn existing_backup_path(path: &PathBuf, suffix: &str) -> PathBuf {
     }
 }
 
-fn is_empty_dir(path: &PathBuf) -> bool {
+fn is_empty_dir(path: &Path) -> bool {
     match fs::read_dir(path) {
         Ok(contents) => contents.peekable().peek().is_none(),
         Err(_e) => false,

src/uu/od/src/od.rs

@@ -118,7 +118,7 @@ struct OdOptions {
 }
 
 impl OdOptions {
-    fn new<'a>(matches: ArgMatches<'a>, args: Vec<String>) -> Result<OdOptions, String> {
+    fn new(matches: ArgMatches, args: Vec<String>) -> Result<OdOptions, String> {
         let byte_order = match matches.value_of(options::ENDIAN) {
             None => ByteOrder::Native,
             Some("little") => ByteOrder::Little,

src/uu/od/src/parse_inputs.rs

@@ -63,7 +63,7 @@ pub fn parse_inputs(matches: &dyn CommandLineOpts) -> Result<CommandLineInputs,
                 }
                 if input_strings.len() == 2 {
                     return Ok(CommandLineInputs::FileAndOffset((
-                        input_strings[0].clone().to_owned(),
+                        input_strings[0].to_string(),
                         n,
                         None,
                     )));
@@ -106,7 +106,7 @@ pub fn parse_inputs_traditional(input_strings: Vec<&str>) -> Result<CommandLineI
                     Some(m),
                 ))),
                 (_, Ok(m)) => Ok(CommandLineInputs::FileAndOffset((
-                    input_strings[0].clone().to_owned(),
+                    input_strings[0].to_string(),
                     m,
                     None,
                 ))),
@@ -118,7 +118,7 @@ pub fn parse_inputs_traditional(input_strings: Vec<&str>) -> Result<CommandLineI
             let label = parse_offset_operand(&input_strings[2]);
             match (offset, label) {
                 (Ok(n), Ok(m)) => Ok(CommandLineInputs::FileAndOffset((
-                    input_strings[0].clone().to_owned(),
+                    input_strings[0].to_string(),
                     n,
                     Some(m),
                 ))),

src/uu/pinky/src/pinky.rs

@@ -15,7 +15,6 @@ use uucore::utmpx::{self, time, Utmpx};
 
 use std::io::prelude::*;
 use std::io::BufReader;
-use std::io::Result as IOResult;
 
 use std::fs::File;
 use std::os::unix::fs::MetadataExt;
@@ -136,12 +135,8 @@ The utmp file will be {}",
     };
 
     if do_short_format {
-        if let Err(e) = pk.short_pinky() {
+        pk.short_pinky();
-            show_usage_error!("{}", e);
+        0
-            1
-        } else {
-            0
-        }
     } else {
         pk.long_pinky()
     }
@@ -282,7 +277,7 @@ impl Pinky {
         println!();
     }
 
-    fn short_pinky(&self) -> IOResult<()> {
+    fn short_pinky(&self) {
         if self.include_heading {
             self.print_heading();
         }
@@ -295,7 +290,6 @@ impl Pinky {
                 }
             }
         }
-        Ok(())
     }
 
     fn long_pinky(&self) -> i32 {

src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs

@@ -199,8 +199,7 @@ pub fn arrnum_int_add(arrnum: &[u8], basenum: u8, base_ten_int_term: u8) -> Vec<
 }
 
 pub fn base_conv_vec(src: &[u8], radix_src: u8, radix_dest: u8) -> Vec<u8> {
-    let mut result: Vec<u8> = Vec::new();
+    let mut result = vec![0];
-    result.push(0);
     for i in src {
         result = arrnum_int_mult(&result, radix_dest, radix_src);
         result = arrnum_int_add(&result, radix_dest, *i);
@@ -226,8 +225,7 @@ pub fn base_conv_float(src: &[u8], radix_src: u8, radix_dest: u8) -> f64 {
     // to implement this for arbitrary string input.
     // until then, the below operates as an outline
     // of how it would work.
-    let mut result: Vec<u8> = Vec::new();
+    let result: Vec<u8> = vec![0];
-    result.push(0);
     let mut factor: f64 = 1_f64;
     let radix_src_float: f64 = f64::from(radix_src);
     let mut r: f64 = 0_f64;

src/uu/printf/src/tokenize/num_format/num_format.rs

@@ -263,9 +263,5 @@ pub fn num_format(field: &FormatField, in_str_opt: Option<&String>) -> Option<St
         };
     // if we have a formatPrimitive, print its results
     // according to the field-char appropriate Formatter
-    if let Some(prim) = prim_opt {
+    prim_opt.map(|prim| fmtr.primitive_to_str(&prim, field.clone()))
-        Some(fmtr.primitive_to_str(&prim, field.clone()))
-    } else {
-        None
-    }
 }

src/uu/ptx/src/ptx.rs

@@ -177,14 +177,14 @@ fn get_config(matches: &clap::ArgMatches) -> Config {
     }
     if matches.is_present(options::WIDTH) {
         let width_str = matches.value_of(options::WIDTH).expect(err_msg).to_string();
-        config.line_width = crash_if_err!(1, usize::from_str_radix(&width_str, 10));
+        config.line_width = crash_if_err!(1, (&width_str).parse::<usize>());
     }
     if matches.is_present(options::GAP_SIZE) {
         let gap_str = matches
             .value_of(options::GAP_SIZE)
             .expect(err_msg)
             .to_string();
-        config.gap_size = crash_if_err!(1, usize::from_str_radix(&gap_str, 10));
+        config.gap_size = crash_if_err!(1, (&gap_str).parse::<usize>());
     }
     if matches.is_present(options::FORMAT_ROFF) {
         config.format = OutFormat::Roff;

src/uu/readlink/src/readlink.rs

@@ -13,7 +13,7 @@ extern crate uucore;
 use clap::{App, Arg};
 use std::fs;
 use std::io::{stdout, Write};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use uucore::fs::{canonicalize, CanonicalizeMode};
 
 const NAME: &str = "readlink";
@@ -160,8 +160,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     0
 }
 
-fn show(path: &PathBuf, no_newline: bool, use_zero: bool) {
+fn show(path: &Path, no_newline: bool, use_zero: bool) {
-    let path = path.as_path().to_str().unwrap();
+    let path = path.to_str().unwrap();
     if use_zero {
         print!("{}\0", path);
     } else if no_newline {

src/uu/realpath/src/realpath.rs

@@ -12,7 +12,7 @@ extern crate uucore;
 
 use clap::{App, Arg};
 use std::fs;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use uucore::fs::{canonicalize, CanonicalizeMode};
 
 static ABOUT: &str = "print the resolved path";
@@ -82,7 +82,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     retcode
 }
 
-fn resolve_path(p: &PathBuf, strip: bool, zero: bool, quiet: bool) -> bool {
+fn resolve_path(p: &Path, strip: bool, zero: bool, quiet: bool) -> bool {
     let abs = canonicalize(p, CanonicalizeMode::Normal).unwrap();
 
     if strip {

src/uu/rm/src/rm.rs

@@ -176,7 +176,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 } else if matches.is_present(OPT_PROMPT_MORE) {
                     InteractiveMode::Once
                 } else if matches.is_present(OPT_INTERACTIVE) {
-                    match &matches.value_of(OPT_INTERACTIVE).unwrap()[..] {
+                    match matches.value_of(OPT_INTERACTIVE).unwrap() {
                         "none" => InteractiveMode::None,
                         "once" => InteractiveMode::Once,
                         "always" => InteractiveMode::Always,

src/uu/seq/src/seq.rs

@@ -102,7 +102,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let mut largest_dec = 0;
     let mut padding = 0;
     let first = if numbers.len() > 1 {
-        let slice = &numbers[0][..];
+        let slice = numbers[0];
         let len = slice.len();
         let dec = slice.find('.').unwrap_or(len);
         largest_dec = len - dec;
@@ -118,7 +118,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         1.0
     };
     let increment = if numbers.len() > 2 {
-        let slice = &numbers[1][..];
+        let slice = numbers[1];
         let len = slice.len();
         let dec = slice.find('.').unwrap_or(len);
         largest_dec = cmp::max(largest_dec, len - dec);
@@ -134,11 +134,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         1.0
     };
     if increment == 0.0 {
-        show_error!("increment value: '{}'", &numbers[1][..]);
+        show_error!("increment value: '{}'", numbers[1]);
         return 1;
     }
     let last = {
-        let slice = &numbers[numbers.len() - 1][..];
+        let slice = numbers[numbers.len() - 1];
         padding = cmp::max(padding, slice.find('.').unwrap_or_else(|| slice.len()));
         match parse_float(slice) {
             Ok(n) => n,

src/uu/shred/src/shred.rs

@@ -363,10 +363,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     let force = matches.is_present(options::FORCE);
     let remove = matches.is_present(options::REMOVE);
-    let size_arg = match matches.value_of(options::SIZE) {
+    let size_arg = matches.value_of(options::SIZE).map(|s| s.to_string());
-        Some(s) => Some(s.to_string()),
-        None => None,
-    };
     let size = get_size(size_arg);
     let exact = matches.is_present(options::EXACT) && size.is_none(); // if -s is given, ignore -x
     let zero = matches.is_present(options::ZERO);
@@ -439,6 +436,7 @@ fn pass_name(pass_type: PassType) -> String {
     }
 }
 
+#[allow(clippy::too_many_arguments)]
 fn wipe_file(
     path_str: &str,
     n_passes: usize,
@@ -472,12 +470,9 @@ fn wipe_file(
 
         let mut perms = metadata.permissions();
         perms.set_readonly(false);
-        match fs::set_permissions(path, perms) {
+        if let Err(e) = fs::set_permissions(path, perms) {
-            Err(e) => {
+            show_error!("{}", e);
-                show_error!("{}", e);
+            return;
-                return;
-            }
-            _ => {}
         }
     }
 

src/uu/sort/BENCHMARKING.md

@@ -9,25 +9,125 @@ list that we should improve / make sure not to regress.
 Run `cargo build --release` before benchmarking after you make a change!
 
 ## Sorting a wordlist
-- Get a wordlist, for example with [words](https://en.wikipedia.org/wiki/Words_(Unix)) on Linux. The exact wordlist
+
-  doesn't matter for performance comparisons. In this example I'm using `/usr/share/dict/american-english` as the wordlist.
+-   Get a wordlist, for example with [words](<https://en.wikipedia.org/wiki/Words_(Unix)>) on Linux. The exact wordlist
-- Shuffle the wordlist by running `sort -R /usr/share/dict/american-english > shuffled_wordlist.txt`.
+    doesn't matter for performance comparisons. In this example I'm using `/usr/share/dict/american-english` as the wordlist.
-- Benchmark sorting the wordlist with hyperfine: `hyperfine "target/release/coreutils sort shuffled_wordlist.txt -o output.txt"`.
+-   Shuffle the wordlist by running `sort -R /usr/share/dict/american-english > shuffled_wordlist.txt`.
+-   Benchmark sorting the wordlist with hyperfine: `hyperfine "target/release/coreutils sort shuffled_wordlist.txt -o output.txt"`.
 
 ## Sorting a wordlist with ignore_case
-- Same wordlist as above
+
-- Benchmark sorting the wordlist ignoring the case with hyperfine: `hyperfine "target/release/coreutils sort shuffled_wordlist.txt -f -o output.txt"`.
+-   Same wordlist as above
+-   Benchmark sorting the wordlist ignoring the case with hyperfine: `hyperfine "target/release/coreutils sort shuffled_wordlist.txt -f -o output.txt"`.
 
 ## Sorting numbers
-- Generate a list of numbers: `seq 0 100000 | sort -R > shuffled_numbers.txt`.
+
-- Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers.txt -n -o output.txt"`.
+-   Generate a list of numbers: `seq 0 100000 | sort -R > shuffled_numbers.txt`.
+-   Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers.txt -n -o output.txt"`.
+
+## Sorting numbers with -g
+
+-   Same list of numbers as above.
+-   Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers.txt -g -o output.txt"`.
+
+## Sorting numbers with SI prefixes
+
+-   Generate a list of numbers:
+    <details>
+      <summary>Rust script</summary>
+
+    ## Cargo.toml
+
+    ```toml
+    [dependencies]
+    rand = "0.8.3"
+    ```
+
+    ## main.rs
+
+    ```rust
+    use rand::prelude::*;
+    fn main() {
+      let suffixes = ['k', 'K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y'];
+      let mut rng = thread_rng();
+      for _ in 0..100000 {
+          println!(
+              "{}{}",
+              rng.gen_range(0..1000000),
+              suffixes.choose(&mut rng).unwrap()
+          )
+      }
+    }
+
+    ```
+
+    ## running
+
+    `cargo run > shuffled_numbers_si.txt`
+
+    </details>
+
+-   Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt"`.
 
 ## Stdout and stdin performance
+
 Try to run the above benchmarks by piping the input through stdin (standard input) and redirect the
 output through stdout (standard output):
-- Remove the input file from the arguments and add `cat [inputfile] | ` at the beginning.
+
-- Remove `-o output.txt` and add `> output.txt` at the end.
+-   Remove the input file from the arguments and add `cat [inputfile] | ` at the beginning.
+-   Remove `-o output.txt` and add `> output.txt` at the end.
 
 Example: `hyperfine "target/release/coreutils sort shuffled_numbers.txt -n -o output.txt"` becomes
 `hyperfine "cat shuffled_numbers.txt | target/release/coreutils sort -n > output.txt`
-- Check that performance is similar to the original benchmark.
+
+-   Check that performance is similar to the original benchmark.
+
+## Comparing with GNU sort
+
+Hyperfine accepts multiple commands to run and will compare them. To compare performance with GNU sort
+duplicate the string you passed to hyperfine but remove the `target/release/coreutils` bit from it.
+
+Example: `hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt"` becomes
+`hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt" "sort shuffled_numbers_si.txt -h -o output.txt"`
+(This assumes GNU sort is installed as `sort`)
+
+## Memory and CPU usage
+
+The above benchmarks use hyperfine to measure the speed of sorting. There are however other useful metrics to determine overall
+resource usage. One way to measure them is the `time` command. This is not to be confused with the `time` that is built in to the bash shell.
+You may have to install `time` first, then you have to run it with `/bin/time -v` to give it precedence over the built in `time`.
+
+<details>
+      <summary>Example output</summary>
+
+      Command being timed: "target/release/coreutils sort shuffled_numbers.txt"
+      User time (seconds): 0.10
+      System time (seconds): 0.00
+      Percent of CPU this job got: 365%
+      Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.02
+      Average shared text size (kbytes): 0
+      Average unshared data size (kbytes): 0
+      Average stack size (kbytes): 0
+      Average total size (kbytes): 0
+      Maximum resident set size (kbytes): 25360
+      Average resident set size (kbytes): 0
+      Major (requiring I/O) page faults: 0
+      Minor (reclaiming a frame) page faults: 5802
+      Voluntary context switches: 462
+      Involuntary context switches: 73
+      Swaps: 0
+      File system inputs: 1184
+      File system outputs: 0
+      Socket messages sent: 0
+      Socket messages received: 0
+      Signals delivered: 0
+      Page size (bytes): 4096
+      Exit status: 0
+
+</details>
+
+Useful metrics to look at could be:
+
+-   User time
+-   Percent of CPU this job got
+-   Maximum resident set size

src/uu/sort/src/numeric_str_cmp.rs

@@ -0,0 +1,455 @@
+//  * This file is part of the uutils coreutils package.
+//  *
+//  * (c) Michael Debertol <michael.debertol..AT..gmail.com>
+//  *
+//  * For the full copyright and license information, please view the LICENSE
+//  * file that was distributed with this source code.
+
+//! Fast comparison for strings representing a base 10 number without precision loss.
+//!
+//! To be able to short-circuit when comparing, [NumInfo] must be passed along with each number
+//! to [numeric_str_cmp]. [NumInfo] is generally obtained by calling [NumInfo::parse] and should be cached.
+//! It is allowed to arbitrarily modify the exponent afterwards, which is equivalent to shifting the decimal point.
+//!
+//! More specifically, exponent can be understood so that the original number is in (1..10)*10^exponent.
+//! From that follows the constraints of this algorithm: It is able to compare numbers in ±(1*10^[i64::MIN]..10*10^[i64::MAX]).
+
+use std::{cmp::Ordering, ops::Range};
+
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
+enum Sign {
+    Negative,
+    Positive,
+}
+
+#[derive(Debug, PartialEq)]
+pub struct NumInfo {
+    exponent: i64,
+    sign: Sign,
+}
+
+pub struct NumInfoParseSettings {
+    pub accept_si_units: bool,
+    pub thousands_separator: Option<char>,
+    pub decimal_pt: Option<char>,
+}
+
+impl Default for NumInfoParseSettings {
+    fn default() -> Self {
+        Self {
+            accept_si_units: false,
+            thousands_separator: None,
+            decimal_pt: Some('.'),
+        }
+    }
+}
+
+impl NumInfo {
+    /// Parse NumInfo for this number.
+    /// Also returns the range of num that should be passed to numeric_str_cmp later
+    pub fn parse(num: &str, parse_settings: NumInfoParseSettings) -> (Self, Range<usize>) {
+        let mut exponent = -1;
+        let mut had_decimal_pt = false;
+        let mut had_digit = false;
+        let mut start = None;
+        let mut sign = Sign::Positive;
+
+        let mut first_char = true;
+
+        for (idx, char) in num.char_indices() {
+            if first_char && char.is_whitespace() {
+                continue;
+            }
+
+            if first_char && char == '-' {
+                sign = Sign::Negative;
+                first_char = false;
+                continue;
+            }
+            first_char = false;
+
+            if parse_settings
+                .thousands_separator
+                .map_or(false, |c| c == char)
+            {
+                continue;
+            }
+
+            if Self::is_invalid_char(char, &mut had_decimal_pt, &parse_settings) {
+                let si_unit = if parse_settings.accept_si_units {
+                    match char {
+                        'K' | 'k' => 3,
+                        'M' => 6,
+                        'G' => 9,
+                        'T' => 12,
+                        'P' => 15,
+                        'E' => 18,
+                        'Z' => 21,
+                        'Y' => 24,
+                        _ => 0,
+                    }
+                } else {
+                    0
+                };
+                return if let Some(start) = start {
+                    (
+                        NumInfo {
+                            exponent: exponent + si_unit,
+                            sign,
+                        },
+                        start..idx,
+                    )
+                } else {
+                    (
+                        NumInfo {
+                            sign: if had_digit { sign } else { Sign::Positive },
+                            exponent: 0,
+                        },
+                        0..0,
+                    )
+                };
+            }
+            if Some(char) == parse_settings.decimal_pt {
+                continue;
+            }
+            had_digit = true;
+            if start.is_none() && char == '0' {
+                if had_decimal_pt {
+                    // We're parsing a number whose first nonzero digit is after the decimal point.
+                    exponent -= 1;
+                } else {
+                    // Skip leading zeroes
+                    continue;
+                }
+            }
+            if !had_decimal_pt {
+                exponent += 1;
+            }
+            if start.is_none() && char != '0' {
+                start = Some(idx);
+            }
+        }
+        if let Some(start) = start {
+            (NumInfo { exponent, sign }, start..num.len())
+        } else {
+            (
+                NumInfo {
+                    sign: if had_digit { sign } else { Sign::Positive },
+                    exponent: 0,
+                },
+                0..0,
+            )
+        }
+    }
+
+    fn is_invalid_char(
+        c: char,
+        had_decimal_pt: &mut bool,
+        parse_settings: &NumInfoParseSettings,
+    ) -> bool {
+        if Some(c) == parse_settings.decimal_pt {
+            if *had_decimal_pt {
+                // this is a decimal pt but we already had one, so it is invalid
+                true
+            } else {
+                *had_decimal_pt = true;
+                false
+            }
+        } else {
+            !c.is_ascii_digit()
+        }
+    }
+}
+
+/// compare two numbers as strings without parsing them as a number first. This should be more performant and can handle numbers more precisely.
+/// NumInfo is needed to provide a fast path for most numbers.
+pub fn numeric_str_cmp((a, a_info): (&str, &NumInfo), (b, b_info): (&str, &NumInfo)) -> Ordering {
+    // check for a difference in the sign
+    if a_info.sign != b_info.sign {
+        return a_info.sign.cmp(&b_info.sign);
+    }
+
+    // check for a difference in the exponent
+    let ordering = if a_info.exponent != b_info.exponent && !a.is_empty() && !b.is_empty() {
+        a_info.exponent.cmp(&b_info.exponent)
+    } else {
+        // walk the characters from the front until we find a difference
+        let mut a_chars = a.chars().filter(|c| c.is_ascii_digit());
+        let mut b_chars = b.chars().filter(|c| c.is_ascii_digit());
+        loop {
+            let a_next = a_chars.next();
+            let b_next = b_chars.next();
+            match (a_next, b_next) {
+                (None, None) => break Ordering::Equal,
+                (Some(c), None) => {
+                    break if c == '0' && a_chars.all(|c| c == '0') {
+                        Ordering::Equal
+                    } else {
+                        Ordering::Greater
+                    }
+                }
+                (None, Some(c)) => {
+                    break if c == '0' && b_chars.all(|c| c == '0') {
+                        Ordering::Equal
+                    } else {
+                        Ordering::Less
+                    }
+                }
+                (Some(a_char), Some(b_char)) => {
+                    let ord = a_char.cmp(&b_char);
+                    if ord != Ordering::Equal {
+                        break ord;
+                    }
+                }
+            }
+        }
+    };
+
+    if a_info.sign == Sign::Negative {
+        ordering.reverse()
+    } else {
+        ordering
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn parses_exp() {
+        let n = "1";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..1
+            )
+        );
+        let n = "100";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 2,
+                    sign: Sign::Positive
+                },
+                0..3
+            )
+        );
+        let n = "1,000";
+        assert_eq!(
+            NumInfo::parse(
+                n,
+                NumInfoParseSettings {
+                    thousands_separator: Some(','),
+                    ..Default::default()
+                }
+            ),
+            (
+                NumInfo {
+                    exponent: 3,
+                    sign: Sign::Positive
+                },
+                0..5
+            )
+        );
+        let n = "1,000";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..1
+            )
+        );
+        let n = "1000.00";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 3,
+                    sign: Sign::Positive
+                },
+                0..7
+            )
+        );
+    }
+    #[test]
+    fn parses_negative_exp() {
+        let n = "0.00005";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: -5,
+                    sign: Sign::Positive
+                },
+                6..7
+            )
+        );
+        let n = "00000.00005";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: -5,
+                    sign: Sign::Positive
+                },
+                10..11
+            )
+        );
+    }
+
+    #[test]
+    fn parses_sign() {
+        let n = "5";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..1
+            )
+        );
+        let n = "-5";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Negative
+                },
+                1..2
+            )
+        );
+        let n = "    -5";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Negative
+                },
+                5..6
+            )
+        );
+    }
+
+    fn test_helper(a: &str, b: &str, expected: Ordering) {
+        let (a_info, a_range) = NumInfo::parse(a, Default::default());
+        let (b_info, b_range) = NumInfo::parse(b, Default::default());
+        let ordering = numeric_str_cmp(
+            (&a[a_range.to_owned()], &a_info),
+            (&b[b_range.to_owned()], &b_info),
+        );
+        assert_eq!(ordering, expected);
+        let ordering = numeric_str_cmp((&b[b_range], &b_info), (&a[a_range], &a_info));
+        assert_eq!(ordering, expected.reverse());
+    }
+    #[test]
+    fn test_single_digit() {
+        test_helper("1", "2", Ordering::Less);
+        test_helper("0", "0", Ordering::Equal);
+    }
+    #[test]
+    fn test_minus() {
+        test_helper("-1", "-2", Ordering::Greater);
+        test_helper("-0", "-0", Ordering::Equal);
+    }
+    #[test]
+    fn test_different_len() {
+        test_helper("-20", "-100", Ordering::Greater);
+        test_helper("10.0", "2.000000", Ordering::Greater);
+    }
+    #[test]
+    fn test_decimal_digits() {
+        test_helper("20.1", "20.2", Ordering::Less);
+        test_helper("20.1", "20.15", Ordering::Less);
+        test_helper("-20.1", "+20.15", Ordering::Less);
+        test_helper("-20.1", "-20", Ordering::Less);
+    }
+    #[test]
+    fn test_trailing_zeroes() {
+        test_helper("20.00000", "20.1", Ordering::Less);
+        test_helper("20.00000", "20.0", Ordering::Equal);
+    }
+    #[test]
+    fn test_invalid_digits() {
+        test_helper("foo", "bar", Ordering::Equal);
+        test_helper("20.1", "a", Ordering::Greater);
+        test_helper("-20.1", "a", Ordering::Less);
+        test_helper("a", "0.15", Ordering::Less);
+    }
+    #[test]
+    fn test_multiple_decimal_pts() {
+        test_helper("10.0.0", "50.0.0", Ordering::Less);
+        test_helper("0.1.", "0.2.0", Ordering::Less);
+        test_helper("1.1.", "0", Ordering::Greater);
+        test_helper("1.1.", "-0", Ordering::Greater);
+    }
+    #[test]
+    fn test_leading_decimal_pts() {
+        test_helper(".0", ".0", Ordering::Equal);
+        test_helper(".1", ".0", Ordering::Greater);
+        test_helper(".02", "0", Ordering::Greater);
+    }
+    #[test]
+    fn test_leading_zeroes() {
+        test_helper("000000.0", ".0", Ordering::Equal);
+        test_helper("0.1", "0000000000000.0", Ordering::Greater);
+        test_helper("-01", "-2", Ordering::Greater);
+    }
+
+    #[test]
+    fn minus_zero() {
+        // This matches GNU sort behavior.
+        test_helper("-0", "0", Ordering::Less);
+        test_helper("-0x", "0", Ordering::Less);
+    }
+    #[test]
+    fn double_minus() {
+        test_helper("--1", "0", Ordering::Equal);
+    }
+    #[test]
+    fn single_minus() {
+        let info = NumInfo::parse("-", Default::default());
+        assert_eq!(
+            info,
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..0
+            )
+        );
+    }
+    #[test]
+    fn invalid_with_unit() {
+        let info = NumInfo::parse(
+            "-K",
+            NumInfoParseSettings {
+                accept_si_units: true,
+                ..Default::default()
+            },
+        );
+        assert_eq!(
+            info,
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..0
+            )
+        );
+    }
+}

src/uu/sort/src/sort.rs

@@ -15,9 +15,12 @@
 #[macro_use]
 extern crate uucore;
 
+mod numeric_str_cmp;
+
 use clap::{App, Arg};
 use fnv::FnvHasher;
 use itertools::Itertools;
+use numeric_str_cmp::{numeric_str_cmp, NumInfo, NumInfoParseSettings};
 use rand::distributions::Alphanumeric;
 use rand::{thread_rng, Rng};
 use rayon::prelude::*;
@@ -162,27 +165,71 @@ impl From<&GlobalSettings> for KeySettings {
 }
 
 /// Represents the string selected by a FieldSelector.
-#[derive(Debug)]
+enum SelectionRange {
-enum Selection {
     /// If we had to transform this selection, we have to store a new string.
     String(String),
     /// If there was no transformation, we can store an index into the line.
     ByIndex(Range<usize>),
 }
 
+impl SelectionRange {
+    /// Gets the actual string slice represented by this Selection.
+    fn get_str<'a>(&'a self, line: &'a str) -> &'a str {
+        match self {
+            SelectionRange::String(string) => string.as_str(),
+            SelectionRange::ByIndex(range) => &line[range.to_owned()],
+        }
+    }
+
+    fn shorten(&mut self, new_range: Range<usize>) {
+        match self {
+            SelectionRange::String(string) => {
+                string.drain(new_range.end..);
+                string.drain(..new_range.start);
+            }
+            SelectionRange::ByIndex(range) => {
+                range.end = range.start + new_range.end;
+                range.start += new_range.start;
+            }
+        }
+    }
+}
+
+enum NumCache {
+    AsF64(f64),
+    WithInfo(NumInfo),
+    None,
+}
+
+impl NumCache {
+    fn as_f64(&self) -> f64 {
+        match self {
+            NumCache::AsF64(n) => *n,
+            _ => unreachable!(),
+        }
+    }
+    fn as_num_info(&self) -> &NumInfo {
+        match self {
+            NumCache::WithInfo(n) => n,
+            _ => unreachable!(),
+        }
+    }
+}
+
+struct Selection {
+    range: SelectionRange,
+    num_cache: NumCache,
+}
+
 impl Selection {
     /// Gets the actual string slice represented by this Selection.
     fn get_str<'a>(&'a self, line: &'a Line) -> &'a str {
-        match self {
+        self.range.get_str(&line.line)
-            Selection::String(string) => string.as_str(),
-            Selection::ByIndex(range) => &line.line[range.to_owned()],
-        }
     }
 }
 
 type Field = Range<usize>;
 
-#[derive(Debug)]
 struct Line {
     line: String,
     // The common case is not to specify fields. Let's make this fast.
@@ -206,18 +253,38 @@ impl Line {
             .selectors
             .iter()
             .map(|selector| {
-                if let Some(range) = selector.get_selection(&line, fields.as_deref()) {
+                let mut range =
-                    if let Some(transformed) =
+                    if let Some(range) = selector.get_selection(&line, fields.as_deref()) {
-                        transform(&line[range.to_owned()], &selector.settings)
+                        if let Some(transformed) =
-                    {
+                            transform(&line[range.to_owned()], &selector.settings)
-                        Selection::String(transformed)
+                        {
+                            SelectionRange::String(transformed)
+                        } else {
+                            SelectionRange::ByIndex(range.start().to_owned()..range.end() + 1)
+                        }
                     } else {
-                        Selection::ByIndex(range.start().to_owned()..range.end() + 1)
+                        // If there is no match, match the empty string.
-                    }
+                        SelectionRange::ByIndex(0..0)
+                    };
+                let num_cache = if selector.settings.mode == SortMode::Numeric
+                    || selector.settings.mode == SortMode::HumanNumeric
+                {
+                    let (info, num_range) = NumInfo::parse(
+                        range.get_str(&line),
+                        NumInfoParseSettings {
+                            accept_si_units: selector.settings.mode == SortMode::HumanNumeric,
+                            thousands_separator: Some(THOUSANDS_SEP),
+                            decimal_pt: Some(DECIMAL_PT),
+                        },
+                    );
+                    range.shorten(num_range);
+                    NumCache::WithInfo(info)
+                } else if selector.settings.mode == SortMode::GeneralNumeric {
+                    NumCache::AsF64(permissive_f64_parse(get_leading_gen(range.get_str(&line))))
                 } else {
-                    // If there is no match, match the empty string.
+                    NumCache::None
-                    Selection::ByIndex(0..0)
+                };
-                }
+                Selection { range, num_cache }
             })
             .collect();
         Self { line, selections }
@@ -677,7 +744,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         .arg(
             Arg::with_name(OPT_PARALLEL)
                 .long(OPT_PARALLEL)
-                .help("change the number of threads running concurrently to N")
+                .help("change the number of threads running concurrently to NUM_THREADS")
                 .takes_value(true)
                 .value_name("NUM_THREADS"),
         )
@@ -918,26 +985,37 @@ fn exec_check_file(unwrapped_lines: &[Line], settings: &GlobalSettings) -> i32 {
 }
 
 fn sort_by(lines: &mut Vec<Line>, settings: &GlobalSettings) {
-    lines.par_sort_by(|a, b| compare_by(a, b, &settings))
+    if settings.stable || settings.unique {
+        lines.par_sort_by(|a, b| compare_by(a, b, &settings))
+    } else {
+        lines.par_sort_unstable_by(|a, b| compare_by(a, b, &settings))
+    }
 }
 
 fn compare_by(a: &Line, b: &Line, global_settings: &GlobalSettings) -> Ordering {
     for (idx, selector) in global_settings.selectors.iter().enumerate() {
-        let a = a.selections[idx].get_str(a);
+        let a_selection = &a.selections[idx];
-        let b = b.selections[idx].get_str(b);
+        let b_selection = &b.selections[idx];
+        let a_str = a_selection.get_str(a);
+        let b_str = b_selection.get_str(b);
         let settings = &selector.settings;
 
         let cmp: Ordering = if settings.random {
-            random_shuffle(a, b, global_settings.salt.clone())
+            random_shuffle(a_str, b_str, global_settings.salt.clone())
         } else {
-            (match settings.mode {
+            match settings.mode {
-                SortMode::Numeric => numeric_compare,
+                SortMode::Numeric | SortMode::HumanNumeric => numeric_str_cmp(
-                SortMode::GeneralNumeric => general_numeric_compare,
+                    (a_str, a_selection.num_cache.as_num_info()),
-                SortMode::HumanNumeric => human_numeric_size_compare,
+                    (b_str, b_selection.num_cache.as_num_info()),
-                SortMode::Month => month_compare,
+                ),
-                SortMode::Version => version_compare,
+                SortMode::GeneralNumeric => general_numeric_compare(
-                SortMode::Default => default_compare,
+                    a_selection.num_cache.as_f64(),
-            })(a, b)
+                    b_selection.num_cache.as_f64(),
+                ),
+                SortMode::Month => month_compare(a_str, b_str),
+                SortMode::Version => version_compare(a_str, b_str),
+                SortMode::Default => default_compare(a_str, b_str),
+            }
         };
         if cmp != Ordering::Equal {
             return if settings.reverse { cmp.reverse() } else { cmp };
@@ -945,7 +1023,6 @@ fn compare_by(a: &Line, b: &Line, global_settings: &GlobalSettings) -> Ordering
     }
 
     // Call "last resort compare" if all selectors returned Equal
-
     let cmp = if global_settings.random || global_settings.stable || global_settings.unique {
         Ordering::Equal
     } else {
@@ -997,34 +1074,6 @@ fn leading_num_common(a: &str) -> &str {
     s
 }
 
-// This function cleans up the initial comparison done by leading_num_common for a numeric compare.
-// GNU sort does its numeric comparison through strnumcmp.  However, we don't have or
-// may not want to use libc.  Instead we emulate the GNU sort numeric compare by ignoring
-// those leading number lines GNU sort would not recognize.  GNU numeric compare would
-// not recognize a positive sign or scientific/E notation so we strip those elements here.
-fn get_leading_num(a: &str) -> &str {
-    let mut s = "";
-
-    let a = leading_num_common(a);
-
-    // GNU numeric sort doesn't recognize '+' or 'e' notation so we strip
-    for (idx, c) in a.char_indices() {
-        if c.eq(&'e') || c.eq(&'E') || a.chars().next().unwrap_or('\0').eq(&POSITIVE) {
-            s = &a[..idx];
-            break;
-        }
-        // If no further processing needed to be done, return the line as-is to be sorted
-        s = &a;
-    }
-
-    // And empty number or non-number lines are to be treated as ‘0’ but only for numeric sort
-    // All '0'-ed lines will be sorted later, but only amongst themselves, during the so-called 'last resort comparison.'
-    if s.is_empty() {
-        s = "0";
-    };
-    s
-}
-
 // This function cleans up the initial comparison done by leading_num_common for a general numeric compare.
 // In contrast to numeric compare, GNU general numeric/FP sort *should* recognize positive signs and
 // scientific notation, so we strip those lines only after the end of the following numeric string.
@@ -1054,17 +1103,6 @@ fn get_leading_gen(a: &str) -> &str {
     result
 }
 
-#[inline(always)]
-fn remove_thousands_sep<'a, S: Into<Cow<'a, str>>>(input: S) -> Cow<'a, str> {
-    let input = input.into();
-    if input.contains(THOUSANDS_SEP) {
-        let output = input.replace(THOUSANDS_SEP, "");
-        Cow::Owned(output)
-    } else {
-        input
-    }
-}
-
 #[inline(always)]
 fn remove_trailing_dec<'a, S: Into<Cow<'a, str>>>(input: S) -> Cow<'a, str> {
     let input = input.into();
@@ -1093,87 +1131,15 @@ fn permissive_f64_parse(a: &str) -> f64 {
     }
 }
 
-fn numeric_compare(a: &str, b: &str) -> Ordering {
-    #![allow(clippy::comparison_chain)]
-
-    let sa = get_leading_num(a);
-    let sb = get_leading_num(b);
-
-    // Avoids a string alloc for every line to remove thousands seperators here
-    // instead of inside the get_leading_num function, which is a HUGE performance benefit
-    let ta = remove_thousands_sep(sa);
-    let tb = remove_thousands_sep(sb);
-
-    let fa = permissive_f64_parse(&ta);
-    let fb = permissive_f64_parse(&tb);
-
-    // f64::cmp isn't implemented (due to NaN issues); implement directly instead
-    if fa > fb {
-        Ordering::Greater
-    } else if fa < fb {
-        Ordering::Less
-    } else {
-        Ordering::Equal
-    }
-}
-
 /// Compares two floats, with errors and non-numerics assumed to be -inf.
 /// Stops coercing at the first non-numeric char.
-fn general_numeric_compare(a: &str, b: &str) -> Ordering {
+/// We explicitly need to convert to f64 in this case.
+fn general_numeric_compare(a: f64, b: f64) -> Ordering {
     #![allow(clippy::comparison_chain)]
-
-    let sa = get_leading_gen(a);
-    let sb = get_leading_gen(b);
-
-    let fa = permissive_f64_parse(&sa);
-    let fb = permissive_f64_parse(&sb);
-
     // f64::cmp isn't implemented (due to NaN issues); implement directly instead
-    if fa > fb {
+    if a > b {
         Ordering::Greater
-    } else if fa < fb {
+    } else if a < b {
-        Ordering::Less
-    } else {
-        Ordering::Equal
-    }
-}
-
-// GNU/BSD does not handle converting numbers to an equal scale
-// properly.  GNU/BSD simply recognize that there is a human scale and sorts
-// those numbers ahead of other number inputs. There are perhaps limits
-// to the type of behavior we should emulate, and this might be such a limit.
-// Properly handling these units seems like a value add to me. And when sorting
-// these types of numbers, we rarely care about pure performance.
-fn human_numeric_convert(a: &str) -> f64 {
-    let num_str = get_leading_num(a);
-    let suffix = a.trim_start_matches(&num_str);
-    let num_part = permissive_f64_parse(&num_str);
-    let suffix: f64 = match suffix.parse().unwrap_or('\0') {
-        // SI Units
-        'K' => 1E3,
-        'M' => 1E6,
-        'G' => 1E9,
-        'T' => 1E12,
-        'P' => 1E15,
-        'E' => 1E18,
-        'Z' => 1E21,
-        'Y' => 1E24,
-        _ => 1f64,
-    };
-    num_part * suffix
-}
-
-/// Compare two strings as if they are human readable sizes.
-/// AKA 1M > 100k
-fn human_numeric_size_compare(a: &str, b: &str) -> Ordering {
-    #![allow(clippy::comparison_chain)]
-    let fa = human_numeric_convert(a);
-    let fb = human_numeric_convert(b);
-
-    // f64::cmp isn't implemented (due to NaN issues); implement directly instead
-    if fa > fb {
-        Ordering::Greater
-    } else if fa < fb {
         Ordering::Less
     } else {
         Ordering::Equal
@@ -1226,7 +1192,7 @@ fn month_parse(line: &str) -> Month {
     // GNU splits at any 3 letter match "JUNNNN" is JUN
     let pattern = if line.trim().len().ge(&3) {
         // Split a 3 and get first element of tuple ".0"
-        line.split_at(3).0
+        line.trim().split_at(3).0
     } else {
         ""
     };
@@ -1262,10 +1228,21 @@ fn month_compare(a: &str, b: &str) -> Ordering {
     }
 }
 
+fn version_parse(a: &str) -> Version {
+    let result = Version::parse(a);
+
+    match result {
+        Ok(vers_a) => vers_a,
+        // Non-version lines parse to 0.0.0
+        Err(_e) => Version::parse("0.0.0").unwrap(),
+    }
+}
+
 fn version_compare(a: &str, b: &str) -> Ordering {
     #![allow(clippy::comparison_chain)]
-    let ver_a = Version::parse(a);
+    let ver_a = version_parse(a);
-    let ver_b = Version::parse(b);
+    let ver_b = version_parse(b);
+
     // Version::cmp is not implemented; implement comparison directly
     if ver_a > ver_b {
         Ordering::Greater
@@ -1362,30 +1339,6 @@ mod tests {
         assert_eq!(Ordering::Less, default_compare(a, b));
     }
 
-    #[test]
-    fn test_numeric_compare1() {
-        let a = "149:7";
-        let b = "150:5";
-
-        assert_eq!(Ordering::Less, numeric_compare(a, b));
-    }
-
-    #[test]
-    fn test_numeric_compare2() {
-        let a = "-1.02";
-        let b = "1";
-
-        assert_eq!(Ordering::Less, numeric_compare(a, b));
-    }
-
-    #[test]
-    fn test_human_numeric_compare() {
-        let a = "300K";
-        let b = "1M";
-
-        assert_eq!(Ordering::Less, human_numeric_size_compare(a, b));
-    }
-
     #[test]
     fn test_month_compare() {
         let a = "JaN";

src/uu/stdbuf/src/libstdbuf/src/libstdbuf.rs

@@ -35,8 +35,8 @@ extern "C" {
 
 fn set_buffer(stream: *mut FILE, value: &str) {
     let (mode, size): (c_int, size_t) = match value {
-        "0" => (_IONBF, 0 as size_t),
+        "0" => (_IONBF, 0_usize),
-        "L" => (_IOLBF, 0 as size_t),
+        "L" => (_IOLBF, 0_usize),
         input => {
             let buff_size: usize = match input.parse() {
                 Ok(num) => num,

src/uu/stdbuf/src/stdbuf.rs

@@ -141,12 +141,12 @@ fn parse_size(size: &str) -> Option<u64> {
 
 fn check_option(matches: &ArgMatches, name: &str) -> Result<BufferType, ProgramOptionsError> {
     match matches.value_of(name) {
-        Some(value) => match &value[..] {
+        Some(value) => match value {
             "L" => {
                 if name == options::INPUT {
-                    Err(ProgramOptionsError(format!(
+                    Err(ProgramOptionsError(
-                        "line buffering stdin is meaningless"
+                        "line buffering stdin is meaningless".to_string(),
-                    )))
+                    ))
                 } else {
                     Ok(BufferType::Line)
                 }

src/uu/sum/src/sum.rs

@@ -75,7 +75,7 @@ fn open(name: &str) -> Result<Box<dyn Read>> {
                     "Is a directory",
                 ));
             };
-            if !path.metadata().is_ok() {
+            if path.metadata().is_err() {
                 return Err(std::io::Error::new(
                     std::io::ErrorKind::NotFound,
                     "No such file or directory",

src/uu/tac/src/tac.rs

@@ -90,7 +90,7 @@ fn tac(filenames: Vec<String>, before: bool, _: bool, separator: &str) -> i32 {
             Box::new(stdin()) as Box<dyn Read>
         } else {
             let path = Path::new(filename);
-            if path.is_dir() || !path.metadata().is_ok() {
+            if path.is_dir() || path.metadata().is_err() {
                 show_error!(
                     "failed to open '{}' for reading: No such file or directory",
                     filename

src/uu/test/src/test.rs

@@ -55,16 +55,16 @@ fn two(args: &[&[u8]], error: &mut bool) -> bool {
         b"-d" => path(args[1], PathCondition::Directory),
         b"-e" => path(args[1], PathCondition::Exists),
         b"-f" => path(args[1], PathCondition::Regular),
-        b"-g" => path(args[1], PathCondition::GroupIDFlag),
+        b"-g" => path(args[1], PathCondition::GroupIdFlag),
         b"-h" => path(args[1], PathCondition::SymLink),
         b"-L" => path(args[1], PathCondition::SymLink),
         b"-n" => one(&args[1..]),
-        b"-p" => path(args[1], PathCondition::FIFO),
+        b"-p" => path(args[1], PathCondition::Fifo),
         b"-r" => path(args[1], PathCondition::Readable),
         b"-S" => path(args[1], PathCondition::Socket),
         b"-s" => path(args[1], PathCondition::NonEmpty),
         b"-t" => isatty(args[1]),
-        b"-u" => path(args[1], PathCondition::UserIDFlag),
+        b"-u" => path(args[1], PathCondition::UserIdFlag),
         b"-w" => path(args[1], PathCondition::Writable),
         b"-x" => path(args[1], PathCondition::Executable),
         b"-z" => !one(&args[1..]),
@@ -322,13 +322,13 @@ enum PathCondition {
     Directory,
     Exists,
     Regular,
-    GroupIDFlag,
+    GroupIdFlag,
     SymLink,
-    FIFO,
+    Fifo,
     Readable,
     Socket,
     NonEmpty,
-    UserIDFlag,
+    UserIdFlag,
     Writable,
     Executable,
 }
@@ -390,13 +390,13 @@ fn path(path: &[u8], cond: PathCondition) -> bool {
         PathCondition::Directory => file_type.is_dir(),
         PathCondition::Exists => true,
         PathCondition::Regular => file_type.is_file(),
-        PathCondition::GroupIDFlag => metadata.mode() & S_ISGID != 0,
+        PathCondition::GroupIdFlag => metadata.mode() & S_ISGID != 0,
         PathCondition::SymLink => metadata.file_type().is_symlink(),
-        PathCondition::FIFO => file_type.is_fifo(),
+        PathCondition::Fifo => file_type.is_fifo(),
         PathCondition::Readable => perm(metadata, Permission::Read),
         PathCondition::Socket => file_type.is_socket(),
         PathCondition::NonEmpty => metadata.size() > 0,
-        PathCondition::UserIDFlag => metadata.mode() & S_ISUID != 0,
+        PathCondition::UserIdFlag => metadata.mode() & S_ISUID != 0,
         PathCondition::Writable => perm(metadata, Permission::Write),
         PathCondition::Executable => perm(metadata, Permission::Execute),
     }
@@ -416,13 +416,13 @@ fn path(path: &[u8], cond: PathCondition) -> bool {
         PathCondition::Directory => stat.is_dir(),
         PathCondition::Exists => true,
         PathCondition::Regular => stat.is_file(),
-        PathCondition::GroupIDFlag => false,
+        PathCondition::GroupIdFlag => false,
         PathCondition::SymLink => false,
-        PathCondition::FIFO => false,
+        PathCondition::Fifo => false,
         PathCondition::Readable => false, // TODO
         PathCondition::Socket => false,
         PathCondition::NonEmpty => stat.len() > 0,
-        PathCondition::UserIDFlag => false,
+        PathCondition::UserIdFlag => false,
         PathCondition::Writable => false,   // TODO
         PathCondition::Executable => false, // TODO
     }

src/uu/touch/src/touch.rs

@@ -18,6 +18,7 @@ use filetime::*;
 use std::fs::{self, File};
 use std::io::Error;
 use std::path::Path;
+use std::process;
 
 static VERSION: &str = env!("CARGO_PKG_VERSION");
 static ABOUT: &str = "Update the access and modification times of each FILE to the current time.";
@@ -137,7 +138,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     let (mut atime, mut mtime) = if matches.is_present(options::sources::REFERENCE) {
         stat(
-            &matches.value_of(options::sources::REFERENCE).unwrap()[..],
+            matches.value_of(options::sources::REFERENCE).unwrap(),
             !matches.is_present(options::NO_DEREF),
         )
     } else if matches.is_present(options::sources::DATE)
@@ -261,7 +262,27 @@ fn parse_timestamp(s: &str) -> FileTime {
     };
 
     match time::strptime(&ts, format) {
-        Ok(tm) => local_tm_to_filetime(to_local(tm)),
+        Ok(tm) => {
+            let mut local = to_local(tm);
+            local.tm_isdst = -1;
+            let ft = local_tm_to_filetime(local);
+
+            // We have to check that ft is valid time. Due to daylight saving
+            // time switch, local time can jump from 1:59 AM to 3:00 AM,
+            // in which case any time between 2:00 AM and 2:59 AM is not valid.
+            // Convert back to local time and see if we got the same value back.
+            let ts = time::Timespec {
+                sec: ft.unix_seconds(),
+                nsec: 0,
+            };
+            let tm2 = time::at(ts);
+            if tm.tm_hour != tm2.tm_hour {
+                show_error!("invalid date format {}", s);
+                process::exit(1);
+            }
+
+            ft
+        }
         Err(e) => panic!("Unable to parse timestamp\n{}", e),
     }
 }

src/uu/tr/src/expand.rs

@@ -24,7 +24,7 @@ use std::ops::RangeInclusive;
 fn parse_sequence(s: &str) -> (char, usize) {
     let c = s.chars().next().expect("invalid escape: empty string");
 
-    if '0' <= c && c <= '7' {
+    if ('0'..='7').contains(&c) {
         let mut v = c.to_digit(8).unwrap();
         let mut consumed = 1;
         let bits_per_digit = 3;

src/uu/tsort/src/tsort.rs

@@ -16,8 +16,8 @@ use std::io::{stdin, BufRead, BufReader, Read};
 use std::path::Path;
 
 static VERSION: &str = env!("CARGO_PKG_VERSION");
-static SUMMARY: &str = "Topological sort the strings in FILE. 
+static SUMMARY: &str = "Topological sort the strings in FILE.
-Strings are defined as any sequence of tokens separated by whitespace (tab, space, or newline). 
+Strings are defined as any sequence of tokens separated by whitespace (tab, space, or newline).
 If FILE is not passed in, stdin is used instead.";
 static USAGE: &str = "tsort [OPTIONS] FILE";
 
@@ -32,13 +32,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         .version(VERSION)
         .usage(USAGE)
         .about(SUMMARY)
-        .arg(Arg::with_name(options::FILE).hidden(true))
+        .arg(
+            Arg::with_name(options::FILE)
+                .default_value("-")
+                .hidden(true),
+        )
         .get_matches_from(args);
 
-    let input = match matches.value_of(options::FILE) {
+    let input = matches
-        Some(v) => v,
+        .value_of(options::FILE)
-        None => "-",
+        .expect("Value is required by clap");
-    };
 
     let mut stdin_buf;
     let mut file_buf;

src/uu/tty/src/tty.rs

@@ -65,9 +65,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         }
     }
 
-    return if is_stdin_interactive() {
+    if is_stdin_interactive() {
         libc::EXIT_SUCCESS
     } else {
         libc::EXIT_FAILURE
-    };
+    }
 }

src/uu/unexpand/src/unexpand.rs

@@ -149,10 +149,8 @@ fn next_tabstop(tabstops: &[usize], col: usize) -> Option<usize> {
         Some(tabstops[0] - col % tabstops[0])
     } else {
         // find next larger tab
-        match tabstops.iter().find(|&&t| t > col) {
+        // if there isn't one in the list, tab becomes a single space
-            Some(t) => Some(t - col),
+        tabstops.iter().find(|&&t| t > col).map(|t| t - col)
-            None => None, // if there isn't one in the list, tab becomes a single space
-        }
     }
 }
 

src/uu/wc/src/count_bytes.rs

@@ -72,30 +72,27 @@ pub(crate) fn count_bytes_fast<T: WordCountable>(handle: &mut T) -> WcResult<usi
     #[cfg(unix)]
     {
         let fd = handle.as_raw_fd();
-        match fstat(fd) {
+        if let Ok(stat) = fstat(fd) {
-            Ok(stat) => {
+            // If the file is regular, then the `st_size` should hold
-                // If the file is regular, then the `st_size` should hold
+            // the file's size in bytes.
-                // the file's size in bytes.
+            if (stat.st_mode & S_IFREG) != 0 {
-                if (stat.st_mode & S_IFREG) != 0 {
+                return Ok(stat.st_size as usize);
-                    return Ok(stat.st_size as usize);
+            }
-                }
+            #[cfg(any(target_os = "linux", target_os = "android"))]
-                #[cfg(any(target_os = "linux", target_os = "android"))]
+            {
-                {
+                // Else, if we're on Linux and our file is a FIFO pipe
-                    // Else, if we're on Linux and our file is a FIFO pipe
+                // (or stdin), we use splice to count the number of bytes.
-                    // (or stdin), we use splice to count the number of bytes.
+                if (stat.st_mode & S_IFIFO) != 0 {
-                    if (stat.st_mode & S_IFIFO) != 0 {
+                    if let Ok(n) = count_bytes_using_splice(fd) {
-                        if let Ok(n) = count_bytes_using_splice(fd) {
+                        return Ok(n);
-                            return Ok(n);
-                        }
                     }
                 }
             }
-            _ => {}
         }
     }
 
     // Fall back on `read`, but without the overhead of counting words and lines.
-    let mut buf = [0 as u8; BUF_SIZE];
+    let mut buf = [0_u8; BUF_SIZE];
     let mut byte_count = 0;
     loop {
         match handle.read(&mut buf) {

src/uu/wc/src/wc.rs

@@ -138,11 +138,8 @@ impl AddAssign for WordCount {
 }
 
 impl WordCount {
-    fn with_title<'a>(self, title: &'a str) -> TitledWordCount<'a> {
+    fn with_title(self, title: &str) -> TitledWordCount {
-        return TitledWordCount {
+        TitledWordCount { title, count: self }
-            title: title,
-            count: self,
-        };
     }
 }
 
@@ -251,7 +248,7 @@ fn is_word_separator(byte: u8) -> bool {
 fn word_count_from_reader<T: WordCountable>(
     mut reader: T,
     settings: &Settings,
-    path: &String,
+    path: &str,
 ) -> WcResult<WordCount> {
     let only_count_bytes = settings.show_bytes
         && (!(settings.show_chars
@@ -333,18 +330,18 @@ fn word_count_from_reader<T: WordCountable>(
     })
 }
 
-fn word_count_from_path(path: &String, settings: &Settings) -> WcResult<WordCount> {
+fn word_count_from_path(path: &str, settings: &Settings) -> WcResult<WordCount> {
     if path == "-" {
         let stdin = io::stdin();
         let stdin_lock = stdin.lock();
-        return Ok(word_count_from_reader(stdin_lock, settings, path)?);
+        word_count_from_reader(stdin_lock, settings, path)
     } else {
         let path_obj = Path::new(path);
         if path_obj.is_dir() {
-            return Err(WcError::IsDirectory(path.clone()));
+            Err(WcError::IsDirectory(path.to_owned()))
         } else {
             let file = File::open(path)?;
-            return Ok(word_count_from_reader(file, settings, path)?);
+            word_count_from_reader(file, settings, path)
         }
     }
 }
@@ -425,7 +422,7 @@ fn print_stats(
     }
 
     if result.title == "-" {
-        writeln!(stdout_lock, "")?;
+        writeln!(stdout_lock)?;
     } else {
         writeln!(stdout_lock, " {}", result.title)?;
     }

src/uu/who/src/who.rs

@@ -222,7 +222,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         need_runlevel,
         need_users,
         my_line_only,
-        has_records: false,
         args: matches.free,
     };
 
@@ -247,7 +246,6 @@ struct Who {
     need_runlevel: bool,
     need_users: bool,
     my_line_only: bool,
-    has_records: bool,
     args: Vec<String>,
 }
 
@@ -321,8 +319,7 @@ impl Who {
             println!("{}", users.join(" "));
             println!("# users={}", users.len());
         } else {
-            let mut records = Utmpx::iter_all_records().read_from(f).peekable();
+            let records = Utmpx::iter_all_records().read_from(f).peekable();
-            self.has_records = records.peek().is_some();
 
             if self.include_heading {
                 self.print_heading()

src/uucore/src/lib/features/perms.rs

@@ -31,9 +31,9 @@ fn chgrp<P: AsRef<Path>>(path: P, dgid: gid_t, follow: bool) -> IOResult<()> {
     let s = CString::new(path.as_os_str().as_bytes()).unwrap();
     let ret = unsafe {
         if follow {
-            libc::chown(s.as_ptr(), (0 as gid_t).wrapping_sub(1), dgid)
+            libc::chown(s.as_ptr(), 0_u32.wrapping_sub(1), dgid)
         } else {
-            lchown(s.as_ptr(), (0 as gid_t).wrapping_sub(1), dgid)
+            lchown(s.as_ptr(), 0_u32.wrapping_sub(1), dgid)
         }
     };
     if ret == 0 {

src/uucore/src/lib/macros.rs

@@ -31,6 +31,14 @@ macro_rules! show_error(
 );
 
 /// Show a warning to stderr in a silimar style to GNU coreutils.
+#[macro_export]
+macro_rules! show_error_custom_description (
+    ($err:expr,$($args:tt)+) => ({
+        eprint!("{}: {}: ", executable!(), $err);
+        eprintln!($($args)+);
+    })
+);
+
 #[macro_export]
 macro_rules! show_warning(
     ($($args:tt)+) => ({

tests/by-util/test_cat.rs

@@ -26,7 +26,7 @@ fn test_no_options() {
 }
 
 #[test]
-#[cfg(unix)]
+#[cfg(any(target_vendor = "apple", target_os = "linux", target_os = "android"))]
 fn test_no_options_big_input() {
     for &n in &[
         0,

tests/by-util/test_chmod.rs

@@ -357,7 +357,8 @@ fn test_chmod_symlink_non_existing_file() {
     at.symlink_file(non_existing, test_symlink);
 
     // this cannot succeed since the symbolic link dangles
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("755")
         .arg("-v")
         .arg(test_symlink)
@@ -367,7 +368,8 @@ fn test_chmod_symlink_non_existing_file() {
         .stderr_contains(expected_stderr);
 
     // this should be the same than with just '-v' but without stderr
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("755")
         .arg("-v")
         .arg("-f")
@@ -394,7 +396,8 @@ fn test_chmod_symlink_non_existing_file_recursive() {
     );
 
     // this should succeed
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("-R")
         .arg("755")
         .arg(test_directory)
@@ -408,7 +411,8 @@ fn test_chmod_symlink_non_existing_file_recursive() {
     );
 
     // '-v': this should succeed without stderr
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("-R")
         .arg("-v")
         .arg("755")
@@ -418,7 +422,8 @@ fn test_chmod_symlink_non_existing_file_recursive() {
         .no_stderr();
 
     // '-vf': this should be the same than with just '-v'
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("-R")
         .arg("-v")
         .arg("-f")

tests/by-util/test_chown.rs

@@ -4,6 +4,34 @@ use rust_users::get_effective_uid;
 
 extern crate chown;
 
+// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'.
+// If we are running inside the CI and "needle" is in "stderr" skipping this test is
+// considered okay. If we are not inside the CI this calls assert!(result.success).
+//
+// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
+//
+// stderr: "whoami: cannot find name for user ID 1001"
+// TODO: Maybe `adduser --uid 1001 username` can put things right?
+//
+// stderr: "id: cannot find name for group ID 116"
+// stderr: "thread 'main' panicked at 'called `Result::unwrap()` on an `Err`
+//     value: Custom { kind: NotFound, error: "No such id: 1001" }',
+//     /project/src/uucore/src/lib/features/perms.rs:176:44"
+//
+fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool {
+    if !result.succeeded() {
+        println!("result.stdout = {}", result.stdout_str());
+        println!("result.stderr = {}", result.stderr_str());
+        if is_ci() && result.stderr_str().contains(needle) {
+            println!("test skipped:");
+            return true;
+        } else {
+            result.success();
+        }
+    }
+    false
+}
+
 #[cfg(test)]
 mod test_passgrp {
     use super::chown::entries::{gid2grp, grp2gid, uid2usr, usr2uid};
@@ -49,116 +77,193 @@ fn test_invalid_option() {
 }
 
 #[test]
-fn test_chown_myself() {
+fn test_chown_only_owner() {
     // test chown username file.txt
+
     let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    println!("results {}", result.stdout_str());
+    let user_name = String::from(result.stdout_str().trim());
-    let username = result.stdout_str().trim_end();
+    assert!(!user_name.is_empty());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
 
+    let file1 = "test_chown_file1";
     at.touch(file1);
-    let result = ucmd.arg(username).arg(file1).run();
-    println!("results stdout {}", result.stdout_str());
-    println!("results stderr {}", result.stderr_str());
-    if is_ci() && result.stderr_str().contains("invalid user") {
-        // In the CI, some server are failing to return id.
-        // As seems to be a configuration issue, ignoring it
-        return;
-    }
-    assert!(result.success);
-}
 
-#[test]
+    // since only superuser can change owner, we have to change from ourself to ourself
-fn test_chown_myself_second() {
+    let result = scene
-    // test chown username: file.txt
+        .ucmd()
-    let scene = TestScenario::new(util_name!());
+        .arg(user_name)
-    let result = scene.cmd("whoami").run();
+        .arg("--verbose")
-    if is_ci() && result.stderr_str().contains("No such user/group") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
-        return;
-    }
-    println!("results {}", result.stdout_str());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
-
-    at.touch(file1);
-    let result = ucmd
-        .arg(result.stdout_str().trim_end().to_owned() + ":")
         .arg(file1)
         .run();
+    result.stderr_contains(&"retained as");
 
-    println!("result.stdout = {}", result.stdout_str());
+    // try to change to another existing user, e.g. 'root'
-    println!("result.stderr = {}", result.stderr_str());
+    scene
-    assert!(result.success);
+        .ucmd()
+        .arg("root")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
-fn test_chown_myself_group() {
+fn test_chown_only_owner_colon() {
-    // test chown username:group file.txt
+    // test chown username: file.txt
+
     let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    println!("user name = {}", result.stdout_str());
+    let user_name = String::from(result.stdout_str().trim());
-    let username = result.stdout_str().trim_end();
+    assert!(!user_name.is_empty());
+
+    let file1 = "test_chown_file1";
+    at.touch(file1);
+
+    scene
+        .ucmd()
+        .arg(format!("{}:", user_name))
+        .arg("--verbose")
+        .arg(file1)
+        .succeeds()
+        .stderr_contains(&"retained as");
+
+    scene
+        .ucmd()
+        .arg("root:")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
+}
+
+#[test]
+fn test_chown_only_colon() {
+    // test chown : file.txt
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let file1 = "test_chown_file1";
+    at.touch(file1);
+
+    // expected:
+    // $ chown -v : file.txt 2>out_err ; echo $? ; cat out_err
+    // ownership of 'file.txt' retained
+    // 0
+    let result = scene.ucmd().arg(":").arg("--verbose").arg(file1).run();
+    if skipping_test_is_okay(&result, "No such id") {
+        return;
+    }
+    result.stderr_contains(&"retained as"); // TODO: verbose is not printed to stderr in GNU chown
+
+    // test chown : file.txt
+    // expected:
+    // $ chown -v :: file.txt 2>out_err ; echo $? ; cat out_err
+    // 1
+    // chown: invalid group: ‘::’
+    scene
+        .ucmd()
+        .arg("::")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"invalid group: ‘::’");
+}
+
+#[test]
+fn test_chown_failed_stdout() {
+    // test chown root file.txt
+
+    // TODO: implement once output "failed to change" to stdout is fixed
+    // expected:
+    // $ chown -v root file.txt 2>out_err ; echo $? ; cat out_err
+    // failed to change ownership of 'file.txt' from jhs to root
+    // 1
+    // chown: changing ownership of 'file.txt': Operation not permitted
+}
+
+#[test]
+fn test_chown_owner_group() {
+    // test chown username:group file.txt
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let result = scene.cmd("whoami").run();
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
+        return;
+    }
+
+    let user_name = String::from(result.stdout_str().trim());
+    assert!(!user_name.is_empty());
+
+    let file1 = "test_chown_file1";
+    at.touch(file1);
 
     let result = scene.cmd("id").arg("-gn").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    println!("group name = {}", result.stdout_str());
+    let group_name = String::from(result.stdout_str().trim());
-    let group = result.stdout_str().trim_end();
+    assert!(!group_name.is_empty());
 
-    let (at, mut ucmd) = at_and_ucmd!();
+    let result = scene
-    let file1 = "test_install_target_dir_file_a1";
+        .ucmd()
-    let perm = username.to_owned() + ":" + group;
+        .arg(format!("{}:{}", user_name, group_name))
-    at.touch(file1);
+        .arg("--verbose")
-    let result = ucmd.arg(perm).arg(file1).run();
+        .arg(file1)
-    println!("result.stdout = {}", result.stdout_str());
+        .run();
-    println!("result.stderr = {}", result.stderr_str());
+    if skipping_test_is_okay(&result, "chown: invalid group:") {
-    if is_ci() && result.stderr_str().contains("chown: invalid group:") {
-        // With some Ubuntu into the CI, we can get this answer
         return;
     }
-    assert!(result.success);
+    result.stderr_contains(&"retained as");
+
+    // TODO: on macos group name is not recognized correctly: "chown: invalid group: 'root:root'
+    #[cfg(any(windows, all(unix, not(target_os = "macos"))))]
+    scene
+        .ucmd()
+        .arg("root:root")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
+// TODO: on macos group name is not recognized correctly: "chown: invalid group: ':groupname'
+#[cfg(any(windows, all(unix, not(target_os = "macos"))))]
 fn test_chown_only_group() {
     // test chown :group file.txt
+
     let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    println!("results {}", result.stdout_str());
+    let user_name = String::from(result.stdout_str().trim());
+    assert!(!user_name.is_empty());
 
-    let (at, mut ucmd) = at_and_ucmd!();
+    let file1 = "test_chown_file1";
-    let file1 = "test_install_target_dir_file_a1";
-    let perm = ":".to_owned() + result.stdout_str().trim_end();
     at.touch(file1);
-    let result = ucmd.arg(perm).arg(file1).run();
-
-    println!("result.stdout = {}", result.stdout_str());
-    println!("result.stderr = {}", result.stderr_str());
 
+    let result = scene
+        .ucmd()
+        .arg(format!(":{}", user_name))
+        .arg("--verbose")
+        .arg(file1)
+        .run();
     if is_ci() && result.stderr_str().contains("Operation not permitted") {
         // With ubuntu with old Rust in the CI, we can get an error
         return;
@@ -167,221 +272,232 @@ fn test_chown_only_group() {
         // With mac into the CI, we can get this answer
         return;
     }
-    assert!(result.success);
+    result.stderr_contains(&"retained as");
+    result.success();
+
+    scene
+        .ucmd()
+        .arg(":root")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
-fn test_chown_only_id() {
+fn test_chown_only_user_id() {
     // test chown 1111 file.txt
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run();
+
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    let scene = TestScenario::new(util_name!());
-        // In the CI, some server are failing to return whoami.
+    let at = &scene.fixtures;
-        // As seems to be a configuration issue, ignoring it
+
+    let result = scene.cmd_keepenv("id").arg("-u").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let user_id = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!user_id.is_empty());
-    let id = String::from(result.stdout_str().trim());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
 
+    let file1 = "test_chown_file1";
     at.touch(file1);
-    let result = ucmd.arg(id).arg(file1).run();
 
-    println!("result.stdout = {}", result.stdout_str());
+    let result = scene.ucmd().arg(user_id).arg("--verbose").arg(file1).run();
-    println!("result.stderr = {}", result.stderr_str());
+    if skipping_test_is_okay(&result, "invalid user") {
-    if is_ci() && result.stderr_str().contains("chown: invalid user:") {
+        // From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
-        // With some Ubuntu into the CI, we can get this answer
+        // stderr: "chown: invalid user: '1001'
         return;
     }
-    assert!(result.success);
+    result.stderr_contains(&"retained as");
+
+    scene
+        .ucmd()
+        .arg("0")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
 fn test_chown_only_group_id() {
     // test chown :1111 file.txt
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-g").run();
+
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    let scene = TestScenario::new(util_name!());
-        // In the CI, some server are failing to return whoami.
+    let at = &scene.fixtures;
-        // As seems to be a configuration issue, ignoring it
+
+    let result = scene.cmd_keepenv("id").arg("-g").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let group_id = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!group_id.is_empty());
-    let id = String::from(result.stdout_str().trim());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
 
+    let file1 = "test_chown_file1";
     at.touch(file1);
-    let perm = ":".to_owned() + &id;
 
-    let result = ucmd.arg(perm).arg(file1).run();
+    let result = scene
-
+        .ucmd()
-    println!("result.stdout = {}", result.stdout_str());
+        .arg(format!(":{}", group_id))
-    println!("result.stderr = {}", result.stderr_str());
+        .arg("--verbose")
-    if is_ci() && result.stderr_str().contains("chown: invalid group:") {
+        .arg(file1)
+        .run();
+    if skipping_test_is_okay(&result, "chown: invalid group:") {
         // With mac into the CI, we can get this answer
         return;
     }
-    assert!(result.success);
+    result.stderr_contains(&"retained as");
+
+    // Apparently on CI "macos-latest, x86_64-apple-darwin, feat_os_macos"
+    // the process has the rights to change from runner:staff to runner:wheel
+    #[cfg(any(windows, all(unix, not(target_os = "macos"))))]
+    scene
+        .ucmd()
+        .arg(":0")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
-fn test_chown_both_id() {
+fn test_chown_owner_group_id() {
     // test chown 1111:1111 file.txt
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run();
+
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    let scene = TestScenario::new(util_name!());
-        // In the CI, some server are failing to return whoami.
+    let at = &scene.fixtures;
-        // As seems to be a configuration issue, ignoring it
+
+    let result = scene.cmd_keepenv("id").arg("-u").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let user_id = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!user_id.is_empty());
-    let id_user = String::from(result.stdout_str().trim());
 
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-g").run();
+    let result = scene.cmd_keepenv("id").arg("-g").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let group_id = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!group_id.is_empty());
-    let id_group = String::from(result.stdout_str().trim());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
 
+    let file1 = "test_chown_file1";
     at.touch(file1);
-    let perm = id_user + &":".to_owned() + &id_group;
 
-    let result = ucmd.arg(perm).arg(file1).run();
+    let result = scene
-    println!("result.stdout = {}", result.stdout_str());
+        .ucmd()
-    println!("result.stderr = {}", result.stderr_str());
+        .arg(format!("{}:{}", user_id, group_id))
-
+        .arg("--verbose")
-    if is_ci() && result.stderr_str().contains("invalid user") {
+        .arg(file1)
-        // In the CI, some server are failing to return id.
+        .run();
-        // As seems to be a configuration issue, ignoring it
+    if skipping_test_is_okay(&result, "invalid user") {
+        // From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
+        // stderr: "chown: invalid user: '1001:116'
         return;
     }
+    result.stderr_contains(&"retained as");
 
-    assert!(result.success);
+    scene
+        .ucmd()
+        .arg("0:0")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
-fn test_chown_both_mix() {
+fn test_chown_owner_group_mix() {
-    // test chown 1111:1111 file.txt
+    // test chown 1111:group file.txt
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
-        return;
-    }
-    println!("result.stdout = {}", result.stdout_str());
-    println!("result.stderr = {}", result.stderr_str());
-    let id_user = String::from(result.stdout_str().trim());
 
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-gn").run();
+    let scene = TestScenario::new(util_name!());
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    let at = &scene.fixtures;
-        // In the CI, some server are failing to return whoami.
+
-        // As seems to be a configuration issue, ignoring it
+    let result = scene.cmd_keepenv("id").arg("-u").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
+        return;
+    }
+    let user_id = String::from(result.stdout_str().trim());
+    assert!(!user_id.is_empty());
+
+    let result = scene.cmd_keepenv("id").arg("-gn").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
-    println!("result.stderr = {}", result.stderr_str());
     let group_name = String::from(result.stdout_str().trim());
+    assert!(!group_name.is_empty());
 
-    let (at, mut ucmd) = at_and_ucmd!();
+    let file1 = "test_chown_file1";
-    let file1 = "test_install_target_dir_file_a1";
-
     at.touch(file1);
-    let perm = id_user + &":".to_owned() + &group_name;
 
-    let result = ucmd.arg(perm).arg(file1).run();
+    let result = scene
+        .ucmd()
+        .arg(format!("{}:{}", user_id, group_name))
+        .arg("--verbose")
+        .arg(file1)
+        .run();
+    result.stderr_contains(&"retained as");
 
-    if is_ci() && result.stderr_str().contains("invalid user") {
+    // TODO: on macos group name is not recognized correctly: "chown: invalid group: '0:root'
-        // In the CI, some server are failing to return id.
+    #[cfg(any(windows, all(unix, not(target_os = "macos"))))]
-        // As seems to be a configuration issue, ignoring it
+    scene
-        return;
+        .ucmd()
-    }
+        .arg("0:root")
-    assert!(result.success);
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
 fn test_chown_recursive() {
     let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let user_name = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!user_name.is_empty());
-    let username = result.stdout_str().trim_end();
 
-    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir_all("a/b/c");
-    at.mkdir("a");
-    at.mkdir("a/b");
-    at.mkdir("a/b/c");
     at.mkdir("z");
     at.touch(&at.plus_as_string("a/a"));
     at.touch(&at.plus_as_string("a/b/b"));
     at.touch(&at.plus_as_string("a/b/c/c"));
     at.touch(&at.plus_as_string("z/y"));
 
-    let result = ucmd
+    let result = scene
+        .ucmd()
         .arg("-R")
         .arg("--verbose")
-        .arg(username)
+        .arg(user_name)
         .arg("a")
         .arg("z")
         .run();
-    println!("result.stdout = {}", result.stdout_str());
+    result.stderr_contains(&"ownership of 'a/a' retained as");
-    println!("result.stderr = {}", result.stderr_str());
+    result.stderr_contains(&"ownership of 'z/y' retained as");
-    if is_ci() && result.stderr_str().contains("invalid user") {
-        // In the CI, some server are failing to return id.
-        // As seems to be a configuration issue, ignoring it
-        return;
-    }
-
-    result
-        .stderr_contains(&"ownership of 'a/a' retained as")
-        .stderr_contains(&"ownership of 'z/y' retained as")
-        .success();
 }
 
 #[test]
 fn test_root_preserve() {
     let scene = TestScenario::new(util_name!());
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let user_name = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!user_name.is_empty());
-    let username = result.stdout_str().trim_end();
 
-    let result = new_ucmd!()
+    let result = scene
+        .ucmd()
         .arg("--preserve-root")
         .arg("-R")
-        .arg(username)
+        .arg(user_name)
         .arg("/")
         .fails();
-    println!("result.stdout = {}", result.stdout_str());
+    result.stderr_contains(&"chown: it is dangerous to operate recursively");
-    println!("result.stderr = {}", result.stderr_str());
-    if is_ci() && result.stderr_str().contains("invalid user") {
-        // In the CI, some server are failing to return id.
-        // As seems to be a configuration issue, ignoring it
-        return;
-    }
-    assert!(result
-        .stderr
-        .contains("chown: it is dangerous to operate recursively"));
 }
 
 #[cfg(target_os = "linux")]
@@ -393,8 +509,34 @@ fn test_big_p() {
             .arg("bin")
             .arg("/proc/self/cwd")
             .fails()
-            .stderr_is(
+            .stderr_contains(
-                "chown: changing ownership of '/proc/self/cwd': Operation not permitted (os error 1)\n",
+                "chown: changing ownership of '/proc/self/cwd': Operation not permitted (os error 1)",
             );
     }
 }
+
+#[test]
+fn test_chown_file_notexisting() {
+    // test chown username not_existing
+
+    let scene = TestScenario::new(util_name!());
+
+    let result = scene.cmd("whoami").run();
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
+        return;
+    }
+    let user_name = String::from(result.stdout_str().trim());
+    assert!(!user_name.is_empty());
+
+    let _result = scene
+        .ucmd()
+        .arg(user_name)
+        .arg("--verbose")
+        .arg("not_existing")
+        .fails();
+
+    // TODO: uncomment once "failed to change ownership of '{}' to {}" added to stdout
+    // result.stderr_contains(&"retained as");
+    // TODO: uncomment once message changed from "cannot dereference" to "cannot access"
+    // result.stderr_contains(&"cannot access 'not_existing': No such file or directory");
+}

tests/by-util/test_cksum.rs

@@ -31,41 +31,50 @@ fn test_empty() {
 
     at.touch("a");
 
-    ucmd.arg("a").succeeds().stdout.ends_with("0 a");
+    ucmd.arg("a")
+        .succeeds()
+        .no_stderr()
+        .normalized_newlines_stdout_is("4294967295 0 a\n");
 }
 
 #[test]
-#[ignore]
 fn test_arg_overrides_stdin() {
     let (at, mut ucmd) = at_and_ucmd!();
     let input = "foobarfoobar";
 
     at.touch("a");
 
-    let result = ucmd.arg("a").pipe_in(input.as_bytes()).run();
+    ucmd.arg("a")
-
+        .pipe_in(input.as_bytes())
-    println!("{}, {}", result.stdout, result.stderr);
+        // the command might have exited before all bytes have been pipe in.
-
+        // in that case, we don't care about the error (broken pipe)
-    assert!(result.stdout.ends_with("0 a\n"))
+        .ignore_stdin_write_error()
+        .succeeds()
+        .no_stderr()
+        .normalized_newlines_stdout_is("4294967295 0 a\n");
 }
 
 #[test]
 fn test_invalid_file() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let ts = TestScenario::new(util_name!());
+    let at = ts.fixtures.clone();
 
-    let ls = TestScenario::new("ls");
+    let folder_name = "asdf";
-    let files = ls.cmd("ls").arg("-l").run();
-    println!("{:?}", files.stdout);
-    println!("{:?}", files.stderr);
 
-    let folder_name = "asdf".to_string();
+    // First check when file doesn't exist
+    ts.ucmd()
+        .arg(folder_name)
+        .fails()
+        .no_stdout()
+        .stderr_contains("cksum: error: 'asdf' No such file or directory");
 
-    let result = ucmd.arg(&folder_name).run();
+    // Then check when the file is of an invalid type
-
+    at.mkdir(folder_name);
-    println!("stdout: {:?}", result.stdout);
+    ts.ucmd()
-    println!("stderr: {:?}", result.stderr);
+        .arg(folder_name)
-    assert!(result.stderr.contains("cksum: error: 'asdf'"));
+        .fails()
-    assert!(!result.success);
+        .no_stdout()
+        .stderr_contains("cksum: error: 'asdf' Is a directory");
 }
 
 // Make sure crc is correct for files larger than 32 bytes
@@ -74,14 +83,13 @@ fn test_invalid_file() {
 fn test_crc_for_bigger_than_32_bytes() {
     let (_, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd.arg("chars.txt").run();
+    let result = ucmd.arg("chars.txt").succeeds();
 
-    let mut stdout_splitted = result.stdout.split(" ");
+    let mut stdout_splitted = result.stdout_str().split(" ");
 
     let cksum: i64 = stdout_splitted.next().unwrap().parse().unwrap();
     let bytes_cnt: i64 = stdout_splitted.next().unwrap().parse().unwrap();
 
-    assert!(result.success);
     assert_eq!(cksum, 586047089);
     assert_eq!(bytes_cnt, 16);
 }
@@ -90,14 +98,13 @@ fn test_crc_for_bigger_than_32_bytes() {
 fn test_stdin_larger_than_128_bytes() {
     let (_, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd.arg("larger_than_2056_bytes.txt").run();
+    let result = ucmd.arg("larger_than_2056_bytes.txt").succeeds();
 
-    let mut stdout_splitted = result.stdout.split(" ");
+    let mut stdout_splitted = result.stdout_str().split(" ");
 
     let cksum: i64 = stdout_splitted.next().unwrap().parse().unwrap();
     let bytes_cnt: i64 = stdout_splitted.next().unwrap().parse().unwrap();
 
-    assert!(result.success);
     assert_eq!(cksum, 945881979);
     assert_eq!(bytes_cnt, 2058);
 }

tests/by-util/test_cp.rs

@@ -1029,7 +1029,7 @@ fn test_cp_one_file_system() {
     at_src.mkdir(TEST_MOUNT_MOUNTPOINT);
     let mountpoint_path = &at_src.plus_as_string(TEST_MOUNT_MOUNTPOINT);
 
-    let _r = scene
+    scene
         .cmd("mount")
         .arg("-t")
         .arg("tmpfs")
@@ -1037,8 +1037,7 @@ fn test_cp_one_file_system() {
         .arg("size=640k") // ought to be enough
         .arg("tmpfs")
         .arg(mountpoint_path)
-        .run();
+        .succeeds();
-    assert!(_r.code == Some(0), "{}", _r.stderr);
 
     at_src.touch(TEST_MOUNT_OTHER_FILESYSTEM_FILE);
 
@@ -1051,8 +1050,7 @@ fn test_cp_one_file_system() {
         .run();
 
     // Ditch the mount before the asserts
-    let _r = scene.cmd("umount").arg(mountpoint_path).run();
+    scene.cmd("umount").arg(mountpoint_path).succeeds();
-    assert!(_r.code == Some(0), "{}", _r.stderr);
 
     assert!(result.success);
     assert!(!at_dst.file_exists(TEST_MOUNT_OTHER_FILESYSTEM_FILE));

tests/by-util/test_du.rs

@@ -7,12 +7,10 @@ const SUB_LINK: &str = "subdir/links/sublink.txt";
 
 #[test]
 fn test_du_basics() {
-    new_ucmd!()
+    new_ucmd!().succeeds().no_stderr();
-        .succeeds()
-        .no_stderr();
 }
 #[cfg(target_vendor = "apple")]
-fn _du_basics(s: String) {
+fn _du_basics(s: &str) {
     let answer = "32\t./subdir
 8\t./subdir/deeper
 24\t./subdir/links
@@ -32,11 +30,18 @@ fn _du_basics(s: &str) {
 
 #[test]
 fn test_du_basics_subdir() {
-    let (_at, mut ucmd) = at_and_ucmd!();
+    let scene = TestScenario::new(util_name!());
 
-    let result = ucmd.arg(SUB_DIR).run();
+    let result = scene.ucmd().arg(SUB_DIR).succeeds();
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg(SUB_DIR).run();
+        if result_reference.succeeded() {
+            assert_eq!(result.stdout_str(), result_reference.stdout_str());
+            return;
+        }
+    }
     _du_basics_subdir(result.stdout_str());
 }
 
@@ -60,26 +65,29 @@ fn _du_basics_subdir(s: &str) {
 
 #[test]
 fn test_du_basics_bad_name() {
-    let (_at, mut ucmd) = at_and_ucmd!();
+    new_ucmd!()
-
+        .arg("bad_name")
-    let result = ucmd.arg("bad_name").run();
+        .succeeds() // TODO: replace with ".fails()" once `du` is fixed
-    assert_eq!(result.stdout_str(), "");
+        .stderr_only("du: error: bad_name: No such file or directory\n");
-    assert_eq!(
-        result.stderr,
-        "du: error: bad_name: No such file or directory\n"
-    );
 }
 
 #[test]
 fn test_du_soft_link() {
-    let ts = TestScenario::new("du");
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
 
-    let link = ts.ccmd("ln").arg("-s").arg(SUB_FILE).arg(SUB_LINK).run();
+    at.symlink_file(SUB_FILE, SUB_LINK);
-    assert!(link.success);
 
-    let result = ts.ucmd().arg(SUB_DIR_LINKS).run();
+    let result = scene.ucmd().arg(SUB_DIR_LINKS).succeeds();
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg(SUB_DIR_LINKS).run();
+        if result_reference.succeeded() {
+            assert_eq!(result.stdout_str(), result_reference.stdout_str());
+            return;
+        }
+    }
     _du_soft_link(result.stdout_str());
 }
 
@@ -104,14 +112,23 @@ fn _du_soft_link(s: &str) {
 
 #[test]
 fn test_du_hard_link() {
-    let ts = TestScenario::new("du");
+    let scene = TestScenario::new(util_name!());
 
-    let link = ts.ccmd("ln").arg(SUB_FILE).arg(SUB_LINK).run();
+    let result_ln = scene.cmd("ln").arg(SUB_FILE).arg(SUB_LINK).run();
-    assert!(link.success);
+    if !result_ln.succeeded() {
+        scene.ccmd("ln").arg(SUB_FILE).arg(SUB_LINK).succeeds();
+    }
 
-    let result = ts.ucmd().arg(SUB_DIR_LINKS).run();
+    let result = scene.ucmd().arg(SUB_DIR_LINKS).succeeds();
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg(SUB_DIR_LINKS).run();
+        if result_reference.succeeded() {
+            assert_eq!(result.stdout_str(), result_reference.stdout_str());
+            return;
+        }
+    }
     // We do not double count hard links as the inodes are identical
     _du_hard_link(result.stdout_str());
 }
@@ -136,11 +153,23 @@ fn _du_hard_link(s: &str) {
 
 #[test]
 fn test_du_d_flag() {
-    let ts = TestScenario::new("du");
+    let scene = TestScenario::new(util_name!());
 
-    let result = ts.ucmd().arg("-d").arg("1").run();
+    let result = scene.ucmd().arg("-d1").succeeds();
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg("-d1").run();
+        if result_reference.succeeded() {
+            assert_eq!(
+                // TODO: gnu `du` doesn't use trailing "/" here
+                // result.stdout_str(), result_reference.stdout_str()
+                result.stdout_str().trim_end_matches("/\n"),
+                result_reference.stdout_str().trim_end_matches("\n")
+            );
+            return;
+        }
+    }
     _du_d_flag(result.stdout_str());
 }
 
@@ -164,9 +193,7 @@ fn _du_d_flag(s: &str) {
 
 #[test]
 fn test_du_h_flag_empty_file() {
-    let ts = TestScenario::new("du");
+    new_ucmd!()
-
-    ts.ucmd()
         .arg("-h")
         .arg("empty.txt")
         .succeeds()
@@ -176,17 +203,51 @@ fn test_du_h_flag_empty_file() {
 #[cfg(feature = "touch")]
 #[test]
 fn test_du_time() {
-    let ts = TestScenario::new("du");
+    let scene = TestScenario::new(util_name!());
 
-    let touch = ts.ccmd("touch").arg("-a").arg("-m").arg("-t").arg("201505150000").arg("date_test").run();
+    scene
-    assert!(touch.success);
+        .ccmd("touch")
+        .arg("-a")
+        .arg("-m")
+        .arg("-t")
+        .arg("201505150000")
+        .arg("date_test")
+        .succeeds();
 
-    let result = ts.ucmd().arg("--time").arg("date_test").run();
+    scene
+        .ucmd()
+        .arg("--time")
+        .arg("date_test")
+        .succeeds()
+        .stdout_only("0\t2015-05-15 00:00\tdate_test\n");
 
     // cleanup by removing test file
-    ts.cmd("rm").arg("date_test").run();
+    scene.cmd("rm").arg("date_test").succeeds(); // TODO: is this necessary?
-
+}
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+#[cfg(not(target_os = "windows"))]
-    assert_eq!(result.stdout, "0\t2015-05-15 00:00\tdate_test\n");
+#[cfg(feature = "chmod")]
+#[test]
+fn test_du_no_permission() {
+    let ts = TestScenario::new(util_name!());
+
+    let _chmod = ts.ccmd("chmod").arg("-r").arg(SUB_DIR_LINKS).succeeds();
+    let result = ts.ucmd().arg(SUB_DIR_LINKS).succeeds();
+
+    ts.ccmd("chmod").arg("+r").arg(SUB_DIR_LINKS).run();
+
+    assert_eq!(
+        result.stderr_str(),
+        "du: cannot read directory ‘subdir/links‘: Permission denied (os error 13)\n"
+    );
+    _du_no_permission(result.stdout_str());
+}
+
+#[cfg(target_vendor = "apple")]
+fn _du_no_permission(s: &str) {
+    assert_eq!(s, "0\tsubdir/links\n");
+}
+#[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))]
+fn _du_no_permission(s: &str) {
+    assert_eq!(s, "4\tsubdir/links\n");
 }

tests/by-util/test_echo.rs

@@ -2,10 +2,7 @@ use crate::common::util::*;
 
 #[test]
 fn test_default() {
-    new_ucmd!()
+    new_ucmd!().arg("hi").succeeds().stdout_only("hi\n");
-        .arg("hi")
-        .succeeds()
-        .stdout_only("hi\n");
 }
 
 #[test]

tests/by-util/test_env.rs

@@ -26,17 +26,18 @@ fn test_env_version() {
 
 #[test]
 fn test_echo() {
-    let result = new_ucmd!()
+    let result = new_ucmd!().arg("echo").arg("FOO-bar").succeeds();
-        .arg("echo")
-        .arg("FOO-bar")
-        .succeeds();
 
     assert_eq!(result.stdout_str().trim(), "FOO-bar");
 }
 
 #[test]
 fn test_file_option() {
-    let out = new_ucmd!().arg("-f").arg("vars.conf.txt").run().stdout_move_str();
+    let out = new_ucmd!()
+        .arg("-f")
+        .arg("vars.conf.txt")
+        .run()
+        .stdout_move_str();
 
     assert_eq!(
         out.lines()
@@ -89,7 +90,8 @@ fn test_multiple_name_value_pairs() {
     let out = new_ucmd!().arg("FOO=bar").arg("ABC=xyz").run();
 
     assert_eq!(
-        out.stdout_str().lines()
+        out.stdout_str()
+            .lines()
             .filter(|&line| line == "FOO=bar" || line == "ABC=xyz")
             .count(),
         2

tests/by-util/test_fold.rs

@@ -542,4 +542,4 @@ fn test_obsolete_syntax() {
         .arg("space_separated_words.txt")
         .succeeds()
         .stdout_is("test1\n \ntest2\n \ntest3\n \ntest4\n \ntest5\n \ntest6\n ");
-}
+}

tests/by-util/test_hostid.rs

@@ -4,10 +4,6 @@ use self::regex::Regex;
 
 #[test]
 fn test_normal() {
-    let (_, mut ucmd) = at_and_ucmd!();
-    let result = ucmd.run();
-
-    assert!(result.success);
     let re = Regex::new(r"^[0-9a-f]{8}").unwrap();
-    assert!(re.is_match(&result.stdout_str()));
+    new_ucmd!().succeeds().stdout_matches(&re);
 }

tests/by-util/test_hostname.rs

@@ -14,9 +14,7 @@ fn test_hostname() {
 #[cfg(not(target_vendor = "apple"))]
 #[test]
 fn test_hostname_ip() {
-    let result = new_ucmd!().arg("-i").run();
+    let result = new_ucmd!().arg("-i").succeeds();
-    println!("{:#?}", result);
-    assert!(result.success);
     assert!(!result.stdout_str().trim().is_empty());
 }
 
@@ -25,6 +23,8 @@ fn test_hostname_full() {
     let ls_short_res = new_ucmd!().arg("-s").succeeds();
     assert!(!ls_short_res.stdout_str().trim().is_empty());
 
-    new_ucmd!().arg("-f").succeeds()
+    new_ucmd!()
+        .arg("-f")
+        .succeeds()
         .stdout_contains(ls_short_res.stdout_str().trim());
 }

tests/by-util/test_id.rs

@@ -1,11 +1,32 @@
 use crate::common::util::*;
 
+// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'.
+// If we are running inside the CI and "needle" is in "stderr" skipping this test is
+// considered okay. If we are not inside the CI this calls assert!(result.success).
+//
+// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
+// stderr: "whoami: cannot find name for user ID 1001"
+// Maybe: "adduser --uid 1001 username" can put things right?
+// stderr = id: error: Could not find uid 1001: No such id: 1001
+fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool {
+    if !result.succeeded() {
+        println!("result.stdout = {}", result.stdout_str());
+        println!("result.stderr = {}", result.stderr_str());
+        if is_ci() && result.stderr_str().contains(needle) {
+            println!("test skipped:");
+            return true;
+        } else {
+            result.success();
+        }
+    }
+    false
+}
+
 fn return_whoami_username() -> String {
     let scene = TestScenario::new("whoami");
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr.contains("cannot find name for user ID") {
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
+        println!("test skipped:");
-        // As seems to be a configuration issue, ignoring it
         return String::from("");
     }
 
@@ -14,39 +35,41 @@ fn return_whoami_username() -> String {
 
 #[test]
 fn test_id() {
-    let result = new_ucmd!().arg("-u").run();
+    let scene = TestScenario::new(util_name!());
-    if result.stderr.contains("cannot find name for user ID") {
+
-        // In the CI, some server are failing to return whoami.
+    let result = scene.ucmd().arg("-u").succeeds();
-        // As seems to be a configuration issue, ignoring it
+    let uid = result.stdout_str().trim();
+
+    let result = scene.ucmd().run();
+    if skipping_test_is_okay(&result, "Could not find uid") {
         return;
     }
 
-    let uid = result.success().stdout_str().trim();
+    // Verify that the id found by --user/-u exists in the list
-    let result = new_ucmd!().run();
+    result.stdout_contains(uid);
-    if is_ci() && result.stderr.contains("cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
-        return;
-    }
-
-    if !result.stderr_str().contains("Could not find uid") {
-        // Verify that the id found by --user/-u exists in the list
-        result.success().stdout_contains(&uid);
-    }
 }
 
 #[test]
 fn test_id_from_name() {
     let username = return_whoami_username();
-    if username == "" {
+    if username.is_empty() {
-        // Sometimes, the CI is failing here
+        return;
+    }
+
+    let scene = TestScenario::new(util_name!());
+    let result = scene.ucmd().arg(&username).run();
+    if skipping_test_is_okay(&result, "Could not find uid") {
         return;
     }
 
-    let result = new_ucmd!().arg(&username).succeeds();
     let uid = result.stdout_str().trim();
 
-    new_ucmd!().succeeds()
+    let result = scene.ucmd().run();
+    if skipping_test_is_okay(&result, "Could not find uid") {
+        return;
+    }
+
+    result
         // Verify that the id found by --user/-u exists in the list
         .stdout_contains(uid)
         // Verify that the username found by whoami exists in the list
@@ -55,51 +78,42 @@ fn test_id_from_name() {
 
 #[test]
 fn test_id_name_from_id() {
-    let result = new_ucmd!().arg("-u").succeeds();
+    let result = new_ucmd!().arg("-nu").run();
-    let uid = result.stdout_str().trim();
 
-    let result = new_ucmd!().arg("-nu").arg(uid).run();
+    let username_id = result.stdout_str().trim();
-    if is_ci() && result.stderr.contains("No such user/group") {
+
-        // In the CI, some server are failing to return whoami.
+    let username_whoami = return_whoami_username();
-        // As seems to be a configuration issue, ignoring it
+    if username_whoami.is_empty() {
         return;
     }
 
-    let username_id = result
-        .success()
-        .stdout_str()
-        .trim();
-
-    let scene = TestScenario::new("whoami");
-    let result = scene.cmd("whoami").succeeds();
-
-    let username_whoami = result.stdout_str().trim();
-
     assert_eq!(username_id, username_whoami);
 }
 
 #[test]
 fn test_id_group() {
-    let mut result = new_ucmd!().arg("-g").succeeds();
+    let scene = TestScenario::new(util_name!());
+
+    let mut result = scene.ucmd().arg("-g").succeeds();
     let s1 = result.stdout_str().trim();
     assert!(s1.parse::<f64>().is_ok());
 
-    result = new_ucmd!().arg("--group").succeeds();
+    result = scene.ucmd().arg("--group").succeeds();
     let s1 = result.stdout_str().trim();
     assert!(s1.parse::<f64>().is_ok());
 }
 
 #[test]
 fn test_id_groups() {
-    let result = new_ucmd!().arg("-G").succeeds();
+    let scene = TestScenario::new(util_name!());
-    assert!(result.success);
+
+    let result = scene.ucmd().arg("-G").succeeds();
     let groups = result.stdout_str().trim().split_whitespace();
     for s in groups {
         assert!(s.parse::<f64>().is_ok());
     }
 
-    let result = new_ucmd!().arg("--groups").succeeds();
+    let result = scene.ucmd().arg("--groups").succeeds();
-    assert!(result.success);
     let groups = result.stdout_str().trim().split_whitespace();
     for s in groups {
         assert!(s.parse::<f64>().is_ok());
@@ -108,11 +122,13 @@ fn test_id_groups() {
 
 #[test]
 fn test_id_user() {
-    let mut result = new_ucmd!().arg("-u").succeeds();
+    let scene = TestScenario::new(util_name!());
+
+    let result = scene.ucmd().arg("-u").succeeds();
     let s1 = result.stdout_str().trim();
     assert!(s1.parse::<f64>().is_ok());
 
-    result = new_ucmd!().arg("--user").succeeds();
+    let result = scene.ucmd().arg("--user").succeeds();
     let s1 = result.stdout_str().trim();
     assert!(s1.parse::<f64>().is_ok());
 }
@@ -120,28 +136,34 @@ fn test_id_user() {
 #[test]
 fn test_id_pretty_print() {
     let username = return_whoami_username();
-    if username == "" {
+    if username.is_empty() {
-        // Sometimes, the CI is failing here
         return;
     }
 
-    let result = new_ucmd!().arg("-p").run();
+    let scene = TestScenario::new(util_name!());
-    if result.stdout_str().trim() == "" {
+    let result = scene.ucmd().arg("-p").run();
-        // Sometimes, the CI is failing here with
+    if result.stdout_str().trim().is_empty() {
-        // old rust versions on Linux
+        // this fails only on: "MinRustV (ubuntu-latest, feat_os_unix)"
+        // `rustc 1.40.0 (73528e339 2019-12-16)`
+        // run: /home/runner/work/coreutils/coreutils/target/debug/coreutils id -p
+        // thread 'test_id::test_id_pretty_print' panicked at 'Command was expected to succeed.
+        // stdout =
+        // stderr = ', tests/common/util.rs:157:13
+        println!("test skipped:");
         return;
     }
+
     result.success().stdout_contains(username);
 }
 
 #[test]
 fn test_id_password_style() {
     let username = return_whoami_username();
-    if username == "" {
+    if username.is_empty() {
-        // Sometimes, the CI is failing here
         return;
     }
 
     let result = new_ucmd!().arg("-P").succeeds();
+
     assert!(result.stdout_str().starts_with(&username));
 }

tests/by-util/test_install.rs

@@ -359,7 +359,7 @@ fn test_install_target_new_file_failing_nonexistent_parent() {
     ucmd.arg(file1)
         .arg(format!("{}/{}", dir, file2))
         .fails()
-        .stderr_contains(&"not a directory");
+        .stderr_contains(&"No such file or directory");
 }
 
 #[test]
@@ -443,9 +443,12 @@ fn test_install_failing_omitting_directory() {
     at.mkdir(dir2);
     at.touch(file1);
 
-    let r = ucmd.arg(dir1).arg(file1).arg(dir2).run();
+    ucmd.arg(dir1)
-    assert!(r.code == Some(1));
+        .arg(file1)
-    assert!(r.stderr.contains("omitting directory"));
+        .arg(dir2)
+        .fails()
+        .code_is(1)
+        .stderr_contains("omitting directory");
 }
 
 #[test]
@@ -458,9 +461,12 @@ fn test_install_failing_no_such_file() {
     at.mkdir(dir1);
     at.touch(file1);
 
-    let r = ucmd.arg(file1).arg(file2).arg(dir1).run();
+    ucmd.arg(file1)
-    assert!(r.code == Some(1));
+        .arg(file2)
-    assert!(r.stderr.contains("No such file or directory"));
+        .arg(dir1)
+        .fails()
+        .code_is(1)
+        .stderr_contains("No such file or directory");
 }
 
 #[test]
@@ -643,3 +649,42 @@ fn test_install_and_strip_with_non_existent_program() {
         .stderr;
     assert!(stderr.contains("No such file or directory"));
 }
+
+#[test]
+fn test_install_creating_leading_dirs() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let source = "create_leading_test_file";
+    let target = "dir1/dir2/dir3/test_file";
+
+    at.touch(source);
+
+    scene
+        .ucmd()
+        .arg("-D")
+        .arg(source)
+        .arg(at.plus(target))
+        .succeeds()
+        .no_stderr();
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_install_creating_leading_dir_fails_on_long_name() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let source = "create_leading_test_file";
+    let target = format!("{}/test_file", "d".repeat(libc::PATH_MAX as usize + 1));
+
+    at.touch(source);
+
+    scene
+        .ucmd()
+        .arg("-D")
+        .arg(source)
+        .arg(at.plus(target.as_str()))
+        .fails()
+        .stderr_contains("failed to create");
+}

tests/by-util/test_ls.rs

@@ -5,6 +5,7 @@ use crate::common::util::*;
 extern crate regex;
 use self::regex::Regex;
 
+use std::path::Path;
 use std::thread::sleep;
 use std::time::Duration;
 
@@ -1456,3 +1457,219 @@ fn test_ls_ignore_backups() {
         .stdout_does_not_contain("somebackup")
         .stdout_does_not_contain(".somehiddenbackup~");
 }
+
+#[test]
+fn test_ls_directory() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.mkdir("some_dir");
+    at.symlink_dir("some_dir", "sym_dir");
+
+    at.touch(Path::new("some_dir").join("nested_file").to_str().unwrap());
+
+    scene
+        .ucmd()
+        .arg("some_dir")
+        .succeeds()
+        .stdout_is("nested_file\n");
+
+    scene
+        .ucmd()
+        .arg("--directory")
+        .arg("some_dir")
+        .succeeds()
+        .stdout_is("some_dir\n");
+
+    scene
+        .ucmd()
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_is("nested_file\n");
+}
+
+#[test]
+fn test_ls_deref_command_line() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.touch("some_file");
+    at.symlink_file("some_file", "sym_file");
+
+    scene
+        .ucmd()
+        .arg("sym_file")
+        .succeeds()
+        .stdout_is("sym_file\n");
+
+    // -l changes the default to no dereferencing
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("sym_file")
+        .succeeds()
+        .stdout_contains("sym_file ->");
+
+    scene
+        .ucmd()
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_file")
+        .succeeds()
+        .stdout_is("sym_file\n");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_file")
+        .succeeds()
+        .stdout_contains("sym_file ->");
+
+    scene
+        .ucmd()
+        .arg("--dereference-command-line")
+        .arg("sym_file")
+        .succeeds()
+        .stdout_is("sym_file\n");
+
+    let result = scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line")
+        .arg("sym_file")
+        .succeeds();
+
+    assert!(!result.stdout_str().contains("->"));
+
+    let result = scene.ucmd().arg("-lH").arg("sym_file").succeeds();
+
+    assert!(!result.stdout_str().contains("sym_file ->"));
+
+    // If the symlink is not a command line argument, it must be shown normally
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line")
+        .succeeds()
+        .stdout_contains("sym_file ->");
+}
+
+#[test]
+fn test_ls_deref_command_line_dir() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.mkdir("some_dir");
+    at.symlink_dir("some_dir", "sym_dir");
+
+    at.touch(Path::new("some_dir").join("nested_file").to_str().unwrap());
+
+    scene
+        .ucmd()
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    scene
+        .ucmd()
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("--dereference-command-line")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("-lH")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    // If the symlink is not a command line argument, it must be shown normally
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    scene
+        .ucmd()
+        .arg("-lH")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    // --directory does not dereference anything by default
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--directory")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    let result = scene
+        .ucmd()
+        .arg("-l")
+        .arg("--directory")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_dir")
+        .succeeds();
+
+    assert!(!result.stdout_str().ends_with("sym_dir"));
+
+    // --classify does not dereference anything by default
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--directory")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    let result = scene
+        .ucmd()
+        .arg("-l")
+        .arg("--directory")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_dir")
+        .succeeds();
+
+    assert!(!result.stdout_str().ends_with("sym_dir"));
+}

tests/by-util/test_mktemp.rs

@@ -113,17 +113,14 @@ fn test_mktemp_mktemp_t() {
         .arg("-t")
         .arg(TEST_TEMPLATE7)
         .succeeds();
-    let result = scene
+    scene
         .ucmd()
         .env(TMPDIR, &pathname)
         .arg("-t")
         .arg(TEST_TEMPLATE8)
-        .fails();
+        .fails()
-    println!("stdout {}", result.stdout);
+        .no_stdout()
-    println!("stderr {}", result.stderr);
+        .stderr_contains("error: suffix cannot contain any path separators");
-    assert!(result
-        .stderr
-        .contains("error: suffix cannot contain any path separators"));
 }
 
 #[test]
@@ -391,10 +388,8 @@ fn test_mktemp_tmpdir_one_arg() {
         .arg("--tmpdir")
         .arg("apt-key-gpghome.XXXXXXXXXX")
         .succeeds();
-    println!("stdout {}", result.stdout);
+    result.no_stderr().stdout_contains("apt-key-gpghome.");
-    println!("stderr {}", result.stderr);
+    assert!(PathBuf::from(result.stdout_str().trim()).is_file());
-    assert!(result.stdout.contains("apt-key-gpghome."));
-    assert!(PathBuf::from(result.stdout.trim()).is_file());
 }
 
 #[test]
@@ -407,8 +402,6 @@ fn test_mktemp_directory_tmpdir() {
         .arg("--tmpdir")
         .arg("apt-key-gpghome.XXXXXXXXXX")
         .succeeds();
-    println!("stdout {}", result.stdout);
+    result.no_stderr().stdout_contains("apt-key-gpghome.");
-    println!("stderr {}", result.stderr);
+    assert!(PathBuf::from(result.stdout_str().trim()).is_dir());
-    assert!(result.stdout.contains("apt-key-gpghome."));
-    assert!(PathBuf::from(result.stdout.trim()).is_dir());
 }

tests/by-util/test_nproc.rs

@@ -2,54 +2,46 @@ use crate::common::util::*;
 
 #[test]
 fn test_nproc() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let nproc: u8 = new_ucmd!().succeeds().stdout_str().trim().parse().unwrap();
-    let result = ucmd.run();
-    assert!(result.success);
-    let nproc: u8 = result.stdout.trim().parse().unwrap();
     assert!(nproc > 0);
 }
 
 #[test]
 fn test_nproc_all_omp() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().arg("--all").succeeds();
-    let result = ucmd.arg("--all").run();
+
-    assert!(result.success);
+    let nproc: u8 = result.stdout_str().trim().parse().unwrap();
-    let nproc: u8 = result.stdout.trim().parse().unwrap();
     assert!(nproc > 0);
 
     let result = TestScenario::new(util_name!())
         .ucmd_keepenv()
         .env("OMP_NUM_THREADS", "1")
-        .run();
+        .succeeds();
-    assert!(result.success);
+
-    let nproc_omp: u8 = result.stdout.trim().parse().unwrap();
+    let nproc_omp: u8 = result.stdout_str().trim().parse().unwrap();
     assert!(nproc - 1 == nproc_omp);
 
     let result = TestScenario::new(util_name!())
         .ucmd_keepenv()
         .env("OMP_NUM_THREADS", "1") // Has no effect
         .arg("--all")
-        .run();
+        .succeeds();
-    assert!(result.success);
+    let nproc_omp: u8 = result.stdout_str().trim().parse().unwrap();
-    let nproc_omp: u8 = result.stdout.trim().parse().unwrap();
     assert!(nproc == nproc_omp);
 }
 
 #[test]
 fn test_nproc_ignore() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().succeeds();
-    let result = ucmd.run();
+    let nproc: u8 = result.stdout_str().trim().parse().unwrap();
-    assert!(result.success);
-    let nproc: u8 = result.stdout.trim().parse().unwrap();
     if nproc > 1 {
         // Ignore all CPU but one
         let result = TestScenario::new(util_name!())
             .ucmd_keepenv()
             .arg("--ignore")
             .arg((nproc - 1).to_string())
-            .run();
+            .succeeds();
-        assert!(result.success);
+        let nproc: u8 = result.stdout_str().trim().parse().unwrap();
-        let nproc: u8 = result.stdout.trim().parse().unwrap();
         assert!(nproc == 1);
     }
 }

tests/by-util/test_pinky.rs

@@ -43,11 +43,9 @@ fn test_short_format_i() {
     let actual = TestScenario::new(util_name!())
         .ucmd()
         .args(&args)
-        .run()
+        .succeeds()
-        .stdout;
+        .stdout_move_str();
     let expect = expected_result(&args);
-    println!("actual: {:?}", actual);
-    println!("expect: {:?}", expect);
     let v_actual: Vec<&str> = actual.split_whitespace().collect();
     let v_expect: Vec<&str> = expect.split_whitespace().collect();
     assert_eq!(v_actual, v_expect);
@@ -62,11 +60,9 @@ fn test_short_format_q() {
     let actual = TestScenario::new(util_name!())
         .ucmd()
         .args(&args)
-        .run()
+        .succeeds()
-        .stdout;
+        .stdout_move_str();
     let expect = expected_result(&args);
-    println!("actual: {:?}", actual);
-    println!("expect: {:?}", expect);
     let v_actual: Vec<&str> = actual.split_whitespace().collect();
     let v_expect: Vec<&str> = expect.split_whitespace().collect();
     assert_eq!(v_actual, v_expect);
@@ -79,5 +75,5 @@ fn expected_result(args: &[&str]) -> String {
         .env("LANGUAGE", "C")
         .args(args)
         .run()
-        .stdout
+        .stdout_move_str()
 }

tests/by-util/test_printenv.rs

@@ -7,10 +7,11 @@ fn test_get_all() {
     env::set_var(key, "VALUE");
     assert_eq!(env::var(key), Ok("VALUE".to_string()));
 
-    let result = TestScenario::new(util_name!()).ucmd_keepenv().run();
+    TestScenario::new(util_name!())
-    assert!(result.success);
+        .ucmd_keepenv()
-    assert!(result.stdout.contains("HOME="));
+        .succeeds()
-    assert!(result.stdout.contains("KEY=VALUE"));
+        .stdout_contains("HOME=")
+        .stdout_contains("KEY=VALUE");
 }
 
 #[test]
@@ -22,9 +23,8 @@ fn test_get_var() {
     let result = TestScenario::new(util_name!())
         .ucmd_keepenv()
         .arg("KEY")
-        .run();
+        .succeeds();
 
-    assert!(result.success);
+    assert!(!result.stdout_str().is_empty());
-    assert!(!result.stdout.is_empty());
+    assert!(result.stdout_str().trim() == "VALUE");
-    assert!(result.stdout.trim() == "VALUE");
 }

tests/by-util/test_readlink.rs

@@ -5,7 +5,7 @@ static GIBBERISH: &'static str = "supercalifragilisticexpialidocious";
 #[test]
 fn test_canonicalize() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let actual = ucmd.arg("-f").arg(".").run().stdout;
+    let actual = ucmd.arg("-f").arg(".").run().stdout_move_str();
     let expect = at.root_dir_resolved() + "\n";
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -15,7 +15,7 @@ fn test_canonicalize() {
 #[test]
 fn test_canonicalize_existing() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let actual = ucmd.arg("-e").arg(".").run().stdout;
+    let actual = ucmd.arg("-e").arg(".").run().stdout_move_str();
     let expect = at.root_dir_resolved() + "\n";
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -25,7 +25,7 @@ fn test_canonicalize_existing() {
 #[test]
 fn test_canonicalize_missing() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let actual = ucmd.arg("-m").arg(GIBBERISH).run().stdout;
+    let actual = ucmd.arg("-m").arg(GIBBERISH).run().stdout_move_str();
     let expect = path_concat!(at.root_dir_resolved(), GIBBERISH) + "\n";
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -37,7 +37,7 @@ fn test_long_redirection_to_current_dir() {
     let (at, mut ucmd) = at_and_ucmd!();
     // Create a 256-character path to current directory
     let dir = path_concat!(".", ..128);
-    let actual = ucmd.arg("-n").arg("-m").arg(dir).run().stdout;
+    let actual = ucmd.arg("-n").arg("-m").arg(dir).run().stdout_move_str();
     let expect = at.root_dir_resolved();
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -48,7 +48,12 @@ fn test_long_redirection_to_current_dir() {
 fn test_long_redirection_to_root() {
     // Create a 255-character path to root
     let dir = path_concat!("..", ..85);
-    let actual = new_ucmd!().arg("-n").arg("-m").arg(dir).run().stdout;
+    let actual = new_ucmd!()
+        .arg("-n")
+        .arg("-m")
+        .arg(dir)
+        .run()
+        .stdout_move_str();
     let expect = get_root_path();
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);

tests/by-util/test_shred.rs

@@ -36,9 +36,7 @@ fn test_shred_force() {
     at.set_readonly(file);
 
     // Try shred -u.
-    let result = scene.ucmd().arg("-u").arg(file).run();
+    scene.ucmd().arg("-u").arg(file).run();
-    println!("stderr = {:?}", result.stderr);
-    println!("stdout = {:?}", result.stdout);
 
     // file_a was not deleted because it is readonly.
     assert!(at.file_exists(file));

tests/by-util/test_sort.rs

@@ -8,6 +8,25 @@ fn test_helper(file_name: &str, args: &str) {
         .stdout_is_fixture(format!("{}.expected", file_name));
 }
 
+#[test]
+fn test_months_whitespace() {
+    test_helper("months-whitespace", "-M");
+}
+
+#[test]
+fn test_version_empty_lines() {
+    new_ucmd!()
+        .arg("-V")
+        .arg("version-empty-lines.txt")
+        .succeeds()
+        .stdout_is("\n\n\n\n\n\n\n1.2.3-alpha\n1.2.3-alpha2\n\t\t\t1.12.4\n11.2.3\n");
+}
+
+#[test]
+fn test_human_numeric_whitespace() {
+    test_helper("human-numeric-whitespace", "-h");
+}
+
 #[test]
 fn test_multiple_decimals_general() {
     new_ucmd!()
@@ -19,11 +38,7 @@ fn test_multiple_decimals_general() {
 
 #[test]
 fn test_multiple_decimals_numeric() {
-    new_ucmd!()
+    test_helper("multiple_decimals_numeric", "-n")
-        .arg("-n")
-        .arg("multiple_decimals_numeric.txt")
-        .succeeds()
-        .stdout_is("-2028789030\n-896689\n-8.90880\n-1\n-.05\n\n\n\n\n\n\n\n\n000\nCARAvan\n00000001\n1\n1.040000000\n1.444\n1.58590\n8.013\n45\n46.89\n               4567.\n4567.1\n4567.34\n\t\t\t\t\t\t\t\t\t\t4567..457\n\t\t\t\t37800\n\t\t\t\t\t\t45670.89079.098\n\t\t\t\t\t\t45670.89079.1\n576,446.88800000\n576,446.890\n4798908.340000000000\n4798908.45\n4798908.8909800\n");
 }
 
 #[test]
@@ -50,7 +65,7 @@ fn test_random_shuffle_len() {
     // check whether output is the same length as the input
     const FILE: &'static str = "default_unsorted_ints.expected";
     let (at, _ucmd) = at_and_ucmd!();
-    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
     let expected = at.read(FILE);
 
     assert_ne!(result, expected);
@@ -62,9 +77,9 @@ fn test_random_shuffle_contains_all_lines() {
     // check whether lines of input are all in output
     const FILE: &'static str = "default_unsorted_ints.expected";
     let (at, _ucmd) = at_and_ucmd!();
-    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
     let expected = at.read(FILE);
-    let result_sorted = new_ucmd!().pipe_in(result.clone()).run().stdout;
+    let result_sorted = new_ucmd!().pipe_in(result.clone()).run().stdout_move_str();
 
     assert_ne!(result, expected);
     assert_eq!(result_sorted, expected);
@@ -77,9 +92,9 @@ fn test_random_shuffle_two_runs_not_the_same() {
     // as the starting order, or if both random sorts end up having the same order.
     const FILE: &'static str = "default_unsorted_ints.expected";
     let (at, _ucmd) = at_and_ucmd!();
-    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
     let expected = at.read(FILE);
-    let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
 
     assert_ne!(result, expected);
     assert_ne!(result, unexpected);
@@ -92,9 +107,9 @@ fn test_random_shuffle_contains_two_runs_not_the_same() {
     // as the starting order, or if both random sorts end up having the same order.
     const FILE: &'static str = "default_unsorted_ints.expected";
     let (at, _ucmd) = at_and_ucmd!();
-    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
     let expected = at.read(FILE);
-    let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
 
     assert_ne!(result, expected);
     assert_ne!(result, unexpected);

tests/by-util/test_stat.rs

@@ -194,7 +194,7 @@ fn test_terse_normal_format() {
     // note: contains birth/creation date which increases test fragility
     // * results may vary due to built-in `stat` limitations as well as linux kernel and rust version capability variations
     let args = ["-t", "/"];
-    let actual = new_ucmd!().args(&args).run().stdout;
+    let actual = new_ucmd!().args(&args).succeeds().stdout_move_str();
     let expect = expected_result(&args);
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -216,7 +216,7 @@ fn test_terse_normal_format() {
 #[cfg(target_os = "linux")]
 fn test_format_created_time() {
     let args = ["-c", "%w", "/boot"];
-    let actual = new_ucmd!().args(&args).run().stdout;
+    let actual = new_ucmd!().args(&args).succeeds().stdout_move_str();
     let expect = expected_result(&args);
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -240,7 +240,7 @@ fn test_format_created_time() {
 #[cfg(target_os = "linux")]
 fn test_format_created_seconds() {
     let args = ["-c", "%W", "/boot"];
-    let actual = new_ucmd!().args(&args).run().stdout;
+    let actual = new_ucmd!().args(&args).succeeds().stdout_move_str();
     let expect = expected_result(&args);
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);

tests/by-util/test_stdbuf.rs

@@ -25,19 +25,15 @@ fn test_stdbuf_line_buffered_stdout() {
 #[cfg(not(target_os = "windows"))]
 #[test]
 fn test_stdbuf_no_buffer_option_fails() {
-    new_ucmd!()
+    new_ucmd!().args(&["head"]).fails().stderr_is(
-        .args(&["head"])
+        "error: The following required arguments were not provided:\n    \
-        .pipe_in("The quick brown fox jumps over the lazy dog.")
-        .fails()
-        .stderr_is(
-            "error: The following required arguments were not provided:\n    \
             --error <MODE>\n    \
             --input <MODE>\n    \
             --output <MODE>\n\n\
             USAGE:\n    \
             stdbuf OPTION... COMMAND\n\n\
             For more information try --help",
-        );
+    );
 }
 
 #[cfg(not(target_os = "windows"))]
@@ -55,7 +51,6 @@ fn test_stdbuf_trailing_var_arg() {
 fn test_stdbuf_line_buffering_stdin_fails() {
     new_ucmd!()
         .args(&["-i", "L", "head"])
-        .pipe_in("The quick brown fox jumps over the lazy dog.")
         .fails()
         .stderr_is("stdbuf: error: line buffering stdin is meaningless\nTry 'stdbuf --help' for more information.");
 }
@@ -65,7 +60,6 @@ fn test_stdbuf_line_buffering_stdin_fails() {
 fn test_stdbuf_invalid_mode_fails() {
     new_ucmd!()
         .args(&["-i", "1024R", "head"])
-        .pipe_in("The quick brown fox jumps over the lazy dog.")
         .fails()
         .stderr_is("stdbuf: error: invalid mode 1024R\nTry 'stdbuf --help' for more information.");
 }

tests/by-util/test_sync.rs

@@ -5,8 +5,7 @@ use tempfile::tempdir;
 
 #[test]
 fn test_sync_default() {
-    let result = new_ucmd!().run();
+    new_ucmd!().succeeds();
-    assert!(result.success);
 }
 
 #[test]
@@ -18,8 +17,10 @@ fn test_sync_incorrect_arg() {
 fn test_sync_fs() {
     let temporary_directory = tempdir().unwrap();
     let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap();
-    let result = new_ucmd!().arg("--file-system").arg(&temporary_path).run();
+    new_ucmd!()
-    assert!(result.success);
+        .arg("--file-system")
+        .arg(&temporary_path)
+        .succeeds();
 }
 
 #[test]
@@ -27,12 +28,14 @@ fn test_sync_data() {
     // Todo add a second arg
     let temporary_directory = tempdir().unwrap();
     let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap();
-    let result = new_ucmd!().arg("--data").arg(&temporary_path).run();
+    new_ucmd!().arg("--data").arg(&temporary_path).succeeds();
-    assert!(result.success);
 }
 
 #[test]
 fn test_sync_no_existing_files() {
-    let result = new_ucmd!().arg("--data").arg("do-no-exist").fails();
+    new_ucmd!()
-    assert!(result.stderr.contains("error: cannot stat"));
+        .arg("--data")
+        .arg("do-no-exist")
+        .fails()
+        .stderr_contains("error: cannot stat");
 }

tests/by-util/test_tail.rs

@@ -226,8 +226,8 @@ fn test_bytes_big() {
         .arg(FILE)
         .arg("-c")
         .arg(format!("{}", N_ARG))
-        .run()
+        .succeeds()
-        .stdout;
+        .stdout_move_str();
     let expected = at.read(EXPECTED_FILE);
 
     assert_eq!(result.len(), expected.len());
@@ -340,6 +340,6 @@ fn test_negative_indexing() {
 
     let negative_bytes_index = new_ucmd!().arg("-c").arg("-20").arg(FOOBAR_TXT).run();
 
-    assert_eq!(positive_lines_index.stdout, negative_lines_index.stdout);
+    assert_eq!(positive_lines_index.stdout(), negative_lines_index.stdout());
-    assert_eq!(positive_bytes_index.stdout, negative_bytes_index.stdout);
+    assert_eq!(positive_bytes_index.stdout(), negative_bytes_index.stdout());
 }

tests/by-util/test_touch.rs

@@ -29,6 +29,7 @@ fn set_file_times(at: &AtPath, path: &str, atime: FileTime, mtime: FileTime) {
 fn str_to_filetime(format: &str, s: &str) -> FileTime {
     let mut tm = time::strptime(s, format).unwrap();
     tm.tm_utcoff = time::now().tm_utcoff;
+    tm.tm_isdst = -1; // Unknown flag DST
     let ts = tm.to_timespec();
     FileTime::from_unix_time(ts.sec as i64, ts.nsec as u32)
 }
@@ -352,3 +353,72 @@ fn test_touch_set_date() {
     assert_eq!(atime, start_of_year);
     assert_eq!(mtime, start_of_year);
 }
+
+#[test]
+fn test_touch_mtime_dst_succeeds() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let file = "test_touch_set_mtime_dst_succeeds";
+
+    ucmd.args(&["-m", "-t", "202103140300", file])
+        .succeeds()
+        .no_stderr();
+
+    assert!(at.file_exists(file));
+
+    let target_time = str_to_filetime("%Y%m%d%H%M", "202103140300");
+    let (_, mtime) = get_file_times(&at, file);
+    assert!(target_time == mtime);
+}
+
+// is_dst_switch_hour returns true if timespec ts is just before the switch
+// to Daylight Saving Time.
+// For example, in EST (UTC-5), Timespec { sec: 1583647200, nsec: 0 }
+// for March 8 2020 01:00:00 AM
+// is just before the switch because on that day clock jumps by 1 hour,
+// so 1 minute after 01:59:00 is 03:00:00.
+fn is_dst_switch_hour(ts: time::Timespec) -> bool {
+    let ts_after = ts + time::Duration::hours(1);
+    let tm = time::at(ts);
+    let tm_after = time::at(ts_after);
+    tm_after.tm_hour == tm.tm_hour + 2
+}
+
+// get_dstswitch_hour returns date string for which touch -m -t fails.
+// For example, in EST (UTC-5), that will be "202003080200" so
+// touch -m -t 202003080200 somefile
+// fails (that date/time does not exist).
+// In other locales it will be a different date/time, and in some locales
+// it doesn't exist at all, in which case this function will return None.
+fn get_dstswitch_hour() -> Option<String> {
+    let now = time::now();
+    // Start from January 1, 2020, 00:00.
+    let mut tm = time::strptime("20200101-0000", "%Y%m%d-%H%M").unwrap();
+    tm.tm_isdst = -1;
+    tm.tm_utcoff = now.tm_utcoff;
+    let mut ts = tm.to_timespec();
+    // Loop through all hours in year 2020 until we find the hour just
+    // before the switch to DST.
+    for _i in 0..(366 * 24) {
+        if is_dst_switch_hour(ts) {
+            let mut tm = time::at(ts);
+            tm.tm_hour = tm.tm_hour + 1;
+            let s = time::strftime("%Y%m%d%H%M", &tm).unwrap().to_string();
+            return Some(s);
+        }
+        ts = ts + time::Duration::hours(1);
+    }
+    None
+}
+
+#[test]
+fn test_touch_mtime_dst_fails() {
+    let (_at, mut ucmd) = at_and_ucmd!();
+    let file = "test_touch_set_mtime_dst_fails";
+
+    match get_dstswitch_hour() {
+        Some(s) => {
+            ucmd.args(&["-m", "-t", &s, file]).fails();
+        }
+        None => (),
+    }
+}

tests/by-util/test_tsort.rs

@@ -28,7 +28,7 @@ fn test_version_flag() {
     let version_short = new_ucmd!().arg("-V").run();
     let version_long = new_ucmd!().arg("--version").run();
 
-    assert_eq!(version_short.stdout, version_long.stdout);
+    assert_eq!(version_short.stdout(), version_long.stdout());
 }
 
 #[test]
@@ -36,7 +36,7 @@ fn test_help_flag() {
     let help_short = new_ucmd!().arg("-h").run();
     let help_long = new_ucmd!().arg("--help").run();
 
-    assert_eq!(help_short.stdout, help_long.stdout);
+    assert_eq!(help_short.stdout(), help_long.stdout());
 }
 
 #[test]

tests/by-util/test_uname.rs

@@ -2,60 +2,41 @@ use crate::common::util::*;
 
 #[test]
 fn test_uname_compatible() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-a").succeeds();
-
-    let result = ucmd.arg("-a").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_uname_name() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-n").succeeds();
-
-    let result = ucmd.arg("-n").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_uname_processor() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().arg("-p").succeeds();
-
+    assert_eq!(result.stdout_str().trim_end(), "unknown");
-    let result = ucmd.arg("-p").run();
-    assert!(result.success);
-    assert_eq!(result.stdout.trim_end(), "unknown");
 }
 
 #[test]
 fn test_uname_hwplatform() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().arg("-i").succeeds();
-
+    assert_eq!(result.stdout_str().trim_end(), "unknown");
-    let result = ucmd.arg("-i").run();
-    assert!(result.success);
-    assert_eq!(result.stdout.trim_end(), "unknown");
 }
 
 #[test]
 fn test_uname_machine() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-m").succeeds();
-
-    let result = ucmd.arg("-m").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_uname_kernel_version() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-v").succeeds();
-
-    let result = ucmd.arg("-v").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_uname_kernel() {
     let (_, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd.arg("-o").run();
+    let result = ucmd.arg("-o").succeeds();
-    assert!(result.success);
     #[cfg(target_os = "linux")]
-    assert!(result.stdout.to_lowercase().contains("linux"));
+    assert!(result.stdout_str().to_lowercase().contains("linux"));
 }

tests/by-util/test_uptime.rs

@@ -4,33 +4,23 @@ use crate::common::util::*;
 
 #[test]
 fn test_uptime() {
-    let result = TestScenario::new(util_name!()).ucmd_keepenv().run();
+    TestScenario::new(util_name!())
+        .ucmd_keepenv()
+        .succeeds()
+        .stdout_contains("load average:")
+        .stdout_contains(" up ");
 
-    println!("stdout = {}", result.stdout);
-    println!("stderr = {}", result.stderr);
-
-    assert!(result.success);
-    assert!(result.stdout.contains("load average:"));
-    assert!(result.stdout.contains(" up "));
     // Don't check for users as it doesn't show in some CI
 }
 
 #[test]
 fn test_uptime_since() {
-    let scene = TestScenario::new(util_name!());
-
-    let result = scene.ucmd().arg("--since").succeeds();
-
-    println!("stdout = {}", result.stdout);
-    println!("stderr = {}", result.stderr);
-
-    assert!(result.success);
     let re = Regex::new(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}").unwrap();
-    assert!(re.is_match(&result.stdout.trim()));
+
+    new_ucmd!().arg("--since").succeeds().stdout_matches(&re);
 }
 
 #[test]
 fn test_failed() {
-    let (_at, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("willfail").fails();
-    ucmd.arg("willfail").fails();
 }

tests/by-util/test_users.rs

@@ -3,14 +3,11 @@ use std::env;
 
 #[test]
 fn test_users_noarg() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().succeeds();
-    let result = ucmd.run();
-    assert!(result.success);
 }
 #[test]
 fn test_users_check_name() {
-    let result = TestScenario::new(util_name!()).ucmd_keepenv().run();
+    let result = TestScenario::new(util_name!()).ucmd_keepenv().succeeds();
-    assert!(result.success);
 
     // Expectation: USER is often set
     let key = "USER";
@@ -21,9 +18,9 @@ fn test_users_check_name() {
         // Check if "users" contains the name of the user
         {
             println!("username found {}", &username);
-            println!("result.stdout {}", &result.stdout);
+            // println!("result.stdout {}", &result.stdout);
-            if !&result.stdout.is_empty() {
+            if !result.stdout_str().is_empty() {
-                assert!(result.stdout.contains(&username))
+                result.stdout_contains(&username);
             }
         }
     }

tests/by-util/test_whoami.rs

@@ -1,50 +1,63 @@
 use crate::common::util::*;
-use std::env;
+
+// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'.
+// If we are running inside the CI and "needle" is in "stderr" skipping this test is
+// considered okay. If we are not inside the CI this calls assert!(result.success).
+//
+// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
+// stderr: "whoami: error: failed to get username"
+// Maybe: "adduser --uid 1001 username" can put things right?
+fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool {
+    if !result.succeeded() {
+        println!("result.stdout = {}", result.stdout_str());
+        println!("result.stderr = {}", result.stderr_str());
+        if is_ci() && result.stderr_str().contains(needle) {
+            println!("test skipped:");
+            return true;
+        } else {
+            result.success();
+        }
+    }
+    false
+}
 
 #[test]
 fn test_normal() {
     let (_, mut ucmd) = at_and_ucmd!();
 
     let result = ucmd.run();
-    println!("result.stdout = {}", result.stdout);
-    println!("result.stderr = {}", result.stderr);
-    println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok());
 
-    for (key, value) in env::vars() {
+    // use std::env;
-        println!("{}: {}", key, value);
+    // println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok());
-    }
+    // for (key, value) in env::vars() {
-    if is_ci() && result.stderr.contains("failed to get username") {
+    //     println!("{}: {}", key, value);
-        // In the CI, some server are failing to return whoami.
+    // }
-        // As seems to be a configuration issue, ignoring it
+
+    if skipping_test_is_okay(&result, "failed to get username") {
         return;
     }
 
-    assert!(result.success);
+    result.no_stderr();
-    assert!(!result.stdout.trim().is_empty());
+    assert!(!result.stdout_str().trim().is_empty());
 }
 
 #[test]
 #[cfg(not(windows))]
 fn test_normal_compare_id() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let scene = TestScenario::new(util_name!());
 
-    let result = ucmd.run();
+    let result_ucmd = scene.ucmd().run();
-
+    if skipping_test_is_okay(&result_ucmd, "failed to get username") {
-    println!("result.stdout = {}", result.stdout);
-    println!("result.stderr = {}", result.stderr);
-    if is_ci() && result.stderr.contains("failed to get username") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    assert!(result.success);
-    let ts = TestScenario::new("id");
-    let id = ts.cmd("id").arg("-un").run();
 
-    if is_ci() && id.stderr.contains("cannot find name for user ID") {
+    let result_cmd = scene.cmd("id").arg("-un").run();
-        // In the CI, some server are failing to return whoami.
+    if skipping_test_is_okay(&result_cmd, "cannot find name for user ID") {
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    assert_eq!(result.stdout.trim(), id.stdout.trim());
+
+    assert_eq!(
+        result_ucmd.stdout_str().trim(),
+        result_cmd.stdout_str().trim()
+    );
 }

tests/common/util.rs

@@ -33,6 +33,8 @@ static ALREADY_RUN: &str = " you have already run this UCommand, if you want to
                             testing();";
 static MULTIPLE_STDIN_MEANINGLESS: &str = "Ucommand is designed around a typical use case of: provide args and input stream -> spawn process -> block until completion -> return output streams. For verifying that a particular section of the input stream is what causes a particular behavior, use the Command type directly.";
 
+static NO_STDIN_MEANINGLESS: &str = "Setting this flag has no effect if there is no stdin";
+
 /// Test if the program is running under CI
 pub fn is_ci() -> bool {
     std::env::var("CI")
@@ -69,7 +71,7 @@ pub struct CmdResult {
     //tmpd is used for convenience functions for asserts against fixtures
     tmpd: Option<Rc<TempDir>>,
     /// exit status for command (if there is one)
-    pub code: Option<i32>,
+    code: Option<i32>,
     /// zero-exit from running the Command?
     /// see [`success`]
     pub success: bool,
@@ -218,6 +220,13 @@ impl CmdResult {
         self
     }
 
+    /// Like `stdout_is` but newlines are normalized to `\n`.
+    pub fn normalized_newlines_stdout_is<T: AsRef<str>>(&self, msg: T) -> &CmdResult {
+        let msg = msg.as_ref().replace("\r\n", "\n");
+        assert_eq!(self.stdout.replace("\r\n", "\n"), msg);
+        self
+    }
+
     /// asserts that the command resulted in stdout stream output,
     /// whose bytes equal those of the passed in slice
     pub fn stdout_is_bytes<T: AsRef<[u8]>>(&self, msg: T) -> &CmdResult {
@@ -688,6 +697,7 @@ pub struct UCommand {
     tmpd: Option<Rc<TempDir>>,
     has_run: bool,
     stdin: Option<Vec<u8>>,
+    ignore_stdin_write_error: bool,
 }
 
 impl UCommand {
@@ -717,6 +727,7 @@ impl UCommand {
             },
             comm_string: String::from(arg.as_ref().to_str().unwrap()),
             stdin: None,
+            ignore_stdin_write_error: false,
         }
     }
 
@@ -769,6 +780,17 @@ impl UCommand {
         self.pipe_in(contents)
     }
 
+    /// Ignores error caused by feeding stdin to the command.
+    /// This is typically useful to test non-standard workflows
+    /// like feeding something to a command that does not read it
+    pub fn ignore_stdin_write_error(&mut self) -> &mut UCommand {
+        if self.stdin.is_none() {
+            panic!("{}", NO_STDIN_MEANINGLESS);
+        }
+        self.ignore_stdin_write_error = true;
+        self
+    }
+
     pub fn env<K, V>(&mut self, key: K, val: V) -> &mut UCommand
     where
         K: AsRef<OsStr>,
@@ -789,7 +811,7 @@ impl UCommand {
         }
         self.has_run = true;
         log_info("run", &self.comm_string);
-        let mut result = self
+        let mut child = self
             .raw
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
@@ -798,15 +820,19 @@ impl UCommand {
             .unwrap();
 
         if let Some(ref input) = self.stdin {
-            result
+            let write_result = child
                 .stdin
                 .take()
                 .unwrap_or_else(|| panic!("Could not take child process stdin"))
-                .write_all(input)
+                .write_all(input);
-                .unwrap_or_else(|e| panic!("{}", e));
+            if !self.ignore_stdin_write_error {
+                if let Err(e) = write_result {
+                    panic!("failed to write to stdin of child: {}", e)
+                }
+            }
         }
 
-        result
+        child
     }
 
     /// Spawns the command, feeds the stdin if any, waits for the result
@@ -1077,4 +1103,33 @@ mod tests {
 
         res.stdout_does_not_match(&positive);
     }
+
+    #[test]
+    fn test_normalized_newlines_stdout_is() {
+        let res = CmdResult {
+            tmpd: None,
+            code: None,
+            success: true,
+            stdout: "A\r\nB\nC".into(),
+            stderr: "".into(),
+        };
+
+        res.normalized_newlines_stdout_is("A\r\nB\nC");
+        res.normalized_newlines_stdout_is("A\nB\nC");
+        res.normalized_newlines_stdout_is("A\nB\r\nC");
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_normalized_newlines_stdout_is_fail() {
+        let res = CmdResult {
+            tmpd: None,
+            code: None,
+            success: true,
+            stdout: "A\r\nB\nC".into(),
+            stderr: "".into(),
+        };
+
+        res.normalized_newlines_stdout_is("A\r\nB\nC\n");
+    }
 }

tests/fixtures/sort/human-numeric-whitespace.expected

@@ -0,0 +1,11 @@
+
+
+
+
+
+
+
+456K
+4568K
+			456M
+              6.2G

tests/fixtures/sort/human-numeric-whitespace.txt

@@ -0,0 +1,11 @@
+
+
+456K
+
+			456M
+
+
+4568K
+
+              6.2G
+

tests/fixtures/sort/months-whitespace.expected

@@ -0,0 +1,8 @@
+
+
+JAN
+  FEb
+ apr
+ apr
+			JUNNNN
+AUG 

tests/fixtures/sort/months-whitespace.txt

@@ -0,0 +1,8 @@
+JAN
+			JUNNNN
+AUG 
+
+ apr
+ apr
+
+  FEb

tests/fixtures/sort/multiple_decimals_numeric.expected

@@ -0,0 +1,35 @@
+-2028789030
+-896689
+-8.90880
+-1
+-.05
+
+
+
+
+
+
+
+
+000
+CARAvan
+00000001
+1
+1.040000000
+1.444
+1.58590
+8.013
+45
+46.89
+										4567..457
+               4567.
+4567.1
+4567.34
+				37800
+						45670.89079.098
+						45670.89079.1
+576,446.88800000
+576,446.890
+4798908.340000000000
+4798908.45
+4798908.8909800

tests/fixtures/sort/version-empty-lines.expected

@@ -0,0 +1,11 @@
+
+
+
+
+
+
+
+1.2.3-alpha
+1.2.3-alpha2
+11.2.3
+			1.12.4

tests/fixtures/sort/version-empty-lines.txt

@@ -0,0 +1,11 @@
+11.2.3
+
+
+
+1.2.3-alpha2
+
+
+1.2.3-alpha
+
+
+			1.12.4