From 2b18e45ecec277f8511299d8294eb3f48cb19faf Mon Sep 17 00:00:00 2001
From: Andreas Hartmann <andreas.h@rtmann.de>
Date: Tue, 13 Jul 2021 15:57:07 +0200
Subject: [PATCH 01/29] install: Use UResult

Related to uutils#2464
---
 src/uu/install/src/install.rs | 233 ++++++++++++++++++++--------------
 1 file changed, 138 insertions(+), 95 deletions(-)

diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs
index 81d44bb6c..6000837cb 100644
--- a/src/uu/install/src/install.rs
+++ b/src/uu/install/src/install.rs
@@ -5,7 +5,7 @@
 //  * For the full copyright and license information, please view the LICENSE file
 //  * that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) rwxr sourcepath targetpath
+// spell-checker:ignore (ToDO) rwxr sourcepath targetpath Isnt
 
 mod mode;
 
@@ -17,15 +17,17 @@ use file_diff::diff;
 use filetime::{set_file_times, FileTime};
 use uucore::backup_control::{self, BackupMode};
 use uucore::entries::{grp2gid, usr2uid};
+use uucore::error::{FromIo, UCustomError, UIoError, UResult, USimpleError};
 use uucore::perms::{wrap_chgrp, wrap_chown, Verbosity};
 
 use libc::{getegid, geteuid};
+use std::error::Error;
+use std::fmt::{Debug, Display};
 use std::fs;
 use std::fs::File;
 use std::os::unix::fs::MetadataExt;
 use std::path::{Path, PathBuf};
 use std::process::Command;
-use std::result::Result;
 
 const DEFAULT_MODE: u32 = 0o755;
 const DEFAULT_STRIP_PROGRAM: &str = "strip";
@@ -47,6 +49,81 @@ pub struct Behavior {
     target_dir: Option<String>,
 }
 
+#[derive(Debug)]
+enum InstallError {
+    Unimplemented(String),
+    DirNeedsArg(),
+    CreateDirFailed(PathBuf, std::io::Error),
+    ChmodFailed(PathBuf),
+    InvalidTarget(PathBuf),
+    TargetDirIsntDir(PathBuf),
+    BackupFailed(PathBuf, PathBuf, String),
+    InstallFailed(PathBuf, PathBuf, std::io::Error),
+    StripProgramFailed(String),
+    MetadataFailed(std::io::Error),
+    NoSuchUser(String),
+    NoSuchGroup(String),
+    SilentError(),
+}
+
+impl UCustomError for InstallError {
+    fn code(&self) -> i32 {
+        match self {
+            InstallError::Unimplemented(_) => 2,
+            _ => 1,
+        }
+    }
+
+    fn usage(&self) -> bool {
+        false
+    }
+}
+
+impl Error for InstallError {}
+
+impl Display for InstallError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        use InstallError as IE;
+        match self {
+            IE::Unimplemented(opt) => write!(f, "Unimplemented feature: {}", opt),
+            IE::DirNeedsArg() => write!(
+                f,
+                "{} with -d requires at least one argument.",
+                executable!()
+            ),
+            IE::CreateDirFailed(dir, e) => write!(f, "failed to create {}: {}", dir.display(), e),
+            IE::ChmodFailed(file) => write!(f, "failed to chmod {}", file.display()),
+            IE::InvalidTarget(target) => write!(
+                f,
+                "invalid target {}: No such file or directory",
+                target.display()
+            ),
+            IE::TargetDirIsntDir(target) => {
+                write!(f, "target '{}' is not a directory", target.display())
+            }
+            IE::BackupFailed(from, to, e) => write!(
+                f,
+                "cannot backup file '{}' to '{}': {}",
+                from.display(),
+                to.display(),
+                e
+            ),
+            IE::InstallFailed(from, to, e) => write!(
+                f,
+                "cannot install '{}' to '{}': {}",
+                from.display(),
+                to.display(),
+                e
+            ),
+            IE::StripProgramFailed(msg) => write!(f, "strip program failed: {}", msg),
+            IE::MetadataFailed(e) => write!(f, "{}", e.to_string()),
+            IE::NoSuchUser(user) => write!(f, "no such user: {}", user),
+            IE::NoSuchGroup(group) => write!(f, "no such group: {}", group),
+            IE::SilentError() => write!(f, ""),
+        }
+    }
+}
+
 #[derive(Clone, Eq, PartialEq)]
 pub enum MainFunction {
     /// Create directories
@@ -97,7 +174,8 @@ fn get_usage() -> String {
 ///
 /// Returns a program return code.
 ///
-pub fn uumain(args: impl uucore::Args) -> i32 {
+#[uucore_procs::gen_uumain]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let usage = get_usage();
 
     let matches = uu_app().usage(&usage[..]).get_matches_from(args);
@@ -107,17 +185,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         .map(|v| v.map(ToString::to_string).collect())
         .unwrap_or_default();
 
-    if let Err(s) = check_unimplemented(&matches) {
-        show_error!("Unimplemented feature: {}", s);
-        return 2;
-    }
+    check_unimplemented(&matches)?;
 
-    let behavior = match behavior(&matches) {
-        Ok(x) => x,
-        Err(ret) => {
-            return ret;
-        }
-    };
+    let behavior = behavior(&matches)?;
 
     match behavior.main_function {
         MainFunction::Directory => directory(paths, behavior),
@@ -269,13 +339,13 @@ pub fn uu_app() -> App<'static, 'static> {
 /// Error datum is a string of the unimplemented argument.
 ///
 ///
-fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> {
+fn check_unimplemented(matches: &ArgMatches) -> UResult<()> {
     if matches.is_present(OPT_NO_TARGET_DIRECTORY) {
-        Err("--no-target-directory, -T")
+        Err(InstallError::Unimplemented(String::from("--no-target-directory, -T")).into())
     } else if matches.is_present(OPT_PRESERVE_CONTEXT) {
-        Err("--preserve-context, -P")
+        Err(InstallError::Unimplemented(String::from("--preserve-context, -P")).into())
     } else if matches.is_present(OPT_CONTEXT) {
-        Err("--context, -Z")
+        Err(InstallError::Unimplemented(String::from("--context, -Z")).into())
     } else {
         Ok(())
     }
@@ -289,7 +359,7 @@ fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> {
 ///
 /// In event of failure, returns an integer intended as a program return code.
 ///
-fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
+fn behavior(matches: &ArgMatches) -> UResult<Behavior> {
     let main_function = if matches.is_present(OPT_DIRECTORY) {
         MainFunction::Directory
     } else {
@@ -314,10 +384,7 @@ fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
         matches.value_of(OPT_BACKUP),
     );
     let backup_mode = match backup_mode {
-        Err(err) => {
-            show_usage_error!("{}", err);
-            return Err(1);
-        }
+        Err(err) => return Err(USimpleError::new(1, err)),
         Ok(mode) => mode,
     };
 
@@ -351,10 +418,9 @@ fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
 ///
 /// Returns an integer intended as a program return code.
 ///
-fn directory(paths: Vec<String>, b: Behavior) -> i32 {
+fn directory(paths: Vec<String>, b: Behavior) -> UResult<()> {
     if paths.is_empty() {
-        println!("{} with -d requires at least one argument.", executable!());
-        1
+        Err(InstallError::DirNeedsArg().into())
     } else {
         let mut all_successful = true;
 
@@ -384,9 +450,9 @@ fn directory(paths: Vec<String>, b: Behavior) -> i32 {
             }
         }
         if all_successful {
-            0
+            Ok(())
         } else {
-            1
+            Err(InstallError::SilentError().into())
         }
     }
 }
@@ -403,7 +469,7 @@ fn is_new_file_path(path: &Path) -> bool {
 ///
 /// Returns an integer intended as a program return code.
 ///
-fn standard(mut paths: Vec<String>, b: Behavior) -> i32 {
+fn standard(mut paths: Vec<String>, b: Behavior) -> UResult<()> {
     let target: PathBuf = b
         .target_dir
         .clone()
@@ -418,13 +484,11 @@ fn standard(mut paths: Vec<String>, b: Behavior) -> i32 {
         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;
+                    return Err(InstallError::CreateDirFailed(parent.to_path_buf(), e).into());
                 }
 
                 if mode::chmod(parent, b.mode()).is_err() {
-                    show_error!("failed to chmod {}", parent.display());
-                    return 1;
+                    return Err(InstallError::ChmodFailed(parent.to_path_buf()).into());
                 }
             }
         }
@@ -432,11 +496,7 @@ fn standard(mut paths: Vec<String>, b: Behavior) -> i32 {
         if target.is_file() || is_new_file_path(&target) {
             copy_file_to_file(&sources[0], &target, &b)
         } else {
-            show_error!(
-                "invalid target {}: No such file or directory",
-                target.display()
-            );
-            1
+            Err(InstallError::InvalidTarget(target).into())
         }
     }
 }
@@ -451,10 +511,9 @@ fn standard(mut 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: &Path, b: &Behavior) -> i32 {
+fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UResult<()> {
     if !target_dir.is_dir() {
-        show_error!("target '{}' is not a directory", target_dir.display());
-        return 1;
+        return Err(InstallError::TargetDirIsntDir(target_dir.to_path_buf()).into());
     }
 
     let mut all_successful = true;
@@ -484,9 +543,9 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i3
         }
     }
     if all_successful {
-        0
+        Ok(())
     } else {
-        1
+        Err(InstallError::SilentError().into())
     }
 }
 
@@ -500,12 +559,8 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i3
 /// _file_ must exist as a non-directory.
 /// _target_ must be a non-directory
 ///
-fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 {
-    if copy(file, target, b).is_err() {
-        1
-    } else {
-        0
-    }
+fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> UResult<()> {
+    copy(file, target, b)
 }
 
 /// Copy one file to a new location, changing metadata.
@@ -520,8 +575,8 @@ fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 {
 /// If the copy system call fails, we print a verbose error and return an empty error value.
 ///
 #[allow(clippy::cognitive_complexity)]
-fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
-    if b.compare && !need_copy(from, to, b) {
+fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> {
+    if b.compare && !need_copy(from, to, b)? {
         return Ok(());
     }
     // Declare the path here as we may need it for the verbose output below.
@@ -536,13 +591,12 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
         if let Some(ref backup_path) = backup_path {
             // TODO!!
             if let Err(err) = fs::rename(to, backup_path) {
-                show_error!(
-                    "install: cannot backup file '{}' to '{}': {}",
-                    to.display(),
-                    backup_path.display(),
-                    err
-                );
-                return Err(());
+                return Err(InstallError::BackupFailed(
+                    to.to_path_buf(),
+                    backup_path.to_path_buf(),
+                    err.to_string(),
+                )
+                .into());
             }
         }
     }
@@ -552,52 +606,41 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
          * https://github.com/rust-lang/rust/issues/79390
          */
         if let Err(err) = File::create(to) {
-            show_error!(
-                "install: cannot install '{}' to '{}': {}",
-                from.display(),
-                to.display(),
-                err
+            return Err(
+                InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into(),
             );
-            return Err(());
         }
     } else if let Err(err) = fs::copy(from, to) {
-        show_error!(
-            "cannot install '{}' to '{}': {}",
-            from.display(),
-            to.display(),
-            err
-        );
-        return Err(());
+        return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into());
     }
 
     if b.strip && cfg!(not(windows)) {
         match Command::new(&b.strip_program).arg(to).output() {
             Ok(o) => {
                 if !o.status.success() {
-                    crash!(
-                        1,
-                        "strip program failed: {}",
-                        String::from_utf8(o.stderr).unwrap_or_default()
-                    );
+                    return Err(InstallError::StripProgramFailed(
+                        String::from_utf8(o.stderr).unwrap_or_default(),
+                    )
+                    .into());
                 }
             }
-            Err(e) => crash!(1, "strip program execution failed: {}", e),
+            Err(e) => return Err(InstallError::StripProgramFailed(e.to_string()).into()),
         }
     }
 
     if mode::chmod(to, b.mode()).is_err() {
-        return Err(());
+        return Err(InstallError::ChmodFailed(to.to_path_buf()).into());
     }
 
     if !b.owner.is_empty() {
         let meta = match fs::metadata(to) {
             Ok(meta) => meta,
-            Err(f) => crash!(1, "{}", f.to_string()),
+            Err(e) => return Err(InstallError::MetadataFailed(e).into()),
         };
 
         let owner_id = match usr2uid(&b.owner) {
             Ok(g) => g,
-            _ => crash!(1, "no such user: {}", b.owner),
+            _ => return Err(InstallError::NoSuchUser(b.owner.clone()).into()),
         };
         let gid = meta.gid();
         match wrap_chown(
@@ -620,12 +663,12 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
     if !b.group.is_empty() {
         let meta = match fs::metadata(to) {
             Ok(meta) => meta,
-            Err(f) => crash!(1, "{}", f.to_string()),
+            Err(e) => return Err(InstallError::MetadataFailed(e).into()),
         };
 
         let group_id = match grp2gid(&b.group) {
             Ok(g) => g,
-            _ => crash!(1, "no such group: {}", b.group),
+            _ => return Err(InstallError::NoSuchGroup(b.group.clone()).into()),
         };
         match wrap_chgrp(to, &meta, group_id, false, Verbosity::Normal) {
             Ok(n) => {
@@ -640,7 +683,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
     if b.preserve_timestamps {
         let meta = match fs::metadata(from) {
             Ok(meta) => meta,
-            Err(f) => crash!(1, "{}", f.to_string()),
+            Err(e) => return Err(InstallError::MetadataFailed(e).into()),
         };
 
         let modified_time = FileTime::from_last_modification_time(&meta);
@@ -679,14 +722,14 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
 ///
 /// Crashes the program if a nonexistent owner or group is specified in _b_.
 ///
-fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool {
+fn need_copy(from: &Path, to: &Path, b: &Behavior) -> UResult<bool> {
     let from_meta = match fs::metadata(from) {
         Ok(meta) => meta,
-        Err(_) => return true,
+        Err(_) => return Ok(true),
     };
     let to_meta = match fs::metadata(to) {
         Ok(meta) => meta,
-        Err(_) => return true,
+        Err(_) => return Ok(true),
     };
 
     // setuid || setgid || sticky
@@ -696,15 +739,15 @@ fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool {
         || from_meta.mode() & extra_mode != 0
         || to_meta.mode() & extra_mode != 0
     {
-        return true;
+        return Ok(true);
     }
 
     if !from_meta.is_file() || !to_meta.is_file() {
-        return true;
+        return Ok(true);
     }
 
     if from_meta.len() != to_meta.len() {
-        return true;
+        return Ok(true);
     }
 
     // TODO: if -P (#1809) and from/to contexts mismatch, return true.
@@ -712,31 +755,31 @@ fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool {
     if !b.owner.is_empty() {
         let owner_id = match usr2uid(&b.owner) {
             Ok(id) => id,
-            _ => crash!(1, "no such user: {}", b.owner),
+            _ => return Err(InstallError::NoSuchUser(b.owner.clone()).into()),
         };
         if owner_id != to_meta.uid() {
-            return true;
+            return Ok(true);
         }
     } else if !b.group.is_empty() {
         let group_id = match grp2gid(&b.group) {
             Ok(id) => id,
-            _ => crash!(1, "no such group: {}", b.group),
+            _ => return Err(InstallError::NoSuchGroup(b.group.clone()).into()),
         };
         if group_id != to_meta.gid() {
-            return true;
+            return Ok(true);
         }
     } else {
         #[cfg(not(target_os = "windows"))]
         unsafe {
             if to_meta.uid() != geteuid() || to_meta.gid() != getegid() {
-                return true;
+                return Ok(true);
             }
         }
     }
 
     if !diff(from.to_str().unwrap(), to.to_str().unwrap()) {
-        return true;
+        return Ok(true);
     }
 
-    false
+    Ok(false)
 }

From 0461a45c9ab5969fea47ac03b490b2135819a24b Mon Sep 17 00:00:00 2001
From: Andreas Hartmann <andreas.h@rtmann.de>
Date: Wed, 14 Jul 2021 08:39:12 +0200
Subject: [PATCH 02/29] install: Use show! macro for noncritical errors during
 execution

Drop the previous flags that would tell whether a noncritical error occured
during execution in favor of the `show!` macro from the error submodule.

This allows us to generate regular error types during execution to signify
failures inside the program, but without prematurely aborting program execution
if not needed or specified.

Also make verbose outputs use `print!` and friends instead of `show_error!` to
ensure verbose output is redirected to stdout, not stderr.
---
 src/uu/install/src/install.rs | 81 ++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 40 deletions(-)

diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs
index 6000837cb..7de69df00 100644
--- a/src/uu/install/src/install.rs
+++ b/src/uu/install/src/install.rs
@@ -63,7 +63,7 @@ enum InstallError {
     MetadataFailed(std::io::Error),
     NoSuchUser(String),
     NoSuchGroup(String),
-    SilentError(),
+    OmittingDirectory(PathBuf),
 }
 
 impl UCustomError for InstallError {
@@ -119,7 +119,7 @@ impl Display for InstallError {
             IE::MetadataFailed(e) => write!(f, "{}", e.to_string()),
             IE::NoSuchUser(user) => write!(f, "no such user: {}", user),
             IE::NoSuchGroup(group) => write!(f, "no such group: {}", group),
-            IE::SilentError() => write!(f, ""),
+            IE::OmittingDirectory(dir) => write!(f, "omitting directory '{}'", dir.display()),
         }
     }
 }
@@ -416,44 +416,46 @@ fn behavior(matches: &ArgMatches) -> UResult<Behavior> {
 /// GNU man pages describe this functionality as creating 'all components of
 /// the specified directories'.
 ///
-/// Returns an integer intended as a program return code.
+/// Returns a Result type with the Err variant containing the error message.
 ///
 fn directory(paths: Vec<String>, b: Behavior) -> UResult<()> {
     if paths.is_empty() {
         Err(InstallError::DirNeedsArg().into())
     } else {
-        let mut all_successful = true;
-
         for path in paths.iter().map(Path::new) {
             // if the path already exist, don't try to create it again
             if !path.exists() {
-                // Differently than the primary functionality (MainFunction::Standard), the directory
-                // functionality should create all ancestors (or components) of a directory regardless
-                // of the presence of the "-D" flag.
-                // NOTE: the GNU "install" sets the expected mode only for the target directory. All
-                // created ancestor directories will have the default mode. Hence it is safe to use
-                // fs::create_dir_all and then only modify the target's dir mode.
-                if let Err(e) = fs::create_dir_all(path) {
-                    show_error!("{}: {}", path.display(), e);
-                    all_successful = false;
+                // Differently than the primary functionality
+                // (MainFunction::Standard), the directory functionality should
+                // create all ancestors (or components) of a directory
+                // regardless of the presence of the "-D" flag.
+                //
+                // NOTE: the GNU "install" sets the expected mode only for the
+                // target directory. All created ancestor directories will have
+                // the default mode. Hence it is safe to use fs::create_dir_all
+                // and then only modify the target's dir mode.
+                if let Err(e) =
+                    fs::create_dir_all(path).map_err_context(|| format!("{}", path.display()))
+                {
+                    show!(e);
                     continue;
                 }
 
                 if b.verbose {
-                    show_error!("creating directory '{}'", path.display());
+                    println!("creating directory '{}'", path.display());
                 }
             }
 
             if mode::chmod(path, b.mode()).is_err() {
-                all_successful = false;
+                // Error messages are printed by the mode::chmod function!
+                uucore::error::set_exit_code(1);
                 continue;
             }
         }
-        if all_successful {
-            Ok(())
-        } else {
-            Err(InstallError::SilentError().into())
-        }
+        // If the exit code was set, or show! has been called at least once
+        // (which sets the exit code as well), function execution will end after
+        // this return.
+        Ok(())
     }
 }
 
@@ -467,7 +469,7 @@ fn is_new_file_path(path: &Path) -> bool {
 
 /// Perform an install, given a list of paths and behavior.
 ///
-/// Returns an integer intended as a program return code.
+/// Returns a Result type with the Err variant containing the error message.
 ///
 fn standard(mut paths: Vec<String>, b: Behavior) -> UResult<()> {
     let target: PathBuf = b
@@ -504,7 +506,7 @@ fn standard(mut paths: Vec<String>, b: Behavior) -> UResult<()> {
 /// Copy some files into a directory.
 ///
 /// Prints verbose information and error messages.
-/// Returns an integer intended as a program return code.
+/// Returns a Result type with the Err variant containing the error message.
 ///
 /// # Parameters
 ///
@@ -515,22 +517,19 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UR
     if !target_dir.is_dir() {
         return Err(InstallError::TargetDirIsntDir(target_dir.to_path_buf()).into());
     }
-
-    let mut all_successful = true;
     for sourcepath in files.iter() {
         if !sourcepath.exists() {
-            show_error!(
-                "cannot stat '{}': No such file or directory",
-                sourcepath.display()
+            let err = UIoError::new(
+                std::io::ErrorKind::NotFound,
+                format!("cannot stat '{}'", sourcepath.display()),
             );
-
-            all_successful = false;
+            show!(err);
             continue;
         }
 
         if sourcepath.is_dir() {
-            show_error!("omitting directory '{}'", sourcepath.display());
-            all_successful = false;
+            let err = InstallError::OmittingDirectory(sourcepath.to_path_buf());
+            show!(err);
             continue;
         }
 
@@ -538,21 +537,20 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UR
         let filename = sourcepath.components().last().unwrap();
         targetpath.push(filename);
 
-        if copy(sourcepath, &targetpath, b).is_err() {
-            all_successful = false;
+        if let Err(e) = copy(sourcepath, &targetpath, b) {
+            show!(e);
         }
     }
-    if all_successful {
-        Ok(())
-    } else {
-        Err(InstallError::SilentError().into())
-    }
+    // If the exit code was set, or show! has been called at least once
+    // (which sets the exit code as well), function execution will end after
+    // this return.
+    Ok(())
 }
 
 /// Copy a file to another file.
 ///
 /// Prints verbose information and error messages.
-/// Returns an integer intended as a program return code.
+/// Returns a Result type with the Err variant containing the error message.
 ///
 /// # Parameters
 ///
@@ -565,6 +563,8 @@ fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> UResult<()> {
 
 /// Copy one file to a new location, changing metadata.
 ///
+/// Returns a Result type with the Err variant containing the error message.
+///
 /// # Parameters
 ///
 /// _from_ must exist as a non-directory.
@@ -707,6 +707,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> {
 }
 
 /// Return true if a file is necessary to copy. This is the case when:
+///
 /// - _from_ or _to_ is nonexistent;
 /// - either file has a sticky bit or set[ug]id bit, or the user specified one;
 /// - either file isn't a regular file;

From 8066ea87cc704680a11c299b7b67e9ecf14f155e Mon Sep 17 00:00:00 2001
From: Andreas Hartmann <andreas.h@rtmann.de>
Date: Tue, 20 Jul 2021 08:38:13 +0200
Subject: [PATCH 03/29] error: Implement `uio_error`-macro

Adds a new macro `uio_error` that acts as a convenience wrapper for
constructing `UIoError` instances from `std::io::Error`s with a custom error
message prepended. It constructs a new `UIoError` instance for the user.

Usage examples are included in the docs.
---
 src/uucore/src/lib/mods/error.rs | 69 ++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs
index c64e7df66..2cf09f9bf 100644
--- a/src/uucore/src/lib/mods/error.rs
+++ b/src/uucore/src/lib/mods/error.rs
@@ -48,6 +48,8 @@
 //! * Using [`ExitCode`] is not recommended but can be useful for converting utils to use
 //!   [`UResult`].
 
+// spell-checker:ignore uioerror
+
 use std::{
     error::Error,
     fmt::{Display, Formatter},
@@ -578,6 +580,73 @@ impl From<UIoError> for UError {
     }
 }
 
+/// Shorthand to construct [`UIoError`]-instances.
+///
+/// This macro serves as a convenience call to quickly construct instances of
+/// [`UIoError`]. It takes:
+///
+/// - An instance of [`std::io::Error`]
+/// - A `format!`-compatible string and
+/// - An arbitrary number of arguments to the format string
+///
+/// In exactly this order. It is equivalent to the more verbose code seen in the
+/// example.
+///
+/// # Examples
+///
+/// ```
+/// use uucore::error::UIoError;
+/// use uucore::uio_error;
+///
+/// let io_err = std::io::Error::new(
+///     std::io::ErrorKind::PermissionDenied, "fix me please!"
+/// );
+///
+/// let uio_err = UIoError::new(
+///     io_err.kind(),
+///     format!("Error code: {}", 2)
+/// );
+///
+/// let other_uio_err = uio_error!(io_err, "Error code: {}", 2);
+///
+/// // prints "fix me please!: Permission denied"
+/// println!("{}", uio_err);
+/// // prints "Error code: 2: Permission denied"
+/// println!("{}", other_uio_err);
+/// ```
+///
+/// The [`std::fmt::Display`] impl of [`UIoError`] will then ensure that an
+/// appropriate error message relating to the actual error kind of the
+/// [`std::io::Error`] is appended to whatever error message is defined in
+/// addition (as secondary argument).
+///
+/// If you want to show only the error message for the [`std::io::ErrorKind`]
+/// that's contained in [`UIoError`], pass the second argument as empty string:
+///
+/// ```
+/// use uucore::error::UIoError;
+/// use uucore::uio_error;
+///
+/// let io_err = std::io::Error::new(
+///     std::io::ErrorKind::PermissionDenied, "fix me please!"
+/// );
+///
+/// let other_uio_err = uio_error!(io_err, "");
+///
+/// // prints: ": Permission denied"
+/// println!("{}", other_uio_err);
+/// ```
+//#[macro_use]
+#[macro_export]
+macro_rules! uio_error(
+    ($err:expr, $($args:tt)+) => ({
+        UIoError::new(
+            $err.kind(),
+            format!($($args)+)
+        )
+    })
+);
+
 /// Common errors for utilities.
 ///
 /// If identical errors appear across multiple utilities, they should be added here.

From 4f235e9e95640227f67330ef00f983ec13c530a4 Mon Sep 17 00:00:00 2001
From: Andreas Hartmann <andreas.h@rtmann.de>
Date: Mon, 19 Jul 2021 16:51:14 +0200
Subject: [PATCH 04/29] install: Use `uio_error` to construct errors from
 std::io::Error

Use new macro to construct UIoError types from `std::io::Error` before
printing. This gives consistent error messages across all utilities as it
prepends custom errors to the error description for the respective application
and error type. This saves the developers from manually appending the
`std::io::Error`-specific error messages.
---
 src/uu/install/src/install.rs | 36 ++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs
index 7de69df00..2186738c4 100644
--- a/src/uu/install/src/install.rs
+++ b/src/uu/install/src/install.rs
@@ -5,7 +5,7 @@
 //  * For the full copyright and license information, please view the LICENSE file
 //  * that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) rwxr sourcepath targetpath Isnt
+// spell-checker:ignore (ToDO) rwxr sourcepath targetpath Isnt uioerror
 
 mod mode;
 
@@ -57,7 +57,7 @@ enum InstallError {
     ChmodFailed(PathBuf),
     InvalidTarget(PathBuf),
     TargetDirIsntDir(PathBuf),
-    BackupFailed(PathBuf, PathBuf, String),
+    BackupFailed(PathBuf, PathBuf, std::io::Error),
     InstallFailed(PathBuf, PathBuf, std::io::Error),
     StripProgramFailed(String),
     MetadataFailed(std::io::Error),
@@ -91,7 +91,9 @@ impl Display for InstallError {
                 "{} with -d requires at least one argument.",
                 executable!()
             ),
-            IE::CreateDirFailed(dir, e) => write!(f, "failed to create {}: {}", dir.display(), e),
+            IE::CreateDirFailed(dir, e) => {
+                Display::fmt(&uio_error!(e, "failed to create {}", dir.display()), f)
+            }
             IE::ChmodFailed(file) => write!(f, "failed to chmod {}", file.display()),
             IE::InvalidTarget(target) => write!(
                 f,
@@ -101,22 +103,26 @@ impl Display for InstallError {
             IE::TargetDirIsntDir(target) => {
                 write!(f, "target '{}' is not a directory", target.display())
             }
-            IE::BackupFailed(from, to, e) => write!(
+            IE::BackupFailed(from, to, e) => Display::fmt(
+                &uio_error!(
+                    e,
+                    "cannot backup '{}' to '{}'",
+                    from.display(),
+                    to.display()
+                ),
                 f,
-                "cannot backup file '{}' to '{}': {}",
-                from.display(),
-                to.display(),
-                e
             ),
-            IE::InstallFailed(from, to, e) => write!(
+            IE::InstallFailed(from, to, e) => Display::fmt(
+                &uio_error!(
+                    e,
+                    "cannot install '{}' to '{}'",
+                    from.display(),
+                    to.display()
+                ),
                 f,
-                "cannot install '{}' to '{}': {}",
-                from.display(),
-                to.display(),
-                e
             ),
             IE::StripProgramFailed(msg) => write!(f, "strip program failed: {}", msg),
-            IE::MetadataFailed(e) => write!(f, "{}", e.to_string()),
+            IE::MetadataFailed(e) => Display::fmt(&uio_error!(e, ""), f),
             IE::NoSuchUser(user) => write!(f, "no such user: {}", user),
             IE::NoSuchGroup(group) => write!(f, "no such group: {}", group),
             IE::OmittingDirectory(dir) => write!(f, "omitting directory '{}'", dir.display()),
@@ -594,7 +600,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> {
                 return Err(InstallError::BackupFailed(
                     to.to_path_buf(),
                     backup_path.to_path_buf(),
-                    err.to_string(),
+                    err,
                 )
                 .into());
             }

From ce037bdf55bc530768c9e45e2c5da632fab80c7e Mon Sep 17 00:00:00 2001
From: Andreas Hartmann <andreas.h@rtmann.de>
Date: Sun, 25 Jul 2021 09:40:37 +0200
Subject: [PATCH 05/29] install: remove obsolete function call

The `copy_file_to_file` function was only a wrapper to a `copy` function with
the exact same interface. It has now beed removed for clarity.
---
 src/uu/install/src/install.rs | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs
index 2186738c4..8ce219f7a 100644
--- a/src/uu/install/src/install.rs
+++ b/src/uu/install/src/install.rs
@@ -502,7 +502,7 @@ fn standard(mut paths: Vec<String>, b: Behavior) -> UResult<()> {
         }
 
         if target.is_file() || is_new_file_path(&target) {
-            copy_file_to_file(&sources[0], &target, &b)
+            copy(&sources[0], &target, &b)
         } else {
             Err(InstallError::InvalidTarget(target).into())
         }
@@ -543,9 +543,7 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UR
         let filename = sourcepath.components().last().unwrap();
         targetpath.push(filename);
 
-        if let Err(e) = copy(sourcepath, &targetpath, b) {
-            show!(e);
-        }
+        show_if_err!(copy(sourcepath, &targetpath, b));
     }
     // If the exit code was set, or show! has been called at least once
     // (which sets the exit code as well), function execution will end after
@@ -553,20 +551,6 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UR
     Ok(())
 }
 
-/// Copy a file to another file.
-///
-/// Prints verbose information and error messages.
-/// Returns a Result type with the Err variant containing the error message.
-///
-/// # Parameters
-///
-/// _file_ must exist as a non-directory.
-/// _target_ must be a non-directory
-///
-fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> UResult<()> {
-    copy(file, target, b)
-}
-
 /// Copy one file to a new location, changing metadata.
 ///
 /// Returns a Result type with the Err variant containing the error message.

From 6af3f774f12d9a2afb5b460f0a69dd8b64387cab Mon Sep 17 00:00:00 2001
From: Jeremy Soller <jeremy@system76.com>
Date: Fri, 6 Aug 2021 14:48:18 -0600
Subject: [PATCH 06/29] od: fix reading from file while supplying a format
 argument

The following test case read stdin instead of file:
```
echo abcdefg > file
cargo run -- od --format x1 file
```

This is because the -t/--format argument was able to absorb multiple
arguments after it. This has now been fixed, and a test case is added
to ensure it will not happen again.
---
 src/uu/od/src/od.rs      |  1 +
 tests/by-util/test_od.rs | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs
index ec5bb595a..359531d4e 100644
--- a/src/uu/od/src/od.rs
+++ b/src/uu/od/src/od.rs
@@ -435,6 +435,7 @@ pub fn uu_app() -> clap::App<'static, 'static> {
                 .long(options::FORMAT)
                 .help("select output format or formats")
                 .multiple(true)
+                .number_of_values(1)
                 .value_name("TYPE"),
         )
         .arg(
diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs
index 33d7d4dc4..0fc1d5106 100644
--- a/tests/by-util/test_od.rs
+++ b/tests/by-util/test_od.rs
@@ -46,6 +46,17 @@ fn test_file() {
         .succeeds()
         .no_stderr()
         .stdout_is(unindent(ALPHA_OUT));
+
+    // Ensure that default format matches `-t o2`, and that `-t` does not absorb file argument
+    new_ucmd!()
+        .arg("--endian=little")
+        .arg("-t")
+        .arg("o2")
+        .arg(file.as_os_str())
+        .succeeds()
+        .no_stderr()
+        .stdout_is(unindent(ALPHA_OUT));
+
     let _ = remove_file(file);
 }
 

From 5be4c48546b58e8ce591ec37823eb3ca69858a02 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Sat, 7 Aug 2021 21:05:47 +0200
Subject: [PATCH 07/29] cat: show \r\n as ^M$ when -E is enabled

This functionality was recently added to GNU cat, but had a bug.
This implementation will be commpatible with gnu once the bug in gnu is fixed.
---
 src/uu/cat/src/cat.rs     | 67 +++++++++++++++++++++++++++------------
 tests/by-util/test_cat.rs |  8 +++++
 2 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs
index f340fa9fa..6b6f68b13 100644
--- a/src/uu/cat/src/cat.rs
+++ b/src/uu/cat/src/cat.rs
@@ -123,6 +123,9 @@ struct OutputState {
 
     /// Whether the output cursor is at the beginning of a new line
     at_line_start: bool,
+
+    /// Whether we skipped a \r, which still needs to be printed
+    skipped_carriage_return: bool,
 }
 
 /// Represents an open file handle, stream, or other device
@@ -339,6 +342,7 @@ fn cat_files(files: Vec<String>, options: &OutputOptions) -> UResult<()> {
     let mut state = OutputState {
         line_number: 1,
         at_line_start: true,
+        skipped_carriage_return: false,
     };
     let mut error_messages: Vec<String> = Vec::new();
 
@@ -347,6 +351,9 @@ fn cat_files(files: Vec<String>, options: &OutputOptions) -> UResult<()> {
             error_messages.push(format!("{}: {}", path, err));
         }
     }
+    if state.skipped_carriage_return {
+        print!("\r");
+    }
     if error_messages.is_empty() {
         Ok(())
     } else {
@@ -435,6 +442,11 @@ fn write_lines<R: Read>(
         while pos < n {
             // skip empty line_number enumerating them if needed
             if in_buf[pos] == b'\n' {
+                // \r followed by \n is printed as ^M when show_ends is enabled, so that \r\n prints as ^M$
+                if state.skipped_carriage_return && options.show_ends {
+                    writer.write_all(b"^M")?;
+                    state.skipped_carriage_return = false;
+                }
                 if !state.at_line_start || !options.squeeze_blank || !one_blank_kept {
                     one_blank_kept = true;
                     if state.at_line_start && options.number == NumberingMode::All {
@@ -450,6 +462,11 @@ fn write_lines<R: Read>(
                 pos += 1;
                 continue;
             }
+            if state.skipped_carriage_return {
+                writer.write_all(b"\r")?;
+                state.skipped_carriage_return = false;
+                state.at_line_start = false;
+            }
             one_blank_kept = false;
             if state.at_line_start && options.number != NumberingMode::None {
                 write!(&mut writer, "{0:6}\t", state.line_number)?;
@@ -465,17 +482,22 @@ fn write_lines<R: Read>(
                 write_to_end(&in_buf[pos..], &mut writer)
             };
             // end of buffer?
-            if offset == 0 {
+            if offset + pos == in_buf.len() {
                 state.at_line_start = false;
                 break;
             }
-            // print suitable end of line
-            writer.write_all(options.end_of_line().as_bytes())?;
-            if handle.is_interactive {
-                writer.flush()?;
+            if in_buf[pos + offset] == b'\r' {
+                state.skipped_carriage_return = true;
+            } else {
+                assert_eq!(in_buf[pos + offset], b'\n');
+                // print suitable end of line
+                writer.write_all(options.end_of_line().as_bytes())?;
+                if handle.is_interactive {
+                    writer.flush()?;
+                }
+                state.at_line_start = true;
             }
-            state.at_line_start = true;
-            pos += offset;
+            pos += offset + 1;
         }
     }
 
@@ -483,17 +505,19 @@ fn write_lines<R: Read>(
 }
 
 // write***_to_end methods
-// Write all symbols till end of line or end of buffer is reached
-// Return the (number of written symbols + 1) or 0 if the end of buffer is reached
+// Write all symbols till \n or \r or end of buffer is reached
+// We need to stop at \r because it may be written as ^M depending on the byte after and settings;
+// however, write_nonprint_to_end doesn't need to stop at \r because it will always write \r as ^M.
+// Return the number of written symbols
 fn write_to_end<W: Write>(in_buf: &[u8], writer: &mut W) -> usize {
-    match in_buf.iter().position(|c| *c == b'\n') {
+    match in_buf.iter().position(|c| *c == b'\n' || *c == b'\r') {
         Some(p) => {
             writer.write_all(&in_buf[..p]).unwrap();
-            p + 1
+            p
         }
         None => {
             writer.write_all(in_buf).unwrap();
-            0
+            in_buf.len()
         }
     }
 }
@@ -501,20 +525,25 @@ fn write_to_end<W: Write>(in_buf: &[u8], writer: &mut W) -> usize {
 fn write_tab_to_end<W: Write>(mut in_buf: &[u8], writer: &mut W) -> usize {
     let mut count = 0;
     loop {
-        match in_buf.iter().position(|c| *c == b'\n' || *c == b'\t') {
+        match in_buf
+            .iter()
+            .position(|c| *c == b'\n' || *c == b'\t' || *c == b'\r')
+        {
             Some(p) => {
                 writer.write_all(&in_buf[..p]).unwrap();
                 if in_buf[p] == b'\n' {
-                    return count + p + 1;
-                } else {
+                    return count + p;
+                } else if in_buf[p] == b'\t' {
                     writer.write_all(b"^I").unwrap();
                     in_buf = &in_buf[p + 1..];
                     count += p + 1;
+                } else {
+                    return count + p;
                 }
             }
             None => {
                 writer.write_all(in_buf).unwrap();
-                return 0;
+                return in_buf.len();
             }
         };
     }
@@ -539,11 +568,7 @@ fn write_nonprint_to_end<W: Write>(in_buf: &[u8], writer: &mut W, tab: &[u8]) ->
         .unwrap();
         count += 1;
     }
-    if count != in_buf.len() {
-        count + 1
-    } else {
-        0
-    }
+    count
 }
 
 #[cfg(test)]
diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs
index d83b5515b..eb50d7dce 100644
--- a/tests/by-util/test_cat.rs
+++ b/tests/by-util/test_cat.rs
@@ -273,6 +273,14 @@ fn test_stdin_show_ends() {
             .stdout_only("\t\0$\n\t");
     }
 }
+#[test]
+fn test_show_ends_crlf() {
+    new_ucmd!()
+        .arg("-E")
+        .pipe_in("a\nb\r\n\rc\n\r\n\r")
+        .succeeds()
+        .stdout_only("a$\nb^M$\n\rc$\n^M$\n\r");
+}
 
 #[test]
 fn test_stdin_show_all() {

From 722936021747ec25df26f32e1aa17e3fe07c33b2 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Sat, 7 Aug 2021 21:31:05 +0200
Subject: [PATCH 08/29] cat: remove all per-file state

cat cannot keep per-file state, so move all remaining state (one_blank_kept)
to the global state.
---
 src/uu/cat/src/cat.rs     | 11 +++++++----
 tests/by-util/test_cat.rs | 12 ++++++++++++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs
index 6b6f68b13..c71f60a7e 100644
--- a/src/uu/cat/src/cat.rs
+++ b/src/uu/cat/src/cat.rs
@@ -126,6 +126,9 @@ struct OutputState {
 
     /// Whether we skipped a \r, which still needs to be printed
     skipped_carriage_return: bool,
+
+    /// Whether we have already printed a blank line
+    one_blank_kept: bool,
 }
 
 /// Represents an open file handle, stream, or other device
@@ -343,6 +346,7 @@ fn cat_files(files: Vec<String>, options: &OutputOptions) -> UResult<()> {
         line_number: 1,
         at_line_start: true,
         skipped_carriage_return: false,
+        one_blank_kept: false,
     };
     let mut error_messages: Vec<String> = Vec::new();
 
@@ -431,7 +435,6 @@ fn write_lines<R: Read>(
     let mut in_buf = [0; 1024 * 31];
     let stdout = io::stdout();
     let mut writer = stdout.lock();
-    let mut one_blank_kept = false;
 
     while let Ok(n) = handle.reader.read(&mut in_buf) {
         if n == 0 {
@@ -447,8 +450,8 @@ fn write_lines<R: Read>(
                     writer.write_all(b"^M")?;
                     state.skipped_carriage_return = false;
                 }
-                if !state.at_line_start || !options.squeeze_blank || !one_blank_kept {
-                    one_blank_kept = true;
+                if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept {
+                    state.one_blank_kept = true;
                     if state.at_line_start && options.number == NumberingMode::All {
                         write!(&mut writer, "{0:6}\t", state.line_number)?;
                         state.line_number += 1;
@@ -467,7 +470,7 @@ fn write_lines<R: Read>(
                 state.skipped_carriage_return = false;
                 state.at_line_start = false;
             }
-            one_blank_kept = false;
+            state.one_blank_kept = false;
             if state.at_line_start && options.number != NumberingMode::None {
                 write!(&mut writer, "{0:6}\t", state.line_number)?;
                 state.line_number += 1;
diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs
index eb50d7dce..e0bc49339 100644
--- a/tests/by-util/test_cat.rs
+++ b/tests/by-util/test_cat.rs
@@ -273,6 +273,18 @@ fn test_stdin_show_ends() {
             .stdout_only("\t\0$\n\t");
     }
 }
+
+#[test]
+fn squeeze_all_files() {
+    // empty lines at the end of a file are "squeezed" together with empty lines at the beginning
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.write("input1", "a\n\n");
+    at.write("input2", "\n\nb");
+    ucmd.args(&["input1", "input2", "-s"])
+        .succeeds()
+        .stdout_only("a\n\nb");
+}
+
 #[test]
 fn test_show_ends_crlf() {
     new_ucmd!()

From 03ceb6750e54f643e39cfb701b98bdbceb4e3b02 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Sun, 8 Aug 2021 00:48:37 +0200
Subject: [PATCH 09/29] cat: check if the input file is also the output file

---
 Cargo.lock                |  1 +
 src/uu/cat/Cargo.toml     |  5 +++--
 src/uu/cat/src/cat.rs     | 40 ++++++++++++++++++++++++++++++---
 tests/by-util/test_cat.rs | 47 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 87 insertions(+), 6 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index b954fffaf..9fe45b941 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2023,6 +2023,7 @@ dependencies = [
  "unix_socket",
  "uucore",
  "uucore_procs",
+ "winapi-util",
 ]
 
 [[package]]
diff --git a/src/uu/cat/Cargo.toml b/src/uu/cat/Cargo.toml
index b0721cee0..d80514385 100644
--- a/src/uu/cat/Cargo.toml
+++ b/src/uu/cat/Cargo.toml
@@ -23,9 +23,10 @@ uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_p
 
 [target.'cfg(unix)'.dependencies]
 unix_socket = "0.5.0"
+nix = "0.20.0"
 
-[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
-nix = "0.20"
+[target.'cfg(windows)'.dependencies]
+winapi-util = "0.1.5"
 
 [[bin]]
 name = "cat"
diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs
index f340fa9fa..9d982c048 100644
--- a/src/uu/cat/src/cat.rs
+++ b/src/uu/cat/src/cat.rs
@@ -22,11 +22,14 @@ use std::io::{self, Read, Write};
 use thiserror::Error;
 use uucore::error::UResult;
 
+#[cfg(unix)]
+use std::os::unix::io::AsRawFd;
+
 /// Linux splice support
 #[cfg(any(target_os = "linux", target_os = "android"))]
 mod splice;
 #[cfg(any(target_os = "linux", target_os = "android"))]
-use std::os::unix::io::{AsRawFd, RawFd};
+use std::os::unix::io::RawFd;
 
 /// Unix domain socket support
 #[cfg(unix)]
@@ -59,6 +62,8 @@ enum CatError {
     },
     #[error("Is a directory")]
     IsDirectory,
+    #[error("input file is output file")]
+    OutputIsInput,
 }
 
 type CatResult<T> = Result<T, CatError>;
@@ -297,7 +302,13 @@ fn cat_handle<R: Read>(
     }
 }
 
-fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> CatResult<()> {
+fn cat_path(
+    path: &str,
+    options: &OutputOptions,
+    state: &mut OutputState,
+    #[cfg(unix)] out_info: &nix::sys::stat::FileStat,
+    #[cfg(windows)] out_info: &winapi_util::file::Information,
+) -> CatResult<()> {
     if path == "-" {
         let stdin = io::stdin();
         let mut handle = InputHandle {
@@ -324,6 +335,10 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat
         }
         _ => {
             let file = File::open(path)?;
+            #[cfg(any(windows, unix))]
+            if same_file(out_info, &file) {
+                return Err(CatError::OutputIsInput);
+            }
             let mut handle = InputHandle {
                 #[cfg(any(target_os = "linux", target_os = "android"))]
                 file_descriptor: file.as_raw_fd(),
@@ -335,7 +350,26 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat
     }
 }
 
+#[cfg(unix)]
+fn same_file(a_info: &nix::sys::stat::FileStat, b: &File) -> bool {
+    let b_info = nix::sys::stat::fstat(b.as_raw_fd()).unwrap();
+    b_info.st_size != 0 && b_info.st_dev == a_info.st_dev && b_info.st_ino == a_info.st_ino
+}
+
+#[cfg(windows)]
+fn same_file(a_info: &winapi_util::file::Information, b: &File) -> bool {
+    let b_info = winapi_util::file::information(b).unwrap();
+    b_info.file_size() != 0
+        && b_info.volume_serial_number() == a_info.volume_serial_number()
+        && b_info.file_index() == a_info.file_index()
+}
+
 fn cat_files(files: Vec<String>, options: &OutputOptions) -> UResult<()> {
+    #[cfg(windows)]
+    let out_info = winapi_util::file::information(&std::io::stdout()).unwrap();
+    #[cfg(unix)]
+    let out_info = nix::sys::stat::fstat(std::io::stdout().as_raw_fd()).unwrap();
+
     let mut state = OutputState {
         line_number: 1,
         at_line_start: true,
@@ -343,7 +377,7 @@ fn cat_files(files: Vec<String>, options: &OutputOptions) -> UResult<()> {
     let mut error_messages: Vec<String> = Vec::new();
 
     for path in &files {
-        if let Err(err) = cat_path(path, options, &mut state) {
+        if let Err(err) = cat_path(path, options, &mut state, &out_info) {
             error_messages.push(format!("{}: {}", path, err));
         }
     }
diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs
index d83b5515b..03fea23e9 100644
--- a/tests/by-util/test_cat.rs
+++ b/tests/by-util/test_cat.rs
@@ -1,5 +1,4 @@
 use crate::common::util::*;
-#[cfg(unix)]
 use std::fs::OpenOptions;
 #[cfg(unix)]
 use std::io::Read;
@@ -443,3 +442,49 @@ fn test_domain_socket() {
 
     thread.join().unwrap();
 }
+
+#[test]
+fn test_write_to_self_empty() {
+    // it's ok if the input file is also the output file if it's empty
+    let s = TestScenario::new(util_name!());
+    let file_path = s.fixtures.plus("file.txt");
+
+    let file = OpenOptions::new()
+        .create_new(true)
+        .write(true)
+        .append(true)
+        .open(&file_path)
+        .unwrap();
+
+    s.ucmd().set_stdout(file).arg(&file_path).succeeds();
+}
+
+#[test]
+fn test_write_to_self() {
+    let s = TestScenario::new(util_name!());
+    let file_path = s.fixtures.plus("first_file");
+    s.fixtures.write("second_file", "second_file_content.");
+
+    let file = OpenOptions::new()
+        .create_new(true)
+        .write(true)
+        .append(true)
+        .open(&file_path)
+        .unwrap();
+
+    s.fixtures.append("first_file", "first_file_content.");
+
+    s.ucmd()
+        .set_stdout(file)
+        .arg("first_file")
+        .arg("first_file")
+        .arg("second_file")
+        .fails()
+        .code_is(2)
+        .stderr_only("cat: first_file: input file is output file\ncat: first_file: input file is output file");
+
+    assert_eq!(
+        s.fixtures.read("first_file"),
+        "first_file_content.second_file_content."
+    );
+}

From 6edc6ee73dae6715d699b2807c4188ee26949382 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Sun, 8 Aug 2021 01:11:46 +0200
Subject: [PATCH 10/29] build-gnu: remove timeout for cat-self

This edge case is now handled and the test should pass.
---
 util/build-gnu.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/util/build-gnu.sh b/util/build-gnu.sh
index 4d70ceccc..bbb16981e 100644
--- a/util/build-gnu.sh
+++ b/util/build-gnu.sh
@@ -96,7 +96,6 @@ sed -i 's|seq |/usr/bin/seq |' tests/misc/sort-discrim.sh
 
 # Add specific timeout to tests that currently hang to limit time spent waiting
 sed -i 's|seq \$|/usr/bin/timeout 0.1 seq \$|' tests/misc/seq-precision.sh tests/misc/seq-long-double.sh
-sed -i 's|cat |/usr/bin/timeout 0.1 cat |' tests/misc/cat-self.sh
 
 
 # Remove dup of /usr/bin/ when executed several times

From 0f39e7c101e408f0d0bb3afcbb253213063df6ae Mon Sep 17 00:00:00 2001
From: Dean Li <deantvv@gmail.com>
Date: Sun, 8 Aug 2021 18:09:25 +0800
Subject: [PATCH 11/29] id: use UResult

---
 src/uu/id/src/id.rs | 57 ++++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs
index 9f92a4561..1dd1b176d 100644
--- a/src/uu/id/src/id.rs
+++ b/src/uu/id/src/id.rs
@@ -44,6 +44,8 @@ use clap::{crate_version, App, Arg};
 use selinux;
 use std::ffi::CStr;
 use uucore::entries::{self, Group, Locate, Passwd};
+use uucore::error::UResult;
+use uucore::error::{set_exit_code, USimpleError};
 pub use uucore::libc;
 use uucore::libc::{getlogin, uid_t};
 use uucore::process::{getegid, geteuid, getgid, getuid};
@@ -123,10 +125,10 @@ struct State {
     // 1000 10 968 975
     // +++ exited with 0 +++
     user_specified: bool,
-    exit_code: i32,
 }
 
-pub fn uumain(args: impl uucore::Args) -> i32 {
+#[uucore_procs::gen_uumain]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let usage = get_usage();
     let after_help = get_description();
 
@@ -161,7 +163,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         },
         user_specified: !users.is_empty(),
         ids: None,
-        exit_code: 0,
     };
 
     let default_format = {
@@ -170,14 +171,23 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     };
 
     if (state.nflag || state.rflag) && default_format && !state.cflag {
-        crash!(1, "cannot print only names or real IDs in default format");
+        return Err(USimpleError::new(
+            1,
+            "cannot print only names or real IDs in default format".to_string(),
+        ));
     }
     if state.zflag && default_format && !state.cflag {
         // NOTE: GNU test suite "id/zero.sh" needs this stderr output:
-        crash!(1, "option --zero not permitted in default format");
+        return Err(USimpleError::new(
+            1,
+            "option --zero not permitted in default format".to_string(),
+        ));
     }
     if state.user_specified && state.cflag {
-        crash!(1, "cannot print security context when user specified");
+        return Err(USimpleError::new(
+            1,
+            "cannot print security context when user specified".to_string(),
+        ));
     }
 
     let delimiter = {
@@ -204,11 +214,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 print!("{}{}", String::from_utf8_lossy(bytes), line_ending);
             } else {
                 // print error because `cflag` was explicitly requested
-                crash!(1, "can't get process context");
+                return Err(USimpleError::new(1, "can't get process context"));
             }
-            return state.exit_code;
+            return Ok(());
         } else {
-            crash!(1, "--context (-Z) works only on an SELinux-enabled kernel");
+            return Err(USimpleError::new(
+                1,
+                "--context (-Z) works only on an SELinux-enabled kernel".to_string(),
+            ));
         }
     }
 
@@ -220,7 +233,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 Ok(p) => Some(p),
                 Err(_) => {
                     show_error!("'{}': no such user", users[i]);
-                    state.exit_code = 1;
+                    set_exit_code(1);
                     if i + 1 >= users.len() {
                         break;
                     } else {
@@ -234,17 +247,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         if matches.is_present(options::OPT_PASSWORD) {
             // BSD's `id` ignores all but the first specified user
             pline(possible_pw.map(|v| v.uid()));
-            return state.exit_code;
+            return Ok(());
         };
         if matches.is_present(options::OPT_HUMAN_READABLE) {
             // BSD's `id` ignores all but the first specified user
             pretty(possible_pw);
-            return state.exit_code;
+            return Ok(());
         }
         if matches.is_present(options::OPT_AUDIT) {
             // BSD's `id` ignores specified users
             auditid();
-            return state.exit_code;
+            return Ok(());
         }
 
         let (uid, gid) = possible_pw.map(|p| (p.uid(), p.gid())).unwrap_or((
@@ -264,7 +277,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 if state.nflag {
                     entries::gid2grp(gid).unwrap_or_else(|_| {
                         show_error!("cannot find name for group ID {}", gid);
-                        state.exit_code = 1;
+                        set_exit_code(1);
                         gid.to_string()
                     })
                 } else {
@@ -279,7 +292,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 if state.nflag {
                     entries::uid2usr(uid).unwrap_or_else(|_| {
                         show_error!("cannot find name for user ID {}", uid);
-                        state.exit_code = 1;
+                        set_exit_code(1);
                         uid.to_string()
                     })
                 } else {
@@ -304,7 +317,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                         if state.nflag {
                             entries::gid2grp(id).unwrap_or_else(|_| {
                                 show_error!("cannot find name for group ID {}", id);
-                                state.exit_code = 1;
+                                set_exit_code(1);
                                 id.to_string()
                             })
                         } else {
@@ -332,7 +345,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         }
     }
 
-    state.exit_code
+    Ok(())
 }
 
 pub fn uu_app() -> App<'static, 'static> {
@@ -560,7 +573,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
         uid,
         entries::uid2usr(uid).unwrap_or_else(|_| {
             show_error!("cannot find name for user ID {}", uid);
-            state.exit_code = 1;
+            set_exit_code(1);
             uid.to_string()
         })
     );
@@ -569,7 +582,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
         gid,
         entries::gid2grp(gid).unwrap_or_else(|_| {
             show_error!("cannot find name for group ID {}", gid);
-            state.exit_code = 1;
+            set_exit_code(1);
             gid.to_string()
         })
     );
@@ -579,7 +592,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
             euid,
             entries::uid2usr(euid).unwrap_or_else(|_| {
                 show_error!("cannot find name for user ID {}", euid);
-                state.exit_code = 1;
+                set_exit_code(1);
                 euid.to_string()
             })
         );
@@ -590,7 +603,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
             euid,
             entries::gid2grp(egid).unwrap_or_else(|_| {
                 show_error!("cannot find name for group ID {}", egid);
-                state.exit_code = 1;
+                set_exit_code(1);
                 egid.to_string()
             })
         );
@@ -604,7 +617,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
                 gr,
                 entries::gid2grp(gr).unwrap_or_else(|_| {
                     show_error!("cannot find name for group ID {}", gr);
-                    state.exit_code = 1;
+                    set_exit_code(1);
                     gr.to_string()
                 })
             ))

From 8389a36eb0f36ccba04adcfb91ebd10daf5e99ad Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Sun, 8 Aug 2021 21:22:54 +0200
Subject: [PATCH 12/29] README: remove duplicate entry

cat was present twice
---
 README.md | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/README.md b/README.md
index 1b2ddf67f..d4426543f 100644
--- a/README.md
+++ b/README.md
@@ -372,18 +372,17 @@ To contribute to uutils, please see [CONTRIBUTING](CONTRIBUTING.md).
 | basenc    | expr      |        |
 | cat       | install   |        |
 | chcon     | join      |        |
-| cat       | ls        |        |
-| chgrp     | more      |        |
-| chmod     | numfmt    |        |
-| chown     | od (`--strings` and 128-bit data types missing) | |
-| chroot    | pr        |        |
-| cksum     | printf    |        |
-| comm      | sort      |        |
-| csplit    | split     |        |
-| cut       | tac       |        |
-| dircolors | tail      |        |
-| dirname   | test      |        |
-| du        |           |        |
+| chgrp     | ls        |        |
+| chmod     | more      |        |
+| chown     | numfmt    |        |
+| chroot    | od (`--strings` and 128-bit data types missing) | |
+| cksum     | pr        |        |
+| comm      | printf    |        |
+| csplit    | sort      |        |
+| cut       | split     |        |
+| dircolors | tac       |        |
+| dirname   | tail      |        |
+| du        | test      |        |
 | echo      |           |        |
 | env       |           |        |
 | expand    |           |        |

From 81430dcb40f085bfb871ce5ac92af0e53502a291 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Sun, 8 Aug 2021 21:23:30 +0200
Subject: [PATCH 13/29] README: remove chcon from To Do

chcon is now listed as "Done"
---
 README.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index d4426543f..bc3cc0a98 100644
--- a/README.md
+++ b/README.md
@@ -365,9 +365,9 @@ To contribute to uutils, please see [CONTRIBUTING](CONTRIBUTING.md).
 
 | Done      | Semi-Done | To Do  |
 |-----------|-----------|--------|
-| arch      | cp        | chcon  |
-| base32    | date      | runcon |
-| base64    | dd        | stty   |
+| arch      | cp        | runcon |
+| base32    | date      | stty   |
+| base64    | dd        |        |
 | basename  | df        |        |
 | basenc    | expr      |        |
 | cat       | install   |        |

From 6e2f6b28454c56d262eb7b49ef7b41e65c6be43f Mon Sep 17 00:00:00 2001
From: David CARLIER <devnexen@gmail.com>
Date: Mon, 9 Aug 2021 21:56:12 +0100
Subject: [PATCH 14/29] nice little code simplification, included already in
 the libc for all unixes

---
 src/uu/nice/src/nice.rs | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/src/uu/nice/src/nice.rs b/src/uu/nice/src/nice.rs
index d5a4094d1..49efe32e0 100644
--- a/src/uu/nice/src/nice.rs
+++ b/src/uu/nice/src/nice.rs
@@ -10,21 +10,13 @@
 #[macro_use]
 extern crate uucore;
 
-use libc::{c_char, c_int, execvp};
+use libc::{c_char, c_int, execvp, PRIO_PROCESS};
 use std::ffi::CString;
 use std::io::Error;
 use std::ptr;
 
 use clap::{crate_version, App, AppSettings, Arg};
 
-// XXX: PRIO_PROCESS is 0 on at least FreeBSD and Linux.  Don't know about Mac OS X.
-const PRIO_PROCESS: c_int = 0;
-
-extern "C" {
-    fn getpriority(which: c_int, who: c_int) -> c_int;
-    fn setpriority(which: c_int, who: c_int, prio: c_int) -> c_int;
-}
-
 pub mod options {
     pub static ADJUSTMENT: &str = "adjustment";
     pub static COMMAND: &str = "COMMAND";
@@ -50,7 +42,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     let mut niceness = unsafe {
         nix::errno::Errno::clear();
-        getpriority(PRIO_PROCESS, 0)
+        libc::getpriority(PRIO_PROCESS, 0)
     };
     if Error::last_os_error().raw_os_error().unwrap() != 0 {
         show_error!("getpriority: {}", Error::last_os_error());
@@ -84,7 +76,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     };
 
     niceness += adjustment;
-    if unsafe { setpriority(PRIO_PROCESS, 0, niceness) } == -1 {
+    if unsafe { libc::setpriority(PRIO_PROCESS, 0, niceness) } == -1 {
         show_warning!("setpriority: {}", Error::last_os_error());
     }
 

From 0edc9b01b9a801ef60f86ad97dc31deeb08f5c0e Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Mon, 9 Aug 2021 16:03:23 +0200
Subject: [PATCH 15/29] factor: prevent writing incomplete lines

This makes it possible to execute multiple `factor` instances that write
to the same output in parallel, without having them interfere.
---
 src/uu/factor/src/cli.rs     | 25 ++++++++++++++--------
 tests/by-util/test_factor.rs | 40 ++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/src/uu/factor/src/cli.rs b/src/uu/factor/src/cli.rs
index 0f5d21362..7963f162f 100644
--- a/src/uu/factor/src/cli.rs
+++ b/src/uu/factor/src/cli.rs
@@ -10,6 +10,7 @@
 extern crate uucore;
 
 use std::error::Error;
+use std::fmt::Write as FmtWrite;
 use std::io::{self, stdin, stdout, BufRead, Write};
 
 mod factor;
@@ -28,21 +29,29 @@ mod options {
     pub static NUMBER: &str = "NUMBER";
 }
 
-fn print_factors_str(num_str: &str, w: &mut impl io::Write) -> Result<(), Box<dyn Error>> {
-    num_str
-        .parse::<u64>()
-        .map_err(|e| e.into())
-        .and_then(|x| writeln!(w, "{}:{}", x, factor(x)).map_err(|e| e.into()))
+fn print_factors_str(
+    num_str: &str,
+    w: &mut io::BufWriter<impl io::Write>,
+    factors_buffer: &mut String,
+) -> Result<(), Box<dyn Error>> {
+    num_str.parse::<u64>().map_err(|e| e.into()).and_then(|x| {
+        factors_buffer.clear();
+        writeln!(factors_buffer, "{}:{}", x, factor(x))?;
+        w.write_all(factors_buffer.as_bytes())?;
+        Ok(())
+    })
 }
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let matches = uu_app().get_matches_from(args);
     let stdout = stdout();
-    let mut w = io::BufWriter::new(stdout.lock());
+    // We use a smaller buffer here to pass a gnu test. 4KiB appears to be the default pipe size for bash.
+    let mut w = io::BufWriter::with_capacity(4 * 1024, stdout.lock());
+    let mut factors_buffer = String::new();
 
     if let Some(values) = matches.values_of(options::NUMBER) {
         for number in values {
-            if let Err(e) = print_factors_str(number, &mut w) {
+            if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) {
                 show_warning!("{}: {}", number, e);
             }
         }
@@ -51,7 +60,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
         for line in stdin.lock().lines() {
             for number in line.unwrap().split_whitespace() {
-                if let Err(e) = print_factors_str(number, &mut w) {
+                if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) {
                     show_warning!("{}: {}", number, e);
                 }
             }
diff --git a/tests/by-util/test_factor.rs b/tests/by-util/test_factor.rs
index a3f06ea8a..bcb1238bf 100644
--- a/tests/by-util/test_factor.rs
+++ b/tests/by-util/test_factor.rs
@@ -8,7 +8,10 @@
 
 // spell-checker:ignore (methods) hexdigest
 
+use tempfile::TempDir;
+
 use crate::common::util::*;
+use std::fs::OpenOptions;
 use std::time::SystemTime;
 
 #[path = "../../src/uu/factor/sieve.rs"]
@@ -24,6 +27,43 @@ use self::sieve::Sieve;
 const NUM_PRIMES: usize = 10000;
 const NUM_TESTS: usize = 100;
 
+#[test]
+fn test_parallel() {
+    // factor should only flush the buffer at line breaks
+    let n_integers = 100_000;
+    let mut input_string = String::new();
+    for i in 0..=n_integers {
+        input_string.push_str(&(format!("{} ", i))[..]);
+    }
+
+    let tmp_dir = TempDir::new().unwrap();
+    let tmp_dir = AtPath::new(tmp_dir.path());
+    tmp_dir.touch("output");
+    let output = OpenOptions::new()
+        .append(true)
+        .open(tmp_dir.plus("output"))
+        .unwrap();
+
+    for mut child in (0..10)
+        .map(|_| {
+            new_ucmd!()
+                .set_stdout(output.try_clone().unwrap())
+                .pipe_in(input_string.clone())
+                .run_no_wait()
+        })
+        .collect::<Vec<_>>()
+    {
+        assert_eq!(child.wait().unwrap().code().unwrap(), 0);
+    }
+
+    let result = TestScenario::new(util_name!())
+        .ccmd("sort")
+        .arg(tmp_dir.plus("output"))
+        .succeeds();
+    let hash_check = sha1::Sha1::from(result.stdout()).hexdigest();
+    assert_eq!(hash_check, "cc743607c0ff300ff575d92f4ff0c87d5660c393");
+}
+
 #[test]
 fn test_first_100000_integers() {
     extern crate sha1;

From 16492171165efd9ade748291a5ae8e1fad7c4921 Mon Sep 17 00:00:00 2001
From: Terts Diepraam <terts.diepraam@gmail.com>
Date: Tue, 10 Aug 2021 15:37:57 +0200
Subject: [PATCH 16/29] uucore: remove distinction between common and custom
 errors

As custom errors are prefered over wrapping around common errors, the
distinction between UCommonError and UCustomError is removed. This
reduces the number of types and makes the error handling easier to
understand.
---
 src/uu/chown/src/chown.rs        |   4 +-
 src/uu/df/src/df.rs              |   4 +-
 src/uu/du/src/du.rs              |   4 +-
 src/uu/false/src/false.rs        |   5 +-
 src/uu/install/src/install.rs    |   4 +-
 src/uu/ln/src/ln.rs              |   4 +-
 src/uu/ls/src/ls.rs              |   4 +-
 src/uu/mktemp/src/mktemp.rs      |   4 +-
 src/uu/sort/src/sort.rs          |   4 +-
 src/uu/touch/src/touch.rs        |   2 +-
 src/uucore/src/lib/mods/error.rs | 297 +++++++------------------------
 11 files changed, 84 insertions(+), 252 deletions(-)

diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs
index 53cf61e83..4a9da3f77 100644
--- a/src/uu/chown/src/chown.rs
+++ b/src/uu/chown/src/chown.rs
@@ -14,7 +14,7 @@ use uucore::fs::resolve_relative_path;
 use uucore::libc::{gid_t, uid_t};
 use uucore::perms::{wrap_chown, Verbosity};
 
-use uucore::error::{FromIo, UError, UResult, USimpleError};
+use uucore::error::{FromIo, UResult, USimpleError};
 
 use clap::{crate_version, App, Arg};
 
@@ -324,7 +324,7 @@ impl Chowner {
             ret |= self.traverse(f);
         }
         if ret != 0 {
-            return Err(UError::from(ret));
+            return Err(ret.into());
         }
         Ok(())
     }
diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs
index baa5fe292..cdfdf0b2d 100644
--- a/src/uu/df/src/df.rs
+++ b/src/uu/df/src/df.rs
@@ -8,7 +8,7 @@
 
 #[macro_use]
 extern crate uucore;
-use uucore::error::UCustomError;
+use uucore::error::UError;
 use uucore::error::UResult;
 #[cfg(unix)]
 use uucore::fsext::statfs_fn;
@@ -274,7 +274,7 @@ impl Display for DfError {
 
 impl Error for DfError {}
 
-impl UCustomError for DfError {
+impl UError for DfError {
     fn code(&self) -> i32 {
         match self {
             DfError::InvalidBaseValue(_) => 1,
diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs
index 9c05eb982..d85cc941c 100644
--- a/src/uu/du/src/du.rs
+++ b/src/uu/du/src/du.rs
@@ -32,7 +32,7 @@ use std::path::PathBuf;
 use std::str::FromStr;
 use std::time::{Duration, UNIX_EPOCH};
 use std::{error::Error, fmt::Display};
-use uucore::error::{UCustomError, UResult};
+use uucore::error::{UError, UResult};
 use uucore::parse_size::{parse_size, ParseSizeError};
 use uucore::InvalidEncodingHandling;
 #[cfg(windows)]
@@ -438,7 +438,7 @@ Try '{} --help' for more information.",
 
 impl Error for DuError {}
 
-impl UCustomError for DuError {
+impl UError for DuError {
     fn code(&self) -> i32 {
         match self {
             Self::InvalidMaxDepthArg(_) => 1,
diff --git a/src/uu/false/src/false.rs b/src/uu/false/src/false.rs
index 2d64c8376..170788898 100644
--- a/src/uu/false/src/false.rs
+++ b/src/uu/false/src/false.rs
@@ -9,13 +9,12 @@
 extern crate uucore;
 
 use clap::App;
-use uucore::error::{UError, UResult};
-use uucore::executable;
+use uucore::{error::UResult, executable};
 
 #[uucore_procs::gen_uumain]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     uu_app().get_matches_from(args);
-    Err(UError::from(1))
+    Err(1.into())
 }
 
 pub fn uu_app() -> App<'static, 'static> {
diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs
index 8ce219f7a..d7b18c895 100644
--- a/src/uu/install/src/install.rs
+++ b/src/uu/install/src/install.rs
@@ -17,7 +17,7 @@ use file_diff::diff;
 use filetime::{set_file_times, FileTime};
 use uucore::backup_control::{self, BackupMode};
 use uucore::entries::{grp2gid, usr2uid};
-use uucore::error::{FromIo, UCustomError, UIoError, UResult, USimpleError};
+use uucore::error::{FromIo, UError, UIoError, UResult, USimpleError};
 use uucore::perms::{wrap_chgrp, wrap_chown, Verbosity};
 
 use libc::{getegid, geteuid};
@@ -66,7 +66,7 @@ enum InstallError {
     OmittingDirectory(PathBuf),
 }
 
-impl UCustomError for InstallError {
+impl UError for InstallError {
     fn code(&self) -> i32 {
         match self {
             InstallError::Unimplemented(_) => 2,
diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs
index 65bdcf36c..7010ff5e4 100644
--- a/src/uu/ln/src/ln.rs
+++ b/src/uu/ln/src/ln.rs
@@ -11,7 +11,7 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
-use uucore::error::{UCustomError, UResult};
+use uucore::error::{UError, UResult};
 
 use std::borrow::Cow;
 use std::error::Error;
@@ -79,7 +79,7 @@ impl Display for LnError {
 
 impl Error for LnError {}
 
-impl UCustomError for LnError {
+impl UError for LnError {
     fn code(&self) -> i32 {
         match self {
             Self::TargetIsDirectory(_) => 1,
diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index ef314dfa8..b8b18bc69 100644
--- a/src/uu/ls/src/ls.rs
+++ b/src/uu/ls/src/ls.rs
@@ -39,7 +39,7 @@ use std::{
     time::Duration,
 };
 use term_grid::{Cell, Direction, Filling, Grid, GridOptions};
-use uucore::error::{set_exit_code, FromIo, UCustomError, UResult};
+use uucore::error::{set_exit_code, FromIo, UError, UResult};
 
 use unicode_width::UnicodeWidthStr;
 #[cfg(unix)]
@@ -133,7 +133,7 @@ enum LsError {
     NoMetadata(PathBuf),
 }
 
-impl UCustomError for LsError {
+impl UError for LsError {
     fn code(&self) -> i32 {
         match self {
             LsError::InvalidLineWidth(_) => 2,
diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs
index 8a4b472aa..e1dd604a0 100644
--- a/src/uu/mktemp/src/mktemp.rs
+++ b/src/uu/mktemp/src/mktemp.rs
@@ -12,7 +12,7 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
-use uucore::error::{FromIo, UCustomError, UResult};
+use uucore::error::{FromIo, UError, UResult};
 
 use std::env;
 use std::error::Error;
@@ -49,7 +49,7 @@ enum MkTempError {
     InvalidTemplate(String),
 }
 
-impl UCustomError for MkTempError {}
+impl UError for MkTempError {}
 
 impl Error for MkTempError {}
 
diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs
index ae869ba49..3eebe7c4a 100644
--- a/src/uu/sort/src/sort.rs
+++ b/src/uu/sort/src/sort.rs
@@ -45,7 +45,7 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::str::Utf8Error;
 use unicode_width::UnicodeWidthStr;
-use uucore::error::{set_exit_code, UCustomError, UResult, USimpleError, UUsageError};
+use uucore::error::{set_exit_code, UError, UResult, USimpleError, UUsageError};
 use uucore::parse_size::{parse_size, ParseSizeError};
 use uucore::version_cmp::version_cmp;
 use uucore::InvalidEncodingHandling;
@@ -164,7 +164,7 @@ enum SortError {
 
 impl Error for SortError {}
 
-impl UCustomError for SortError {
+impl UError for SortError {
     fn code(&self) -> i32 {
         match self {
             SortError::Disorder { .. } => 1,
diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs
index dd2b05d0e..49efa676a 100644
--- a/src/uu/touch/src/touch.rs
+++ b/src/uu/touch/src/touch.rs
@@ -17,7 +17,7 @@ use clap::{crate_version, App, Arg, ArgGroup};
 use filetime::*;
 use std::fs::{self, File};
 use std::path::Path;
-use uucore::error::{FromIo, UResult, USimpleError};
+use uucore::error::{FromIo, UError, UResult, USimpleError};
 
 static ABOUT: &str = "Update the access and modification times of each FILE to the current time.";
 pub mod options {
diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs
index c6120672f..fbeea4ed2 100644
--- a/src/uucore/src/lib/mods/error.rs
+++ b/src/uucore/src/lib/mods/error.rs
@@ -6,11 +6,11 @@
 //! This module provides types to reconcile these exit codes with idiomatic Rust error
 //! handling. This has a couple advantages over manually using [`std::process::exit`]:
 //! 1. It enables the use of `?`, `map_err`, `unwrap_or`, etc. in `uumain`.
-//! 1. It encourages the use of `UResult`/`Result` in functions in the utils.
+//! 1. It encourages the use of [`UResult`]/[`Result`] in functions in the utils.
 //! 1. The error messages are largely standardized across utils.
 //! 1. Standardized error messages can be created from external result types
 //!    (i.e. [`std::io::Result`] & `clap::ClapResult`).
-//! 1. `set_exit_code` takes away the burden of manually tracking exit codes for non-fatal errors.
+//! 1. [`set_exit_code`] takes away the burden of manually tracking exit codes for non-fatal errors.
 //!
 //! # Usage
 //! The signature of a typical util should be:
@@ -19,7 +19,7 @@
 //!     ...
 //! }
 //! ```
-//! [`UResult`] is a simple wrapper around [`Result`] with a custom error type: [`UError`]. The
+//! [`UResult`] is a simple wrapper around [`Result`] with a custom error trait: [`UError`]. The
 //! most important difference with types implementing [`std::error::Error`] is that [`UError`]s
 //! can specify the exit code of the program when they are returned from `uumain`:
 //! * When `Ok` is returned, the code set with [`set_exit_code`] is used as exit code. If
@@ -41,8 +41,8 @@
 //! [`set_exit_code`]. See the documentation on that function for more information.
 //!
 //! # Guidelines
-//! * Use common errors where possible.
-//! * Add variants to [`UCommonError`] if an error appears in multiple utils.
+//! * Use error types from `uucore` where possible.
+//! * Add error types to `uucore` if an error appears in multiple utils.
 //! * Prefer proper custom error types over [`ExitCode`] and [`USimpleError`].
 //! * [`USimpleError`] may be used in small utils with simple error handling.
 //! * Using [`ExitCode`] is not recommended but can be useful for converting utils to use
@@ -87,115 +87,10 @@ pub fn set_exit_code(code: i32) {
     EXIT_CODE.store(code, Ordering::SeqCst);
 }
 
-/// Should be returned by all utils.
-///
-/// Two additional methods are implemented on [`UResult`] on top of the normal [`Result`] methods:
-/// `map_err_code` & `map_err_code_message`.
-///
-/// These methods are used to convert [`UCommonError`]s into errors with a custom error code and
-/// message.
-pub type UResult<T> = Result<T, UError>;
+/// Result type that should be returned by all utils.
+pub type UResult<T> = Result<T, Box<dyn UError>>;
 
-trait UResultTrait<T> {
-    fn map_err_code(self, mapper: fn(&UCommonError) -> Option<i32>) -> Self;
-    fn map_err_code_and_message(self, mapper: fn(&UCommonError) -> Option<(i32, String)>) -> Self;
-}
-
-impl<T> UResultTrait<T> for UResult<T> {
-    fn map_err_code(self, mapper: fn(&UCommonError) -> Option<i32>) -> Self {
-        if let Err(UError::Common(error)) = self {
-            if let Some(code) = mapper(&error) {
-                Err(UCommonErrorWithCode { code, error }.into())
-            } else {
-                Err(error.into())
-            }
-        } else {
-            self
-        }
-    }
-
-    fn map_err_code_and_message(self, mapper: fn(&UCommonError) -> Option<(i32, String)>) -> Self {
-        if let Err(UError::Common(ref error)) = self {
-            if let Some((code, message)) = mapper(error) {
-                return Err(USimpleError { code, message }.into());
-            }
-        }
-        self
-    }
-}
-
-/// The error type of [`UResult`].
-///
-/// `UError::Common` errors are defined in [`uucore`](crate) while `UError::Custom` errors are
-/// defined by the utils.
-/// ```
-/// use uucore::error::USimpleError;
-/// let err = USimpleError::new(1, "Error!!".into());
-/// assert_eq!(1, err.code());
-/// assert_eq!(String::from("Error!!"), format!("{}", err));
-/// ```
-pub enum UError {
-    Common(UCommonError),
-    Custom(Box<dyn UCustomError>),
-}
-
-impl UError {
-    /// The error code of [`UResult`]
-    ///
-    /// This function defines the error code associated with an instance of
-    /// [`UResult`]. To associate error codes for self-defined instances of
-    /// `UResult::Custom` (i.e. [`UCustomError`]), implement the
-    /// [`code`-function there](UCustomError::code).
-    pub fn code(&self) -> i32 {
-        match self {
-            UError::Common(e) => e.code(),
-            UError::Custom(e) => e.code(),
-        }
-    }
-
-    /// Whether to print usage help for a [`UResult`]
-    ///
-    /// Defines if a variant of [`UResult`] should print a short usage message
-    /// below the error. The usage message is printed when this function returns
-    /// `true`. To do this for self-defined instances of `UResult::Custom` (i.e.
-    /// [`UCustomError`]), implement the [`usage`-function
-    /// there](UCustomError::usage).
-    pub fn usage(&self) -> bool {
-        match self {
-            UError::Common(e) => e.usage(),
-            UError::Custom(e) => e.usage(),
-        }
-    }
-}
-
-impl From<UCommonError> for UError {
-    fn from(v: UCommonError) -> Self {
-        UError::Common(v)
-    }
-}
-
-impl From<i32> for UError {
-    fn from(v: i32) -> Self {
-        UError::Custom(Box::new(ExitCode(v)))
-    }
-}
-
-impl<E: UCustomError + 'static> From<E> for UError {
-    fn from(v: E) -> Self {
-        UError::Custom(Box::new(v) as Box<dyn UCustomError>)
-    }
-}
-
-impl Display for UError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        match self {
-            UError::Common(e) => e.fmt(f),
-            UError::Custom(e) => e.fmt(f),
-        }
-    }
-}
-
-/// Custom errors defined by the utils.
+/// Custom errors defined by the utils and `uucore`.
 ///
 /// All errors should implement [`std::error::Error`], [`std::fmt::Display`] and
 /// [`std::fmt::Debug`] and have an additional `code` method that specifies the
@@ -204,7 +99,7 @@ impl Display for UError {
 /// An example of a custom error from `ls`:
 ///
 /// ```
-/// use uucore::error::{UCustomError, UResult};
+/// use uucore::error::{UError, UResult};
 /// use std::{
 ///     error::Error,
 ///     fmt::{Display, Debug},
@@ -217,7 +112,7 @@ impl Display for UError {
 ///     NoMetadata(PathBuf),
 /// }
 ///
-/// impl UCustomError for LsError {
+/// impl UError for LsError {
 ///     fn code(&self) -> i32 {
 ///         match self {
 ///             LsError::InvalidLineWidth(_) => 2,
@@ -248,12 +143,12 @@ impl Display for UError {
 /// }
 /// ```
 ///
-/// The call to `into()` is required to convert the [`UCustomError`] to an
-/// instance of [`UError`].
+/// The call to `into()` is required to convert the `LsError` to
+/// [`Box<dyn UError>`]. The implementation for `From` is provided automatically.
 ///
 /// A crate like [`quick_error`](https://crates.io/crates/quick-error) might
 /// also be used, but will still require an `impl` for the `code` method.
-pub trait UCustomError: Error + Send {
+pub trait UError: Error + Send {
     /// Error code of a custom error.
     ///
     /// Set a return value for each variant of an enum-type to associate an
@@ -263,7 +158,7 @@ pub trait UCustomError: Error + Send {
     /// # Example
     ///
     /// ```
-    /// use uucore::error::{UCustomError};
+    /// use uucore::error::{UError};
     /// use std::{
     ///     error::Error,
     ///     fmt::{Display, Debug},
@@ -277,7 +172,7 @@ pub trait UCustomError: Error + Send {
     ///     Bing(),
     /// }
     ///
-    /// impl UCustomError for MyError {
+    /// impl UError for MyError {
     ///     fn code(&self) -> i32 {
     ///         match self {
     ///             MyError::Foo(_) => 2,
@@ -314,7 +209,7 @@ pub trait UCustomError: Error + Send {
     /// # Example
     ///
     /// ```
-    /// use uucore::error::{UCustomError};
+    /// use uucore::error::{UError};
     /// use std::{
     ///     error::Error,
     ///     fmt::{Display, Debug},
@@ -328,7 +223,7 @@ pub trait UCustomError: Error + Send {
     ///     Bing(),
     /// }
     ///
-    /// impl UCustomError for MyError {
+    /// impl UError for MyError {
     ///     fn usage(&self) -> bool {
     ///         match self {
     ///             // This will have a short usage help appended
@@ -357,41 +252,17 @@ pub trait UCustomError: Error + Send {
     }
 }
 
-impl From<Box<dyn UCustomError>> for i32 {
-    fn from(e: Box<dyn UCustomError>) -> i32 {
-        e.code()
+impl<T> From<T> for Box<dyn UError>
+where
+    T: UError + 'static,
+{
+    fn from(t: T) -> Box<dyn UError> {
+        Box::new(t)
     }
 }
 
-/// A [`UCommonError`] with an overridden exit code.
+/// A simple error type with an exit code and a message that implements [`UError`].
 ///
-/// This exit code is returned instead of the default exit code for the [`UCommonError`]. This is
-/// typically created with the either the `UResult::map_err_code` or `UCommonError::with_code`
-/// method.
-#[derive(Debug)]
-pub struct UCommonErrorWithCode {
-    code: i32,
-    error: UCommonError,
-}
-
-impl Error for UCommonErrorWithCode {}
-
-impl Display for UCommonErrorWithCode {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
-        self.error.fmt(f)
-    }
-}
-
-impl UCustomError for UCommonErrorWithCode {
-    fn code(&self) -> i32 {
-        self.code
-    }
-}
-
-/// A simple error type with an exit code and a message that implements [`UCustomError`].
-///
-/// It is typically created with the `UResult::map_err_code_and_message` method. Alternatively, it
-/// can be constructed by manually:
 /// ```
 /// use uucore::error::{UResult, USimpleError};
 /// let err = USimpleError { code: 1, message: "error!".into()};
@@ -407,8 +278,8 @@ pub struct USimpleError {
 
 impl USimpleError {
     #[allow(clippy::new_ret_no_self)]
-    pub fn new(code: i32, message: String) -> UError {
-        UError::Custom(Box::new(Self { code, message }))
+    pub fn new(code: i32, message: String) -> Box<dyn UError> {
+        Box::new(Self { code, message })
     }
 }
 
@@ -420,7 +291,7 @@ impl Display for USimpleError {
     }
 }
 
-impl UCustomError for USimpleError {
+impl UError for USimpleError {
     fn code(&self) -> i32 {
         self.code
     }
@@ -434,8 +305,8 @@ pub struct UUsageError {
 
 impl UUsageError {
     #[allow(clippy::new_ret_no_self)]
-    pub fn new(code: i32, message: String) -> UError {
-        UError::Custom(Box::new(Self { code, message }))
+    pub fn new(code: i32, message: String) -> Box<dyn UError> {
+        Box::new(Self { code, message })
     }
 }
 
@@ -447,7 +318,7 @@ impl Display for UUsageError {
     }
 }
 
-impl UCustomError for UUsageError {
+impl UError for UUsageError {
     fn code(&self) -> i32 {
         self.code
     }
@@ -465,13 +336,13 @@ impl UCustomError for UUsageError {
 /// There are two ways to construct this type: with [`UIoError::new`] or by calling the
 /// [`FromIo::map_err_context`] method on a [`std::io::Result`] or [`std::io::Error`].
 /// ```
-/// use uucore::error::{FromIo, UResult, UIoError, UCommonError};
+/// use uucore::error::{FromIo, UResult, UIoError, UError};
 /// use std::fs::File;
 /// use std::path::Path;
 /// let path = Path::new("test.txt");
 ///
 /// // Manual construction
-/// let e: UIoError = UIoError::new(
+/// let e: Box<dyn UError> = UIoError::new(
 ///     std::io::ErrorKind::NotFound,
 ///     format!("cannot access '{}'", path.display())
 /// );
@@ -487,18 +358,21 @@ pub struct UIoError {
 }
 
 impl UIoError {
-    pub fn new(kind: std::io::ErrorKind, context: String) -> Self {
-        Self {
+    #[allow(clippy::new_ret_no_self)]
+    pub fn new(kind: std::io::ErrorKind, context: String) -> Box<dyn UError> {
+        Box::new(Self {
             context,
             inner: std::io::Error::new(kind, ""),
-        }
+        })
     }
+}
 
-    pub fn code(&self) -> i32 {
+impl UError for UIoError {
+    fn code(&self) -> i32 {
         1
     }
 
-    pub fn usage(&self) -> bool {
+    fn usage(&self) -> bool {
         false
     }
 }
@@ -537,46 +411,33 @@ impl Display for UIoError {
     }
 }
 
-/// Enables the conversion from `std::io::Error` to `UError` and from `std::io::Result` to
-/// `UResult`.
+/// Enables the conversion from [`std::io::Error`] to [`UError`] and from [`std::io::Result`] to
+/// [`UResult`].
 pub trait FromIo<T> {
     fn map_err_context(self, context: impl FnOnce() -> String) -> T;
 }
 
-impl FromIo<UIoError> for std::io::Error {
-    fn map_err_context(self, context: impl FnOnce() -> String) -> UIoError {
-        UIoError {
+impl FromIo<Box<UIoError>> for std::io::Error {
+    fn map_err_context(self, context: impl FnOnce() -> String) -> Box<UIoError> {
+        Box::new(UIoError {
             context: (context)(),
             inner: self,
-        }
+        })
     }
 }
 
 impl<T> FromIo<UResult<T>> for std::io::Result<T> {
     fn map_err_context(self, context: impl FnOnce() -> String) -> UResult<T> {
-        self.map_err(|e| UError::Common(UCommonError::Io(e.map_err_context(context))))
+        self.map_err(|e| e.map_err_context(context) as Box<dyn UError>)
     }
 }
 
-impl FromIo<UIoError> for std::io::ErrorKind {
-    fn map_err_context(self, context: impl FnOnce() -> String) -> UIoError {
-        UIoError {
+impl FromIo<Box<UIoError>> for std::io::ErrorKind {
+    fn map_err_context(self, context: impl FnOnce() -> String) -> Box<UIoError> {
+        Box::new(UIoError {
             context: (context)(),
             inner: std::io::Error::new(self, ""),
-        }
-    }
-}
-
-impl From<UIoError> for UCommonError {
-    fn from(e: UIoError) -> UCommonError {
-        UCommonError::Io(e)
-    }
-}
-
-impl From<UIoError> for UError {
-    fn from(e: UIoError) -> UError {
-        let common: UCommonError = e.into();
-        common.into()
+        })
     }
 }
 
@@ -647,47 +508,6 @@ macro_rules! uio_error(
     })
 );
 
-/// Common errors for utilities.
-///
-/// If identical errors appear across multiple utilities, they should be added here.
-#[derive(Debug)]
-pub enum UCommonError {
-    Io(UIoError),
-    // Clap(UClapError),
-}
-
-impl UCommonError {
-    pub fn with_code(self, code: i32) -> UCommonErrorWithCode {
-        UCommonErrorWithCode { code, error: self }
-    }
-
-    pub fn code(&self) -> i32 {
-        1
-    }
-
-    pub fn usage(&self) -> bool {
-        false
-    }
-}
-
-impl From<UCommonError> for i32 {
-    fn from(common: UCommonError) -> i32 {
-        match common {
-            UCommonError::Io(e) => e.code(),
-        }
-    }
-}
-
-impl Error for UCommonError {}
-
-impl Display for UCommonError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
-        match self {
-            UCommonError::Io(e) => e.fmt(f),
-        }
-    }
-}
-
 /// A special error type that does not print any message when returned from
 /// `uumain`. Especially useful for porting utilities to using [`UResult`].
 ///
@@ -705,6 +525,13 @@ impl Display for UCommonError {
 #[derive(Debug)]
 pub struct ExitCode(pub i32);
 
+impl ExitCode {
+    #[allow(clippy::new_ret_no_self)]
+    pub fn new(code: i32) -> Box<dyn UError> {
+        Box::new(Self(code))
+    }
+}
+
 impl Error for ExitCode {}
 
 impl Display for ExitCode {
@@ -713,8 +540,14 @@ impl Display for ExitCode {
     }
 }
 
-impl UCustomError for ExitCode {
+impl UError for ExitCode {
     fn code(&self) -> i32 {
         self.0
     }
 }
+
+impl From<i32> for Box<dyn UError> {
+    fn from(i: i32) -> Self {
+        ExitCode::new(i)
+    }
+}

From d12d4f760c09a75b6c4ea12dd180a1bfb92aba38 Mon Sep 17 00:00:00 2001
From: Terts Diepraam <terts.diepraam@gmail.com>
Date: Tue, 10 Aug 2021 16:50:42 +0200
Subject: [PATCH 17/29] uucore: {USimpleError, UUsageError}::new take
 Into<String> instead of String

---
 src/uu/chown/src/chown.rs        |  2 +-
 src/uu/dirname/src/dirname.rs    |  2 +-
 src/uu/id/src/id.rs              |  8 ++++----
 src/uu/sort/src/sort.rs          |  4 ++--
 src/uucore/src/lib/mods/error.rs | 14 +++++++-------
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs
index 4a9da3f77..3d882a564 100644
--- a/src/uu/chown/src/chown.rs
+++ b/src/uu/chown/src/chown.rs
@@ -109,7 +109,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             if derefer == 1 {
                 return Err(USimpleError::new(
                     1,
-                    "-R --dereference requires -H or -L".to_string(),
+                    "-R --dereference requires -H or -L",
                 ));
             }
             derefer = 0;
diff --git a/src/uu/dirname/src/dirname.rs b/src/uu/dirname/src/dirname.rs
index 63ee57272..e7dcc2195 100644
--- a/src/uu/dirname/src/dirname.rs
+++ b/src/uu/dirname/src/dirname.rs
@@ -79,7 +79,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             print!("{}", separator);
         }
     } else {
-        return Err(UUsageError::new(1, "missing operand".to_string()));
+        return Err(UUsageError::new(1, "missing operand"));
     }
 
     Ok(())
diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs
index 1dd1b176d..b067be42c 100644
--- a/src/uu/id/src/id.rs
+++ b/src/uu/id/src/id.rs
@@ -173,20 +173,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     if (state.nflag || state.rflag) && default_format && !state.cflag {
         return Err(USimpleError::new(
             1,
-            "cannot print only names or real IDs in default format".to_string(),
+            "cannot print only names or real IDs in default format",
         ));
     }
     if state.zflag && default_format && !state.cflag {
         // NOTE: GNU test suite "id/zero.sh" needs this stderr output:
         return Err(USimpleError::new(
             1,
-            "option --zero not permitted in default format".to_string(),
+            "option --zero not permitted in default format",
         ));
     }
     if state.user_specified && state.cflag {
         return Err(USimpleError::new(
             1,
-            "cannot print security context when user specified".to_string(),
+            "cannot print security context when user specified",
         ));
     }
 
@@ -220,7 +220,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         } else {
             return Err(USimpleError::new(
                 1,
-                "--context (-Z) works only on an SELinux-enabled kernel".to_string(),
+                "--context (-Z) works only on an SELinux-enabled kernel",
             ));
         }
     }
diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs
index 3eebe7c4a..e0b445782 100644
--- a/src/uu/sort/src/sort.rs
+++ b/src/uu/sort/src/sort.rs
@@ -1238,7 +1238,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         if separator.len() != 1 {
             return Err(UUsageError::new(
                 2,
-                "separator must be exactly one character long".into(),
+                "separator must be exactly one character long",
             ));
         }
         settings.separator = Some(separator.chars().next().unwrap())
@@ -1517,7 +1517,7 @@ fn exec(
         file_merger.write_all(settings, output)
     } else if settings.check {
         if files.len() > 1 {
-            Err(UUsageError::new(2, "only one file allowed with -c".into()))
+            Err(UUsageError::new(2, "only one file allowed with -c"))
         } else {
             check::check(files.first().unwrap(), settings)
         }
diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs
index fbeea4ed2..caccb6ac0 100644
--- a/src/uucore/src/lib/mods/error.rs
+++ b/src/uucore/src/lib/mods/error.rs
@@ -268,7 +268,7 @@ where
 /// let err = USimpleError { code: 1, message: "error!".into()};
 /// let res: UResult<()> = Err(err.into());
 /// // or using the `new` method:
-/// let res: UResult<()> = Err(USimpleError::new(1, "error!".into()));
+/// let res: UResult<()> = Err(USimpleError::new(1, "error!"));
 /// ```
 #[derive(Debug)]
 pub struct USimpleError {
@@ -278,8 +278,8 @@ pub struct USimpleError {
 
 impl USimpleError {
     #[allow(clippy::new_ret_no_self)]
-    pub fn new(code: i32, message: String) -> Box<dyn UError> {
-        Box::new(Self { code, message })
+    pub fn new<S: Into<String>>(code: i32, message: S) -> Box<dyn UError> {
+        Box::new(Self { code, message: message.into() })
     }
 }
 
@@ -305,8 +305,8 @@ pub struct UUsageError {
 
 impl UUsageError {
     #[allow(clippy::new_ret_no_self)]
-    pub fn new(code: i32, message: String) -> Box<dyn UError> {
-        Box::new(Self { code, message })
+    pub fn new<S: Into<String>>(code: i32, message: S) -> Box<dyn UError> {
+        Box::new(Self { code, message: message.into() })
     }
 }
 
@@ -359,9 +359,9 @@ pub struct UIoError {
 
 impl UIoError {
     #[allow(clippy::new_ret_no_self)]
-    pub fn new(kind: std::io::ErrorKind, context: String) -> Box<dyn UError> {
+    pub fn new<S: Into<String>>(kind: std::io::ErrorKind, context: S) -> Box<dyn UError> {
         Box::new(Self {
-            context,
+            context: context.into(),
             inner: std::io::Error::new(kind, ""),
         })
     }

From ea3c15f0dd815d76cb084cdd59e5fec3cc6e95d2 Mon Sep 17 00:00:00 2001
From: Terts Diepraam <terts.diepraam@gmail.com>
Date: Tue, 10 Aug 2021 17:00:16 +0200
Subject: [PATCH 18/29] uucore: use default UError impl for UIoError

---
 src/uucore/src/lib/mods/error.rs | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs
index caccb6ac0..c22b434c4 100644
--- a/src/uucore/src/lib/mods/error.rs
+++ b/src/uucore/src/lib/mods/error.rs
@@ -367,15 +367,7 @@ impl UIoError {
     }
 }
 
-impl UError for UIoError {
-    fn code(&self) -> i32 {
-        1
-    }
-
-    fn usage(&self) -> bool {
-        false
-    }
-}
+impl UError for UIoError {}
 
 impl Error for UIoError {}
 

From 14459cf611baf2d4d2c87dfbadb1837ea069434d Mon Sep 17 00:00:00 2001
From: Terts Diepraam <terts.diepraam@gmail.com>
Date: Tue, 10 Aug 2021 17:56:41 +0200
Subject: [PATCH 19/29] rustfmt

---
 src/uu/chown/src/chown.rs        |  5 +----
 src/uucore/src/lib/mods/error.rs | 10 ++++++++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs
index 3d882a564..7df263c5d 100644
--- a/src/uu/chown/src/chown.rs
+++ b/src/uu/chown/src/chown.rs
@@ -107,10 +107,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     if recursive {
         if bit_flag == FTS_PHYSICAL {
             if derefer == 1 {
-                return Err(USimpleError::new(
-                    1,
-                    "-R --dereference requires -H or -L",
-                ));
+                return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
             }
             derefer = 0;
         }
diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs
index c22b434c4..664fc9841 100644
--- a/src/uucore/src/lib/mods/error.rs
+++ b/src/uucore/src/lib/mods/error.rs
@@ -279,7 +279,10 @@ pub struct USimpleError {
 impl USimpleError {
     #[allow(clippy::new_ret_no_self)]
     pub fn new<S: Into<String>>(code: i32, message: S) -> Box<dyn UError> {
-        Box::new(Self { code, message: message.into() })
+        Box::new(Self {
+            code,
+            message: message.into(),
+        })
     }
 }
 
@@ -306,7 +309,10 @@ pub struct UUsageError {
 impl UUsageError {
     #[allow(clippy::new_ret_no_self)]
     pub fn new<S: Into<String>>(code: i32, message: S) -> Box<dyn UError> {
-        Box::new(Self { code, message: message.into() })
+        Box::new(Self {
+            code,
+            message: message.into(),
+        })
     }
 }
 

From 83a515e4c3aab8c529526db059d5df606fffa872 Mon Sep 17 00:00:00 2001
From: Koutheir Attouchi <koutheir@gmail.com>
Date: Fri, 6 Aug 2021 09:52:34 -0400
Subject: [PATCH 20/29] chcon: reduce the number of unsafe blocks.

---
 Cargo.lock                 |  10 +-
 Cargo.toml                 |   2 +-
 src/uu/chcon/Cargo.toml    |   2 +-
 src/uu/chcon/src/chcon.rs  | 699 +++++++------------------------------
 src/uu/chcon/src/errors.rs |  71 ++++
 src/uu/chcon/src/fts.rs    | 193 ++++++++++
 src/uu/id/Cargo.toml       |   2 +-
 src/uu/id/src/id.rs        |   2 -
 8 files changed, 407 insertions(+), 574 deletions(-)
 create mode 100644 src/uu/chcon/src/errors.rs
 create mode 100644 src/uu/chcon/src/fts.rs

diff --git a/Cargo.lock b/Cargo.lock
index 9fe45b941..8cf7cddcb 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -713,9 +713,9 @@ checksum = "31a7a908b8f32538a2143e59a6e4e2508988832d5d4d6f7c156b3cbc762643a5"
 
 [[package]]
 name = "filetime"
-version = "0.2.14"
+version = "0.2.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1d34cfa13a63ae058bfa601fe9e313bbdb3746427c1459185464ce0fcf62e1e8"
+checksum = "975ccf83d8d9d0d84682850a38c8169027be83368805971cc4f238c2b245bc98"
 dependencies = [
  "cfg-if 1.0.0",
  "libc",
@@ -1631,9 +1631,9 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
 
 [[package]]
 name = "selinux"
-version = "0.1.3"
+version = "0.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bd525eeb189eb26c8471463186bba87644e3d8a9c7ae392adaf9ec45ede574bc"
+checksum = "1aa2f705dd871c2eb90888bb2d44b13218b34f5c7318c3971df62f799d0143eb"
 dependencies = [
  "bitflags",
  "libc",
@@ -2033,7 +2033,7 @@ dependencies = [
  "clap",
  "fts-sys",
  "libc",
- "selinux-sys",
+ "selinux",
  "thiserror",
  "uucore",
  "uucore_procs",
diff --git a/Cargo.toml b/Cargo.toml
index df60206ca..abf6a42de 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -241,7 +241,7 @@ clap = { version = "2.33", features = ["wrap_help"] }
 lazy_static = { version="1.3" }
 textwrap = { version="=0.11.0", features=["term_size"] } # !maint: [2020-05-10; rivy] unstable crate using undocumented features; pinned currently, will review
 uucore = { version=">=0.0.9", package="uucore", path="src/uucore" }
-selinux = { version="0.1.3", optional = true }
+selinux = { version="0.2.1", optional = true }
 # * uutils
 uu_test  = { optional=true, version="0.0.7", package="uu_test", path="src/uu/test" }
 #
diff --git a/src/uu/chcon/Cargo.toml b/src/uu/chcon/Cargo.toml
index 7e7f82c3b..56fbef9ba 100644
--- a/src/uu/chcon/Cargo.toml
+++ b/src/uu/chcon/Cargo.toml
@@ -17,7 +17,7 @@ path = "src/chcon.rs"
 clap         = { version = "2.33", features = ["wrap_help"] }
 uucore       = { version = ">=0.0.9", package="uucore", path="../../uucore", features=["entries", "fs", "perms"] }
 uucore_procs = { version = ">=0.0.6", package="uucore_procs", path="../../uucore_procs" }
-selinux-sys  = { version = "0.5"   }
+selinux      = { version = "0.2"   }
 fts-sys      = { version = "0.2"   }
 thiserror    = { version = "1.0"   }
 libc         = { version = "0.2"   }
diff --git a/src/uu/chcon/src/chcon.rs b/src/uu/chcon/src/chcon.rs
index 0e8b4b440..2a5efba46 100644
--- a/src/uu/chcon/src/chcon.rs
+++ b/src/uu/chcon/src/chcon.rs
@@ -5,14 +5,18 @@
 use uucore::{executable, show_error, show_usage_error, show_warning};
 
 use clap::{App, Arg};
+use selinux::{OpaqueSecurityContext, SecurityContext};
 
+use std::borrow::Cow;
 use std::ffi::{CStr, CString, OsStr, OsString};
-use std::fmt::Write;
-use std::os::raw::{c_char, c_int};
+use std::os::raw::c_int;
 use std::path::{Path, PathBuf};
-use std::{fs, io, ptr, slice};
+use std::{fs, io};
 
-type Result<T> = std::result::Result<T, Error>;
+mod errors;
+mod fts;
+
+use errors::*;
 
 static VERSION: &str = env!("CARGO_PKG_VERSION");
 static ABOUT: &str = "Change the SELinux security context of each FILE to CONTEXT. \n\
@@ -81,15 +85,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     let context = match &options.mode {
         CommandLineMode::ReferenceBased { reference } => {
-            let result = selinux::FileContext::new(reference, true)
-                .and_then(|r| {
-                    if r.is_empty() {
-                        Err(io::Error::from_raw_os_error(libc::ENODATA))
-                    } else {
-                        Ok(r)
-                    }
-                })
-                .map_err(|r| Error::io1("Getting security context", reference, r));
+            let result = match SecurityContext::of_path(reference, true, false) {
+                Ok(Some(context)) => Ok(context),
+
+                Ok(None) => {
+                    let err = io::Error::from_raw_os_error(libc::ENODATA);
+                    Err(Error::from_io1("Getting security context", reference, err))
+                }
+
+                Err(r) => Err(Error::from_selinux("Getting security context", r)),
+            };
 
             match result {
                 Err(r) => {
@@ -102,33 +107,24 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         }
 
         CommandLineMode::ContextBased { context } => {
-            match selinux::SecurityContext::security_check_context(context)
-                .map_err(|r| Error::io1("Checking security context", context, r))
-            {
-                Err(r) => {
-                    show_error!("{}.", report_full_error(&r));
-                    return libc::EXIT_FAILURE;
-                }
+            let c_context = match os_str_to_c_string(context) {
+                Ok(context) => context,
 
-                Ok(Some(false)) => {
+                Err(_r) => {
                     show_error!("Invalid security context '{}'.", context.to_string_lossy());
                     return libc::EXIT_FAILURE;
                 }
-
-                Ok(Some(true)) | Ok(None) => {}
-            }
-
-            let c_context = if let Ok(value) = os_str_to_c_string(context) {
-                value
-            } else {
-                show_error!("Invalid security context '{}'.", context.to_string_lossy());
-                return libc::EXIT_FAILURE;
             };
 
-            SELinuxSecurityContext::String(c_context)
+            if SecurityContext::from_c_str(&c_context, false).check() == Some(false) {
+                show_error!("Invalid security context '{}'.", context.to_string_lossy());
+                return libc::EXIT_FAILURE;
+            }
+
+            SELinuxSecurityContext::String(Some(c_context))
         }
 
-        CommandLineMode::Custom { .. } => SELinuxSecurityContext::default(),
+        CommandLineMode::Custom { .. } => SELinuxSecurityContext::String(None),
     };
 
     let root_dev_ino = if options.preserve_root && options.recursive_mode.is_recursive() {
@@ -282,65 +278,6 @@ pub fn uu_app() -> App<'static, 'static> {
         .arg(Arg::with_name("FILE").multiple(true).min_values(1))
 }
 
-fn report_full_error(mut err: &dyn std::error::Error) -> String {
-    let mut desc = String::with_capacity(256);
-    write!(&mut desc, "{}", err).unwrap();
-    while let Some(source) = err.source() {
-        err = source;
-        write!(&mut desc, ". {}", err).unwrap();
-    }
-    desc
-}
-
-#[derive(thiserror::Error, Debug)]
-enum Error {
-    #[error("No context is specified")]
-    MissingContext,
-
-    #[error("No files are specified")]
-    MissingFiles,
-
-    #[error("{0}")]
-    ArgumentsMismatch(String),
-
-    #[error(transparent)]
-    CommandLine(#[from] clap::Error),
-
-    #[error("{operation} failed")]
-    Io {
-        operation: &'static str,
-        source: io::Error,
-    },
-
-    #[error("{operation} failed on '{}'", .operand1.to_string_lossy())]
-    Io1 {
-        operation: &'static str,
-        operand1: OsString,
-        source: io::Error,
-    },
-}
-
-impl Error {
-    fn io1(operation: &'static str, operand1: impl Into<OsString>, source: io::Error) -> Self {
-        Self::Io1 {
-            operation,
-            operand1: operand1.into(),
-            source,
-        }
-    }
-
-    #[cfg(unix)]
-    fn io1_c_str(operation: &'static str, operand1: &CStr, source: io::Error) -> Self {
-        if operand1.to_bytes().is_empty() {
-            Self::Io { operation, source }
-        } else {
-            use std::os::unix::ffi::OsStrExt;
-
-            Self::io1(operation, OsStr::from_bytes(operand1.to_bytes()), source)
-        }
-    }
-}
-
 #[derive(Debug)]
 struct Options {
     verbose: bool,
@@ -500,39 +437,29 @@ fn process_files(
     root_dev_ino: Option<(libc::ino_t, libc::dev_t)>,
 ) -> Vec<Error> {
     let fts_options = options.recursive_mode.fts_open_options();
-    let mut fts = match fts::FTS::new(options.files.iter(), fts_options, None) {
+    let mut fts = match fts::FTS::new(options.files.iter(), fts_options) {
         Ok(fts) => fts,
-        Err(source) => {
-            return vec![Error::Io {
-                operation: "fts_open()",
-                source,
-            }]
-        }
+        Err(err) => return vec![err],
     };
 
-    let mut results = vec![];
-
+    let mut errors = Vec::default();
     loop {
         match fts.read_next_entry() {
             Ok(true) => {
                 if let Err(err) = process_file(options, context, &mut fts, root_dev_ino) {
-                    results.push(err);
+                    errors.push(err);
                 }
             }
 
             Ok(false) => break,
 
-            Err(source) => {
-                results.push(Error::Io {
-                    operation: "fts_read()",
-                    source,
-                });
-
+            Err(err) => {
+                errors.push(err);
                 break;
             }
         }
     }
-    results
+    errors
 }
 
 fn process_file(
@@ -541,46 +468,40 @@ fn process_file(
     fts: &mut fts::FTS,
     root_dev_ino: Option<(libc::ino_t, libc::dev_t)>,
 ) -> Result<()> {
-    let entry = fts.last_entry_mut().unwrap();
+    let mut entry = fts.last_entry_ref().unwrap();
 
-    let file_full_name = if entry.fts_path.is_null() {
-        None
-    } else {
-        let fts_path_size = usize::from(entry.fts_pathlen).saturating_add(1);
-
-        // SAFETY: `entry.fts_path` is a non-null pointer that is assumed to be valid.
-        let bytes = unsafe { slice::from_raw_parts(entry.fts_path.cast(), fts_path_size) };
-        CStr::from_bytes_with_nul(bytes).ok()
-    }
-    .ok_or_else(|| Error::Io {
-        operation: "File name validation",
-        source: io::ErrorKind::InvalidInput.into(),
+    let file_full_name = entry.path().map(PathBuf::from).ok_or_else(|| {
+        Error::from_io("File name validation", io::ErrorKind::InvalidInput.into())
     })?;
 
-    let fts_access_path = ptr_to_c_str(entry.fts_accpath)
-        .map_err(|r| Error::io1_c_str("File name validation", file_full_name, r))?;
+    let fts_access_path = entry.access_path().ok_or_else(|| {
+        let err = io::ErrorKind::InvalidInput.into();
+        Error::from_io1("File name validation", &file_full_name, err)
+    })?;
 
-    let err = |s, k: io::ErrorKind| Error::io1_c_str(s, file_full_name, k.into());
+    let err = |s, k: io::ErrorKind| Error::from_io1(s, &file_full_name, k.into());
 
     let fts_err = |s| {
-        let r = io::Error::from_raw_os_error(entry.fts_errno);
-        Err(Error::io1_c_str(s, file_full_name, r))
+        let r = io::Error::from_raw_os_error(entry.errno());
+        Err(Error::from_io1(s, &file_full_name, r))
     };
 
     // SAFETY: If `entry.fts_statp` is not null, then is is assumed to be valid.
-    let file_dev_ino = unsafe { entry.fts_statp.as_ref() }
-        .map(|stat| (stat.st_ino, stat.st_dev))
-        .ok_or_else(|| err("Getting meta data", io::ErrorKind::InvalidInput))?;
+    let file_dev_ino = if let Some(stat) = entry.stat() {
+        (stat.st_ino, stat.st_dev)
+    } else {
+        return Err(err("Getting meta data", io::ErrorKind::InvalidInput));
+    };
 
     let mut result = Ok(());
 
-    match c_int::from(entry.fts_info) {
+    match entry.flags() {
         fts_sys::FTS_D => {
             if options.recursive_mode.is_recursive() {
                 if root_dev_ino_check(root_dev_ino, file_dev_ino) {
                     // This happens e.g., with "chcon -R --preserve-root ... /"
                     // and with "chcon -RH --preserve-root ... symlink-to-root".
-                    root_dev_ino_warn(file_full_name);
+                    root_dev_ino_warn(&file_full_name);
 
                     // Tell fts not to traverse into this hierarchy.
                     let _ignored = fts.set(fts_sys::FTS_SKIP);
@@ -607,8 +528,8 @@ fn process_file(
             // that modify permissions, it is possible that the file in question is accessible when
             // control reaches this point. So, if this is the first time we've seen the FTS_NS for
             // this file, tell fts_read to stat it "again".
-            if entry.fts_level == 0 && entry.fts_number == 0 {
-                entry.fts_number = 1;
+            if entry.level() == 0 && entry.number() == 0 {
+                entry.set_number(1);
                 let _ignored = fts.set(fts_sys::FTS_AGAIN);
                 return Ok(());
             }
@@ -621,8 +542,8 @@ fn process_file(
         fts_sys::FTS_DNR => result = fts_err("Reading directory"),
 
         fts_sys::FTS_DC => {
-            if cycle_warning_required(options.recursive_mode.fts_open_options(), entry) {
-                emit_cycle_warning(file_full_name);
+            if cycle_warning_required(options.recursive_mode.fts_open_options(), &entry) {
+                emit_cycle_warning(&file_full_name);
                 return Err(err("Reading cyclic directory", io::ErrorKind::InvalidData));
             }
         }
@@ -630,11 +551,11 @@ fn process_file(
         _ => {}
     }
 
-    if c_int::from(entry.fts_info) == fts_sys::FTS_DP
+    if entry.flags() == fts_sys::FTS_DP
         && result.is_ok()
         && root_dev_ino_check(root_dev_ino, file_dev_ino)
     {
-        root_dev_ino_warn(file_full_name);
+        root_dev_ino_warn(&file_full_name);
         result = Err(err("Modifying root path", io::ErrorKind::PermissionDenied));
     }
 
@@ -656,24 +577,10 @@ fn process_file(
     result
 }
 
-fn set_file_security_context(
-    path: &Path,
-    context: *const c_char,
-    follow_symbolic_links: bool,
-) -> Result<()> {
-    let mut file_context = selinux::FileContext::from_ptr(context as *mut c_char);
-    if file_context.context.is_null() {
-        Err(io::Error::from(io::ErrorKind::InvalidInput))
-    } else {
-        file_context.set_for_file(path, follow_symbolic_links)
-    }
-    .map_err(|r| Error::io1("Setting security context", path, r))
-}
-
 fn change_file_context(
     options: &Options,
     context: &SELinuxSecurityContext,
-    file: &CStr,
+    path: &Path,
 ) -> Result<()> {
     match &options.mode {
         CommandLineMode::Custom {
@@ -682,91 +589,88 @@ fn change_file_context(
             the_type,
             range,
         } => {
-            let path = PathBuf::from(c_str_to_os_string(file));
-            let file_context = selinux::FileContext::new(&path, options.affect_symlink_referent)
-                .map_err(|r| Error::io1("Getting security context", &path, r))?;
+            let err0 = || -> Result<()> {
+                // If the file doesn't have a context, and we're not setting all of the context
+                // components, there isn't really an obvious default. Thus, we just give up.
+                let op = "Applying partial security context to unlabeled file";
+                let err = io::ErrorKind::InvalidInput.into();
+                Err(Error::from_io1(op, path, err))
+            };
 
-            // If the file doesn't have a context, and we're not setting all of the context
-            // components, there isn't really an obvious default. Thus, we just give up.
-            if file_context.is_empty() {
-                return Err(Error::io1(
-                    "Applying partial security context to unlabeled file",
-                    path,
-                    io::ErrorKind::InvalidInput.into(),
-                ));
-            }
+            let file_context =
+                match SecurityContext::of_path(path, options.affect_symlink_referent, false) {
+                    Ok(Some(context)) => context,
 
-            let mut se_context = selinux::SecurityContext::new(file_context.as_ptr())
-                .map_err(|r| Error::io1("Creating security context", &path, r))?;
+                    Ok(None) => return err0(),
+                    Err(r) => return Err(Error::from_selinux("Getting security context", r)),
+                };
 
-            if let Some(user) = user {
-                se_context
-                    .set_user(user)
-                    .map_err(|r| Error::io1("Setting security context user", &path, r))?;
-            }
+            let c_file_context = match file_context.to_c_string() {
+                Ok(Some(context)) => context,
 
-            if let Some(role) = role {
-                se_context
-                    .set_role(role)
-                    .map_err(|r| Error::io1("Setting security context role", &path, r))?;
-            }
+                Ok(None) => return err0(),
+                Err(r) => return Err(Error::from_selinux("Getting security context", r)),
+            };
 
-            if let Some(the_type) = the_type {
-                se_context
-                    .set_type(the_type)
-                    .map_err(|r| Error::io1("Setting security context type", &path, r))?;
-            }
+            let se_context =
+                OpaqueSecurityContext::from_c_str(c_file_context.as_ref()).map_err(|_r| {
+                    let err = io::ErrorKind::InvalidInput.into();
+                    Error::from_io1("Creating security context", path, err)
+                })?;
 
-            if let Some(range) = range {
-                se_context
-                    .set_range(range)
-                    .map_err(|r| Error::io1("Setting security context range", &path, r))?;
+            type SetValueProc = fn(&OpaqueSecurityContext, &CStr) -> selinux::errors::Result<()>;
+
+            let list: &[(&Option<OsString>, SetValueProc)] = &[
+                (user, OpaqueSecurityContext::set_user),
+                (role, OpaqueSecurityContext::set_role),
+                (the_type, OpaqueSecurityContext::set_type),
+                (range, OpaqueSecurityContext::set_range),
+            ];
+
+            for (new_value, set_value_proc) in list {
+                if let Some(new_value) = new_value {
+                    let c_new_value = os_str_to_c_string(new_value).map_err(|_r| {
+                        let err = io::ErrorKind::InvalidInput.into();
+                        Error::from_io1("Creating security context", path, err)
+                    })?;
+
+                    set_value_proc(&se_context, &c_new_value)
+                        .map_err(|r| Error::from_selinux("Setting security context user", r))?;
+                }
             }
 
             let context_string = se_context
-                .str_bytes()
-                .map_err(|r| Error::io1("Getting security context", &path, r))?;
+                .to_c_string()
+                .map_err(|r| Error::from_selinux("Getting security context", r))?;
 
-            if !file_context.is_empty() && file_context.as_bytes() == context_string {
+            if c_file_context.as_ref().to_bytes() == context_string.as_ref().to_bytes() {
                 Ok(()) // Nothing to change.
             } else {
-                set_file_security_context(
-                    &path,
-                    context_string.as_ptr().cast(),
-                    options.affect_symlink_referent,
-                )
+                SecurityContext::from_c_str(&context_string, false)
+                    .set_for_path(path, options.affect_symlink_referent, false)
+                    .map_err(|r| Error::from_selinux("Setting security context", r))
             }
         }
 
         CommandLineMode::ReferenceBased { .. } | CommandLineMode::ContextBased { .. } => {
-            let path = PathBuf::from(c_str_to_os_string(file));
-            let ctx_ptr = context.as_ptr() as *mut c_char;
-            set_file_security_context(&path, ctx_ptr, options.affect_symlink_referent)
+            if let Some(c_context) = context.to_c_string()? {
+                SecurityContext::from_c_str(c_context.as_ref(), false)
+                    .set_for_path(path, options.affect_symlink_referent, false)
+                    .map_err(|r| Error::from_selinux("Setting security context", r))
+            } else {
+                let err = io::ErrorKind::InvalidInput.into();
+                Err(Error::from_io1("Setting security context", path, err))
+            }
         }
     }
 }
 
 #[cfg(unix)]
-fn c_str_to_os_string(s: &CStr) -> OsString {
-    use std::os::unix::ffi::OsStringExt;
-
-    OsString::from_vec(s.to_bytes().to_vec())
-}
-
-#[cfg(unix)]
-pub(crate) fn os_str_to_c_string(s: &OsStr) -> io::Result<CString> {
+pub(crate) fn os_str_to_c_string(s: &OsStr) -> Result<CString> {
     use std::os::unix::ffi::OsStrExt;
 
-    CString::new(s.as_bytes()).map_err(|_r| io::ErrorKind::InvalidInput.into())
-}
-
-/// SAFETY:
-/// - If `p` is not null, then it is assumed to be a valid null-terminated C string.
-/// - The returned `CStr` must not live more than the data pointed-to by `p`.
-fn ptr_to_c_str<'s>(p: *const c_char) -> io::Result<&'s CStr> {
-    ptr::NonNull::new(p as *mut c_char)
-        .map(|p| unsafe { CStr::from_ptr(p.as_ptr()) })
-        .ok_or_else(|| io::ErrorKind::InvalidInput.into())
+    CString::new(s.as_bytes())
+        .map_err(|_r| Error::from_io("CString::new()", io::ErrorKind::InvalidInput.into()))
 }
 
 /// Call `lstat()` to get the device and inode numbers for `/`.
@@ -776,7 +680,7 @@ fn get_root_dev_ino() -> Result<(libc::ino_t, libc::dev_t)> {
 
     fs::symlink_metadata("/")
         .map(|md| (md.ino(), md.dev()))
-        .map_err(|r| Error::io1("std::fs::symlink_metadata", "/", r))
+        .map_err(|r| Error::from_io1("std::fs::symlink_metadata", "/", r))
 }
 
 fn root_dev_ino_check(
@@ -786,8 +690,8 @@ fn root_dev_ino_check(
     root_dev_ino.map_or(false, |root_dev_ino| root_dev_ino == dir_dev_ino)
 }
 
-fn root_dev_ino_warn(dir_name: &CStr) {
-    if dir_name.to_bytes() == b"/" {
+fn root_dev_ino_warn(dir_name: &Path) {
+    if dir_name.as_os_str() == "/" {
         show_warning!(
             "It is dangerous to operate recursively on '/'. \
              Use --{} to override this failsafe.",
@@ -810,370 +714,37 @@ fn root_dev_ino_warn(dir_name: &CStr) {
 // However, when invoked with "-P -R", it deserves a warning.
 // The fts_options parameter records the options that control this aspect of fts's behavior,
 // so test that.
-fn cycle_warning_required(fts_options: c_int, entry: &fts_sys::FTSENT) -> bool {
+fn cycle_warning_required(fts_options: c_int, entry: &fts::EntryRef) -> bool {
     // When dereferencing no symlinks, or when dereferencing only those listed on the command line
     // and we're not processing a command-line argument, then a cycle is a serious problem.
     ((fts_options & fts_sys::FTS_PHYSICAL) != 0)
-        && (((fts_options & fts_sys::FTS_COMFOLLOW) == 0) || entry.fts_level != 0)
+        && (((fts_options & fts_sys::FTS_COMFOLLOW) == 0) || entry.level() != 0)
 }
 
-fn emit_cycle_warning(file_name: &CStr) {
+fn emit_cycle_warning(file_name: &Path) {
     show_warning!(
         "Circular directory structure.\n\
 This almost certainly means that you have a corrupted file system.\n\
 NOTIFY YOUR SYSTEM MANAGER.\n\
 The following directory is part of the cycle '{}'.",
-        file_name.to_string_lossy()
+        file_name.display()
     )
 }
 
 #[derive(Debug)]
-enum SELinuxSecurityContext {
-    File(selinux::FileContext),
-    String(CString),
+enum SELinuxSecurityContext<'t> {
+    File(SecurityContext<'t>),
+    String(Option<CString>),
 }
 
-impl Default for SELinuxSecurityContext {
-    fn default() -> Self {
-        Self::String(CString::default())
-    }
-}
-
-impl SELinuxSecurityContext {
-    #[cfg(unix)]
-    fn as_ptr(&self) -> *const c_char {
+impl<'t> SELinuxSecurityContext<'t> {
+    fn to_c_string(&self) -> Result<Option<Cow<CStr>>> {
         match self {
-            SELinuxSecurityContext::File(context) => context.as_ptr(),
-            SELinuxSecurityContext::String(context) => context.to_bytes_with_nul().as_ptr().cast(),
-        }
-    }
-}
-
-mod fts {
-    use std::ffi::{CStr, CString, OsStr};
-    use std::os::raw::c_int;
-    use std::{io, iter, ptr};
-
-    use super::os_str_to_c_string;
-
-    pub(crate) type FTSOpenCallBack = unsafe extern "C" fn(
-        arg1: *mut *const fts_sys::FTSENT,
-        arg2: *mut *const fts_sys::FTSENT,
-    ) -> c_int;
-
-    #[derive(Debug)]
-    pub(crate) struct FTS {
-        fts: ptr::NonNull<fts_sys::FTS>,
-        entry: *mut fts_sys::FTSENT,
-    }
-
-    impl FTS {
-        pub(crate) fn new<I>(
-            paths: I,
-            options: c_int,
-            compar: Option<FTSOpenCallBack>,
-        ) -> io::Result<Self>
-        where
-            I: IntoIterator,
-            I::Item: AsRef<OsStr>,
-        {
-            let files_paths = paths
-                .into_iter()
-                .map(|s| os_str_to_c_string(s.as_ref()))
-                .collect::<io::Result<Vec<_>>>()?;
-
-            if files_paths.is_empty() {
-                return Err(io::ErrorKind::InvalidInput.into());
-            }
-
-            let path_argv = files_paths
-                .iter()
-                .map(CString::as_ref)
-                .map(CStr::as_ptr)
-                .chain(iter::once(ptr::null()))
-                .collect::<Vec<_>>();
-
-            // SAFETY: We assume calling fts_open() is safe:
-            // - `path_argv` is an array holding at least one path, and null-terminated.
-            let r = unsafe { fts_sys::fts_open(path_argv.as_ptr().cast(), options, compar) };
-            let fts = ptr::NonNull::new(r).ok_or_else(io::Error::last_os_error)?;
-
-            Ok(Self {
-                fts,
-                entry: ptr::null_mut(),
-            })
-        }
-
-        pub(crate) fn read_next_entry(&mut self) -> io::Result<bool> {
-            // SAFETY: We assume calling fts_read() is safe with a non-null `fts`
-            // pointer assumed to be valid.
-            self.entry = unsafe { fts_sys::fts_read(self.fts.as_ptr()) };
-            if self.entry.is_null() {
-                let r = io::Error::last_os_error();
-                if let Some(0) = r.raw_os_error() {
-                    Ok(false)
-                } else {
-                    Err(r)
-                }
-            } else {
-                Ok(true)
-            }
-        }
-
-        pub(crate) fn last_entry_mut(&mut self) -> Option<&mut fts_sys::FTSENT> {
-            // SAFETY: If `self.entry` is not null, then is is assumed to be valid.
-            unsafe { self.entry.as_mut() }
-        }
-
-        pub(crate) fn set(&mut self, instr: c_int) -> io::Result<()> {
-            let fts = self.fts.as_ptr();
-
-            let entry = self
-                .last_entry_mut()
-                .ok_or_else(|| io::Error::from(io::ErrorKind::UnexpectedEof))?;
-
-            // SAFETY: We assume calling fts_set() is safe with non-null `fts`
-            // and `entry` pointers assumed to be valid.
-            if unsafe { fts_sys::fts_set(fts, entry, instr) } == -1 {
-                Err(io::Error::last_os_error())
-            } else {
-                Ok(())
-            }
-        }
-    }
-
-    impl Drop for FTS {
-        fn drop(&mut self) {
-            // SAFETY: We assume calling fts_close() is safe with a non-null `fts`
-            // pointer assumed to be valid.
-            unsafe { fts_sys::fts_close(self.fts.as_ptr()) };
-        }
-    }
-}
-
-mod selinux {
-    use std::ffi::OsStr;
-    use std::os::raw::c_char;
-    use std::path::Path;
-    use std::{io, ptr, slice};
-
-    use super::os_str_to_c_string;
-
-    #[derive(Debug)]
-    pub(crate) struct SecurityContext(ptr::NonNull<selinux_sys::context_s_t>);
-
-    impl SecurityContext {
-        pub(crate) fn new(context_str: *const c_char) -> io::Result<Self> {
-            if context_str.is_null() {
-                Err(io::ErrorKind::InvalidInput.into())
-            } else {
-                // SAFETY: We assume calling context_new() is safe with
-                // a non-null `context_str` pointer assumed to be valid.
-                let p = unsafe { selinux_sys::context_new(context_str) };
-                ptr::NonNull::new(p)
-                    .ok_or_else(io::Error::last_os_error)
-                    .map(Self)
-            }
-        }
-
-        pub(crate) fn is_selinux_enabled() -> bool {
-            // SAFETY: We assume calling is_selinux_enabled() is always safe.
-            unsafe { selinux_sys::is_selinux_enabled() != 0 }
-        }
-
-        pub(crate) fn security_check_context(context: &OsStr) -> io::Result<Option<bool>> {
-            let c_context = os_str_to_c_string(context)?;
-
-            // SAFETY: We assume calling security_check_context() is safe with
-            // a non-null `context` pointer assumed to be valid.
-            if unsafe { selinux_sys::security_check_context(c_context.as_ptr()) } == 0 {
-                Ok(Some(true))
-            } else if Self::is_selinux_enabled() {
-                Ok(Some(false))
-            } else {
-                Ok(None)
-            }
-        }
-
-        pub(crate) fn str_bytes(&self) -> io::Result<&[u8]> {
-            // SAFETY: We assume calling context_str() is safe with
-            // a non-null `context` pointer assumed to be valid.
-            let p = unsafe { selinux_sys::context_str(self.0.as_ptr()) };
-            if p.is_null() {
-                Err(io::ErrorKind::InvalidInput.into())
-            } else {
-                let len = unsafe { libc::strlen(p.cast()) }.saturating_add(1);
-                Ok(unsafe { slice::from_raw_parts(p.cast(), len) })
-            }
-        }
-
-        pub(crate) fn set_user(&mut self, user: &OsStr) -> io::Result<()> {
-            let c_user = os_str_to_c_string(user)?;
-
-            // SAFETY: We assume calling context_user_set() is safe with non-null
-            // `context` and `user` pointers assumed to be valid.
-            if unsafe { selinux_sys::context_user_set(self.0.as_ptr(), c_user.as_ptr()) } == 0 {
-                Ok(())
-            } else {
-                Err(io::Error::last_os_error())
-            }
-        }
-
-        pub(crate) fn set_role(&mut self, role: &OsStr) -> io::Result<()> {
-            let c_role = os_str_to_c_string(role)?;
-
-            // SAFETY: We assume calling context_role_set() is safe with non-null
-            // `context` and `role` pointers assumed to be valid.
-            if unsafe { selinux_sys::context_role_set(self.0.as_ptr(), c_role.as_ptr()) } == 0 {
-                Ok(())
-            } else {
-                Err(io::Error::last_os_error())
-            }
-        }
-
-        pub(crate) fn set_type(&mut self, the_type: &OsStr) -> io::Result<()> {
-            let c_type = os_str_to_c_string(the_type)?;
-
-            // SAFETY: We assume calling context_type_set() is safe with non-null
-            // `context` and `the_type` pointers assumed to be valid.
-            if unsafe { selinux_sys::context_type_set(self.0.as_ptr(), c_type.as_ptr()) } == 0 {
-                Ok(())
-            } else {
-                Err(io::Error::last_os_error())
-            }
-        }
-
-        pub(crate) fn set_range(&mut self, range: &OsStr) -> io::Result<()> {
-            let c_range = os_str_to_c_string(range)?;
-
-            // SAFETY: We assume calling context_range_set() is safe with non-null
-            // `context` and `range` pointers assumed to be valid.
-            if unsafe { selinux_sys::context_range_set(self.0.as_ptr(), c_range.as_ptr()) } == 0 {
-                Ok(())
-            } else {
-                Err(io::Error::last_os_error())
-            }
-        }
-    }
-
-    impl Drop for SecurityContext {
-        fn drop(&mut self) {
-            // SAFETY: We assume calling context_free() is safe with
-            // a non-null `context` pointer assumed to be valid.
-            unsafe { selinux_sys::context_free(self.0.as_ptr()) }
-        }
-    }
-
-    #[derive(Debug)]
-    pub(crate) struct FileContext {
-        pub context: *mut c_char,
-        pub len: usize,
-        pub allocated: bool,
-    }
-
-    impl FileContext {
-        pub(crate) fn new(path: &Path, follow_symbolic_links: bool) -> io::Result<Self> {
-            let c_path = os_str_to_c_string(path.as_os_str())?;
-            let mut context: *mut c_char = ptr::null_mut();
-
-            // SAFETY: We assume calling getfilecon()/lgetfilecon() is safe with
-            // non-null `path` and `context` pointers assumed to be valid.
-            let len = if follow_symbolic_links {
-                unsafe { selinux_sys::getfilecon(c_path.as_ptr(), &mut context) }
-            } else {
-                unsafe { selinux_sys::lgetfilecon(c_path.as_ptr(), &mut context) }
-            };
-
-            if len == -1 {
-                let err = io::Error::last_os_error();
-                if let Some(libc::ENODATA) = err.raw_os_error() {
-                    Ok(Self::default())
-                } else {
-                    Err(err)
-                }
-            } else if context.is_null() {
-                Ok(Self::default())
-            } else {
-                Ok(Self {
-                    context,
-                    len: len as usize,
-                    allocated: true,
-                })
-            }
-        }
-
-        pub(crate) fn from_ptr(context: *mut c_char) -> Self {
-            if context.is_null() {
-                Self::default()
-            } else {
-                // SAFETY: We assume calling strlen() is safe with a non-null
-                // `context` pointer assumed to be valid.
-                let len = unsafe { libc::strlen(context) };
-                Self {
-                    context,
-                    len,
-                    allocated: false,
-                }
-            }
-        }
-
-        pub(crate) fn set_for_file(
-            &mut self,
-            path: &Path,
-            follow_symbolic_links: bool,
-        ) -> io::Result<()> {
-            let c_path = os_str_to_c_string(path.as_os_str())?;
-
-            // SAFETY: We assume calling setfilecon()/lsetfilecon() is safe with
-            // non-null `path` and `context` pointers assumed to be valid.
-            let r = if follow_symbolic_links {
-                unsafe { selinux_sys::setfilecon(c_path.as_ptr(), self.context) }
-            } else {
-                unsafe { selinux_sys::lsetfilecon(c_path.as_ptr(), self.context) }
-            };
-
-            if r == -1 {
-                Err(io::Error::last_os_error())
-            } else {
-                Ok(())
-            }
-        }
-
-        pub(crate) fn as_ptr(&self) -> *const c_char {
-            self.context
-        }
-
-        pub(crate) fn is_empty(&self) -> bool {
-            self.context.is_null() || self.len == 0
-        }
-
-        pub(crate) fn as_bytes(&self) -> &[u8] {
-            if self.context.is_null() {
-                &[]
-            } else {
-                // SAFETY: `self.0.context` is a non-null pointer that is assumed to be valid.
-                unsafe { slice::from_raw_parts(self.context.cast(), self.len) }
-            }
-        }
-    }
-
-    impl Default for FileContext {
-        fn default() -> Self {
-            Self {
-                context: ptr::null_mut(),
-                len: 0,
-                allocated: false,
-            }
-        }
-    }
-
-    impl Drop for FileContext {
-        fn drop(&mut self) {
-            if self.allocated && !self.context.is_null() {
-                // SAFETY: We assume calling freecon() is safe with a non-null
-                // `context` pointer assumed to be valid.
-                unsafe { selinux_sys::freecon(self.context) }
-            }
+            Self::File(context) => context
+                .to_c_string()
+                .map_err(|r| Error::from_selinux("SELinuxSecurityContext::to_c_string()", r)),
+
+            Self::String(context) => Ok(context.as_deref().map(Cow::Borrowed)),
         }
     }
 }
diff --git a/src/uu/chcon/src/errors.rs b/src/uu/chcon/src/errors.rs
new file mode 100644
index 000000000..ecbd7d409
--- /dev/null
+++ b/src/uu/chcon/src/errors.rs
@@ -0,0 +1,71 @@
+use std::ffi::OsString;
+use std::fmt::Write;
+use std::io;
+
+pub(crate) type Result<T> = std::result::Result<T, Error>;
+
+#[derive(thiserror::Error, Debug)]
+pub(crate) enum Error {
+    #[error("No context is specified")]
+    MissingContext,
+
+    #[error("No files are specified")]
+    MissingFiles,
+
+    #[error("{0}")]
+    ArgumentsMismatch(String),
+
+    #[error(transparent)]
+    CommandLine(#[from] clap::Error),
+
+    #[error("{operation} failed")]
+    SELinux {
+        operation: &'static str,
+        source: selinux::errors::Error,
+    },
+
+    #[error("{operation} failed")]
+    Io {
+        operation: &'static str,
+        source: io::Error,
+    },
+
+    #[error("{operation} failed on '{}'", .operand1.to_string_lossy())]
+    Io1 {
+        operation: &'static str,
+        operand1: OsString,
+        source: io::Error,
+    },
+}
+
+impl Error {
+    pub(crate) fn from_io(operation: &'static str, source: io::Error) -> Self {
+        Self::Io { operation, source }
+    }
+
+    pub(crate) fn from_io1(
+        operation: &'static str,
+        operand1: impl Into<OsString>,
+        source: io::Error,
+    ) -> Self {
+        Self::Io1 {
+            operation,
+            operand1: operand1.into(),
+            source,
+        }
+    }
+
+    pub(crate) fn from_selinux(operation: &'static str, source: selinux::errors::Error) -> Self {
+        Self::SELinux { operation, source }
+    }
+}
+
+pub(crate) fn report_full_error(mut err: &dyn std::error::Error) -> String {
+    let mut desc = String::with_capacity(256);
+    write!(&mut desc, "{}", err).unwrap();
+    while let Some(source) = err.source() {
+        err = source;
+        write!(&mut desc, ". {}", err).unwrap();
+    }
+    desc
+}
diff --git a/src/uu/chcon/src/fts.rs b/src/uu/chcon/src/fts.rs
new file mode 100644
index 000000000..89dd6184d
--- /dev/null
+++ b/src/uu/chcon/src/fts.rs
@@ -0,0 +1,193 @@
+use std::ffi::{CStr, CString, OsStr};
+use std::marker::PhantomData;
+use std::os::raw::{c_int, c_long, c_short};
+use std::path::Path;
+use std::ptr::NonNull;
+use std::{io, iter, ptr, slice};
+
+use crate::errors::{Error, Result};
+use crate::os_str_to_c_string;
+
+#[derive(Debug)]
+pub(crate) struct FTS {
+    fts: ptr::NonNull<fts_sys::FTS>,
+
+    entry: Option<ptr::NonNull<fts_sys::FTSENT>>,
+    _phantom_data: PhantomData<fts_sys::FTSENT>,
+}
+
+impl FTS {
+    pub(crate) fn new<I>(paths: I, options: c_int) -> Result<Self>
+    where
+        I: IntoIterator,
+        I::Item: AsRef<OsStr>,
+    {
+        let files_paths: Vec<CString> = paths
+            .into_iter()
+            .map(|s| os_str_to_c_string(s.as_ref()))
+            .collect::<Result<_>>()?;
+
+        if files_paths.is_empty() {
+            return Err(Error::from_io(
+                "FTS::new()",
+                io::ErrorKind::InvalidInput.into(),
+            ));
+        }
+
+        let path_argv: Vec<_> = files_paths
+            .iter()
+            .map(CString::as_ref)
+            .map(CStr::as_ptr)
+            .chain(iter::once(ptr::null()))
+            .collect();
+
+        // SAFETY: We assume calling fts_open() is safe:
+        // - `path_argv` is an array holding at least one path, and null-terminated.
+        // - `compar` is None.
+        let fts = unsafe { fts_sys::fts_open(path_argv.as_ptr().cast(), options, None) };
+
+        let fts = ptr::NonNull::new(fts)
+            .ok_or_else(|| Error::from_io("fts_open()", io::Error::last_os_error()))?;
+
+        Ok(Self {
+            fts,
+            entry: None,
+            _phantom_data: PhantomData,
+        })
+    }
+
+    pub(crate) fn last_entry_ref(&mut self) -> Option<EntryRef> {
+        self.entry.map(move |entry| EntryRef::new(self, entry))
+    }
+
+    pub(crate) fn read_next_entry(&mut self) -> Result<bool> {
+        // SAFETY: We assume calling fts_read() is safe with a non-null `fts`
+        // pointer assumed to be valid.
+        let new_entry = unsafe { fts_sys::fts_read(self.fts.as_ptr()) };
+
+        self.entry = NonNull::new(new_entry);
+        if self.entry.is_none() {
+            let r = io::Error::last_os_error();
+            if let Some(0) = r.raw_os_error() {
+                Ok(false)
+            } else {
+                Err(Error::from_io("fts_read()", r))
+            }
+        } else {
+            Ok(true)
+        }
+    }
+
+    pub(crate) fn set(&mut self, instr: c_int) -> Result<()> {
+        let fts = self.fts.as_ptr();
+        let entry = self
+            .entry
+            .ok_or_else(|| Error::from_io("FTS::set()", io::ErrorKind::UnexpectedEof.into()))?;
+
+        // SAFETY: We assume calling fts_set() is safe with non-null `fts`
+        // and `entry` pointers assumed to be valid.
+        if unsafe { fts_sys::fts_set(fts, entry.as_ptr(), instr) } == -1 {
+            Err(Error::from_io("fts_set()", io::Error::last_os_error()))
+        } else {
+            Ok(())
+        }
+    }
+}
+
+impl Drop for FTS {
+    fn drop(&mut self) {
+        // SAFETY: We assume calling fts_close() is safe with a non-null `fts`
+        // pointer assumed to be valid.
+        unsafe { fts_sys::fts_close(self.fts.as_ptr()) };
+    }
+}
+
+#[derive(Debug)]
+pub(crate) struct EntryRef<'fts> {
+    pub(crate) pointer: ptr::NonNull<fts_sys::FTSENT>,
+
+    _fts: PhantomData<&'fts FTS>,
+    _phantom_data: PhantomData<fts_sys::FTSENT>,
+}
+
+impl<'fts> EntryRef<'fts> {
+    fn new(_fts: &'fts FTS, entry: ptr::NonNull<fts_sys::FTSENT>) -> Self {
+        Self {
+            pointer: entry,
+            _fts: PhantomData,
+            _phantom_data: PhantomData,
+        }
+    }
+
+    fn as_ref(&self) -> &fts_sys::FTSENT {
+        // SAFETY: `self.pointer` is a non-null pointer that is assumed to be valid.
+        unsafe { self.pointer.as_ref() }
+    }
+
+    fn as_mut(&mut self) -> &mut fts_sys::FTSENT {
+        // SAFETY: `self.pointer` is a non-null pointer that is assumed to be valid.
+        unsafe { self.pointer.as_mut() }
+    }
+
+    pub(crate) fn flags(&self) -> c_int {
+        c_int::from(self.as_ref().fts_info)
+    }
+
+    pub(crate) fn errno(&self) -> c_int {
+        self.as_ref().fts_errno
+    }
+
+    pub(crate) fn level(&self) -> c_short {
+        self.as_ref().fts_level
+    }
+
+    pub(crate) fn number(&self) -> c_long {
+        self.as_ref().fts_number
+    }
+
+    pub(crate) fn set_number(&mut self, new_number: c_long) {
+        self.as_mut().fts_number = new_number;
+    }
+
+    pub(crate) fn path(&self) -> Option<&Path> {
+        let entry = self.as_ref();
+        if entry.fts_pathlen == 0 {
+            return None;
+        }
+
+        NonNull::new(entry.fts_path)
+            .map(|path_ptr| {
+                let path_size = usize::from(entry.fts_pathlen).saturating_add(1);
+
+                // SAFETY: `entry.fts_path` is a non-null pointer that is assumed to be valid.
+                unsafe { slice::from_raw_parts(path_ptr.as_ptr().cast(), path_size) }
+            })
+            .and_then(|bytes| CStr::from_bytes_with_nul(bytes).ok())
+            .map(c_str_to_os_str)
+            .map(Path::new)
+    }
+
+    pub(crate) fn access_path(&self) -> Option<&Path> {
+        ptr::NonNull::new(self.as_ref().fts_accpath)
+            .map(|path_ptr| {
+                // SAFETY: `entry.fts_accpath` is a non-null pointer that is assumed to be valid.
+                unsafe { CStr::from_ptr(path_ptr.as_ptr()) }
+            })
+            .map(c_str_to_os_str)
+            .map(Path::new)
+    }
+
+    pub(crate) fn stat(&self) -> Option<&libc::stat> {
+        ptr::NonNull::new(self.as_ref().fts_statp).map(|stat_ptr| {
+            // SAFETY: `entry.fts_statp` is a non-null pointer that is assumed to be valid.
+            unsafe { stat_ptr.as_ref() }
+        })
+    }
+}
+
+#[cfg(unix)]
+fn c_str_to_os_str(s: &CStr) -> &OsStr {
+    use std::os::unix::ffi::OsStrExt;
+
+    OsStr::from_bytes(s.to_bytes())
+}
diff --git a/src/uu/id/Cargo.toml b/src/uu/id/Cargo.toml
index 518de2729..b58c7fd78 100644
--- a/src/uu/id/Cargo.toml
+++ b/src/uu/id/Cargo.toml
@@ -18,7 +18,7 @@ path = "src/id.rs"
 clap = { version = "2.33", features = ["wrap_help"] }
 uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["entries", "process"] }
 uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" }
-selinux = { version="0.1.3", optional = true }
+selinux = { version="0.2.1", optional = true }
 
 [[bin]]
 name = "id"
diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs
index 1dd1b176d..61684c829 100644
--- a/src/uu/id/src/id.rs
+++ b/src/uu/id/src/id.rs
@@ -40,8 +40,6 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
-#[cfg(all(target_os = "linux", feature = "selinux"))]
-use selinux;
 use std::ffi::CStr;
 use uucore::entries::{self, Group, Locate, Passwd};
 use uucore::error::UResult;

From 81a5f0a4dc7f9a4de7598339d39558171e69940f Mon Sep 17 00:00:00 2001
From: James Robson <jaggededgedjustice@gmail.com>
Date: Thu, 5 Aug 2021 15:44:03 +0100
Subject: [PATCH 21/29] Add step to GnuTest workflow to compare results against
 master

---
 .github/workflows/GnuTests.yml | 13 +++++++++++++
 util/compare_gnu_result.py     | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 util/compare_gnu_result.py

diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml
index 8bf6c091b..fe225d2fa 100644
--- a/.github/workflows/GnuTests.yml
+++ b/.github/workflows/GnuTests.yml
@@ -90,3 +90,16 @@ jobs:
       with:
         name: gnu-result
         path: gnu-result.json
+    - name: Download the result
+      uses: dawidd6/action-download-artifact@v2
+      with:
+        workflow: GnuTests.yml
+        name: gnu-result
+        repo: uutils/coreutils
+        branch: master
+        path: dl
+    - name: Compare against master results
+      shell: bash
+      run: |
+        mv dl/gnu-result.json master-gnu-result.json
+        python uutils/util/compare_gnu_result.py
diff --git a/util/compare_gnu_result.py b/util/compare_gnu_result.py
new file mode 100644
index 000000000..52aa96abe
--- /dev/null
+++ b/util/compare_gnu_result.py
@@ -0,0 +1,32 @@
+#! /usr/bin/python
+
+"""
+Compare the current results to the last results gathered from the master branch to highlight
+if a PR is making the results better/worse
+"""
+
+import json
+import sys
+from os import environ
+
+NEW = json.load(open("gnu-result.json"))
+OLD = json.load(open("master-gnu-result.json"))
+
+# Extract the specific results from the dicts
+last = OLD[list(OLD.keys())[0]]
+current = NEW[list(NEW.keys())[0]]
+
+
+pass_d = int(current["pass"]) - int(last["pass"])
+fail_d = int(current["fail"]) - int(last["fail"])
+error_d = int(current["error"]) - int(last["error"])
+skip_d = int(current["skip"]) - int(last["skip"])
+
+# Get an annotation to highlight changes
+print(
+    f"::warning ::Changes from master: PASS {pass_d:+d} / FAIL {fail_d:+d} / ERROR {error_d:+d} / SKIP {skip_d:+d} "
+)
+
+# If results are worse fail the job to draw attention
+if pass_d < 0:
+    sys.exit(1)

From 882b5ad1f162da14d591c56b1006d7ef26ca5375 Mon Sep 17 00:00:00 2001
From: James Robson <jaggededgedjustice@gmail.com>
Date: Sun, 8 Aug 2021 15:48:38 +0100
Subject: [PATCH 22/29] Display changes in the failing GNU tests

Co-authored-by: Michael Debertol <michael.debertol@gmail.com>
---
 .github/workflows/GnuTests.yml | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml
index fe225d2fa..dad53f20c 100644
--- a/.github/workflows/GnuTests.yml
+++ b/.github/workflows/GnuTests.yml
@@ -98,6 +98,32 @@ jobs:
         repo: uutils/coreutils
         branch: master
         path: dl
+    - name: Download the log
+      uses: dawidd6/action-download-artifact@v2
+      with:
+        workflow: GnuTests.yml
+        name: test-report
+        repo: uutils/coreutils
+        branch: master
+        path: dl
+    - name: Compare failing tests against master
+      shell: bash
+      run: |
+            OLD_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" dl/test-suite.log | sort)
+            NEW_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" gnu/tests/test-suite.log | sort)
+            for LINE in $OLD_FAILING
+            do
+              if ! grep -Fxq $LINE<<<"$NEW_FAILING"; then
+                echo "::warning ::Congrats! The gnu test $LINE is now passing!"
+              fi
+            done
+            for LINE in $NEW_FAILING
+            do
+              if ! grep -Fxq $LINE<<<"$OLD_FAILING"
+              then
+                echo "::error ::GNU test failed: $LINE. $LINE is passing on 'master'. Maybe you have to rebase?"
+              fi
+            done
     - name: Compare against master results
       shell: bash
       run: |

From 1c30fb42d258e28d4335e34a9e0231058ed3bc89 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Tue, 10 Aug 2021 23:31:42 +0200
Subject: [PATCH 23/29] chgrp: handle empty group

---
 src/uu/chgrp/src/chgrp.rs            | 27 +++++++++++++--------------
 src/uu/install/src/install.rs        |  2 +-
 src/uucore/src/lib/features/perms.rs |  3 ++-
 tests/by-util/test_chgrp.rs          |  7 +++++++
 4 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs
index 489be59eb..d351eb2bb 100644
--- a/src/uu/chgrp/src/chgrp.rs
+++ b/src/uu/chgrp/src/chgrp.rs
@@ -161,12 +161,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         Verbosity::Normal
     };
 
-    let dest_gid: u32;
-    if let Some(file) = matches.value_of(options::REFERENCE) {
+    let dest_gid = if let Some(file) = matches.value_of(options::REFERENCE) {
         match fs::metadata(&file) {
-            Ok(meta) => {
-                dest_gid = meta.gid();
-            }
+            Ok(meta) => Some(meta.gid()),
             Err(e) => {
                 show_error!("failed to get attributes of '{}': {}", file, e);
                 return 1;
@@ -174,16 +171,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         }
     } else {
         let group = matches.value_of(options::ARG_GROUP).unwrap_or_default();
-        match entries::grp2gid(group) {
-            Ok(g) => {
-                dest_gid = g;
-            }
-            _ => {
-                show_error!("invalid group: {}", group);
-                return 1;
+        if group.is_empty() {
+            None
+        } else {
+            match entries::grp2gid(group) {
+                Ok(g) => Some(g),
+                _ => {
+                    show_error!("invalid group: {}", group);
+                    return 1;
+                }
             }
         }
-    }
+    };
 
     let executor = Chgrper {
         bit_flag,
@@ -278,7 +277,7 @@ pub fn uu_app() -> App<'static, 'static> {
 }
 
 struct Chgrper {
-    dest_gid: gid_t,
+    dest_gid: Option<gid_t>,
     bit_flag: u8,
     verbosity: Verbosity,
     files: Vec<String>,
diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs
index 8ce219f7a..df273405f 100644
--- a/src/uu/install/src/install.rs
+++ b/src/uu/install/src/install.rs
@@ -660,7 +660,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> {
             Ok(g) => g,
             _ => return Err(InstallError::NoSuchGroup(b.group.clone()).into()),
         };
-        match wrap_chgrp(to, &meta, group_id, false, Verbosity::Normal) {
+        match wrap_chgrp(to, &meta, Some(group_id), false, Verbosity::Normal) {
             Ok(n) => {
                 if !n.is_empty() {
                     show_error!("{}", n);
diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs
index 89c30b53b..69491c297 100644
--- a/src/uucore/src/lib/features/perms.rs
+++ b/src/uucore/src/lib/features/perms.rs
@@ -49,13 +49,14 @@ fn chgrp<P: AsRef<Path>>(path: P, gid: gid_t, follow: bool) -> IOResult<()> {
 pub fn wrap_chgrp<P: AsRef<Path>>(
     path: P,
     meta: &Metadata,
-    dest_gid: gid_t,
+    dest_gid: Option<gid_t>,
     follow: bool,
     verbosity: Verbosity,
 ) -> Result<String, String> {
     use self::Verbosity::*;
     let path = path.as_ref();
     let mut out: String = String::new();
+    let dest_gid = dest_gid.unwrap_or_else(|| meta.gid());
 
     if let Err(e) = chgrp(path, dest_gid, follow) {
         match verbosity {
diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs
index 762e922c4..e887f659d 100644
--- a/tests/by-util/test_chgrp.rs
+++ b/tests/by-util/test_chgrp.rs
@@ -228,3 +228,10 @@ fn test_big_h() {
         );
     }
 }
+
+#[test]
+fn test_no_change() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.touch("file");
+    ucmd.arg("").arg(at.plus("file")).succeeds();
+}

From 13a62489c5db23a1687272a5d59f80cc4556fa92 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Wed, 11 Aug 2021 14:32:46 +0200
Subject: [PATCH 24/29] ls: correct fallbacks for terminal width

If options::WIDTH is not given, we should try to use the terminal width.
If that is unavailable, we should fall back to the 'COLUMNS' environment variable.
If that is unavailable (or invalid), we should fall back to a default of 80.
---
 src/uu/ls/src/ls.rs      | 38 ++++++++++++------
 tests/by-util/test_ls.rs | 87 +++++++++++-----------------------------
 2 files changed, 49 insertions(+), 76 deletions(-)

diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index ef314dfa8..44a374dbe 100644
--- a/src/uu/ls/src/ls.rs
+++ b/src/uu/ls/src/ls.rs
@@ -127,6 +127,8 @@ pub mod options {
     pub static IGNORE: &str = "ignore";
 }
 
+const DEFAULT_TERM_WIDTH: u16 = 80;
+
 #[derive(Debug)]
 enum LsError {
     InvalidLineWidth(String),
@@ -229,7 +231,7 @@ struct Config {
     inode: bool,
     color: Option<LsColors>,
     long: LongFormat,
-    width: Option<u16>,
+    width: u16,
     quoting_style: QuotingStyle,
     indicator_style: IndicatorStyle,
     time_style: TimeStyle,
@@ -399,10 +401,25 @@ impl Config {
 
         let width = match options.value_of(options::WIDTH) {
             Some(x) => match x.parse::<u16>() {
-                Ok(u) => Some(u),
+                Ok(u) => u,
                 Err(_) => return Err(LsError::InvalidLineWidth(x.into()).into()),
             },
-            None => termsize::get().map(|s| s.cols),
+            None => match termsize::get() {
+                Some(size) => size.cols,
+                None => match std::env::var("COLUMNS") {
+                    Ok(columns) => match columns.parse() {
+                        Ok(columns) => columns,
+                        Err(_) => {
+                            show_error!(
+                                "ignoring invalid width in environment variable COLUMNS: '{}'",
+                                columns
+                            );
+                            DEFAULT_TERM_WIDTH
+                        }
+                    },
+                    Err(_) => DEFAULT_TERM_WIDTH,
+                },
+            },
         };
 
         #[allow(clippy::needless_bool)]
@@ -1411,15 +1428,10 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
     } else {
         let names = items.iter().filter_map(|i| display_file_name(i, config));
 
-        match (&config.format, config.width) {
-            (Format::Columns, Some(width)) => {
-                display_grid(names, width, Direction::TopToBottom, out)
-            }
-            (Format::Across, Some(width)) => {
-                display_grid(names, width, Direction::LeftToRight, out)
-            }
-            (Format::Commas, width_opt) => {
-                let term_width = width_opt.unwrap_or(1);
+        match config.format {
+            Format::Columns => display_grid(names, config.width, Direction::TopToBottom, out),
+            Format::Across => display_grid(names, config.width, Direction::LeftToRight, out),
+            Format::Commas => {
                 let mut current_col = 0;
                 let mut names = names;
                 if let Some(name) = names.next() {
@@ -1428,7 +1440,7 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
                 }
                 for name in names {
                     let name_width = name.width as u16;
-                    if current_col + name_width + 1 > term_width {
+                    if current_col + name_width + 1 > config.width {
                         current_col = name_width + 2;
                         let _ = write!(out, ",\n{}", name.contents);
                     } else {
diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs
index 44d14c304..0a19a44fa 100644
--- a/tests/by-util/test_ls.rs
+++ b/tests/by-util/test_ls.rs
@@ -184,16 +184,10 @@ fn test_ls_columns() {
     // Columns is the default
     let result = scene.ucmd().succeeds();
 
-    #[cfg(not(windows))]
-    result.stdout_only("test-columns-1\ntest-columns-2\ntest-columns-3\ntest-columns-4\n");
-    #[cfg(windows)]
     result.stdout_only("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n");
 
     for option in &["-C", "--format=columns"] {
         let result = scene.ucmd().arg(option).succeeds();
-        #[cfg(not(windows))]
-        result.stdout_only("test-columns-1\ntest-columns-2\ntest-columns-3\ntest-columns-4\n");
-        #[cfg(windows)]
         result.stdout_only("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n");
     }
 
@@ -205,6 +199,22 @@ fn test_ls_columns() {
             .succeeds()
             .stdout_only("test-columns-1  test-columns-3\ntest-columns-2  test-columns-4\n");
     }
+
+    for option in &["-C", "--format=columns"] {
+        scene
+            .ucmd()
+            .env("COLUMNS", "40")
+            .arg(option)
+            .succeeds()
+            .stdout_only("test-columns-1  test-columns-3\ntest-columns-2  test-columns-4\n");
+    }
+
+    scene
+        .ucmd()
+        .env("COLUMNS", "garbage")
+        .succeeds()
+        .stdout_is("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n")
+        .stderr_is("ls: ignoring invalid width in environment variable COLUMNS: 'garbage'");
 }
 
 #[test]
@@ -220,11 +230,7 @@ fn test_ls_across() {
         let result = scene.ucmd().arg(option).succeeds();
         // Because the test terminal has width 0, this is the same output as
         // the columns option.
-        if cfg!(unix) {
-            result.stdout_only("test-across-1\ntest-across-2\ntest-across-3\ntest-across-4\n");
-        } else {
-            result.stdout_only("test-across-1  test-across-2  test-across-3  test-across-4\n");
-        }
+        result.stdout_only("test-across-1  test-across-2  test-across-3  test-across-4\n");
     }
 
     for option in &["-x", "--format=across"] {
@@ -250,11 +256,7 @@ fn test_ls_commas() {
 
     for option in &["-m", "--format=commas"] {
         let result = scene.ucmd().arg(option).succeeds();
-        if cfg!(unix) {
-            result.stdout_only("test-commas-1,\ntest-commas-2,\ntest-commas-3,\ntest-commas-4\n");
-        } else {
-            result.stdout_only("test-commas-1, test-commas-2, test-commas-3, test-commas-4\n");
-        }
+        result.stdout_only("test-commas-1, test-commas-2, test-commas-3, test-commas-4\n");
     }
 
     for option in &["-m", "--format=commas"] {
@@ -571,13 +573,11 @@ fn test_ls_sort_name() {
     at.touch("test-1");
     at.touch("test-2");
 
-    let sep = if cfg!(unix) { "\n" } else { "  " };
-
     scene
         .ucmd()
         .arg("--sort=name")
         .succeeds()
-        .stdout_is(["test-1", "test-2", "test-3\n"].join(sep));
+        .stdout_is("test-1  test-2  test-3\n");
 
     let scene_dot = TestScenario::new(util_name!());
     let at = &scene_dot.fixtures;
@@ -591,7 +591,7 @@ fn test_ls_sort_name() {
         .arg("--sort=name")
         .arg("-A")
         .succeeds()
-        .stdout_is([".a", ".b", "a", "b\n"].join(sep));
+        .stdout_is(".a  .b  a  b\n");
 }
 
 #[test]
@@ -612,27 +612,15 @@ fn test_ls_order_size() {
     scene.ucmd().arg("-al").succeeds();
 
     let result = scene.ucmd().arg("-S").succeeds();
-    #[cfg(not(windows))]
-    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-    #[cfg(windows)]
     result.stdout_only("test-4  test-3  test-2  test-1\n");
 
     let result = scene.ucmd().arg("-S").arg("-r").succeeds();
-    #[cfg(not(windows))]
-    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
-    #[cfg(windows)]
     result.stdout_only("test-1  test-2  test-3  test-4\n");
 
     let result = scene.ucmd().arg("--sort=size").succeeds();
-    #[cfg(not(windows))]
-    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-    #[cfg(windows)]
     result.stdout_only("test-4  test-3  test-2  test-1\n");
 
     let result = scene.ucmd().arg("--sort=size").arg("-r").succeeds();
-    #[cfg(not(windows))]
-    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
-    #[cfg(windows)]
     result.stdout_only("test-1  test-2  test-3  test-4\n");
 }
 
@@ -755,9 +743,6 @@ fn test_ls_styles() {
 
     at.touch("test2");
     let result = scene.ucmd().arg("--full-time").arg("-x").succeeds();
-    #[cfg(not(windows))]
-    assert_eq!(result.stdout_str(), "test\ntest2\n");
-    #[cfg(windows)]
     assert_eq!(result.stdout_str(), "test  test2\n");
 }
 
@@ -794,27 +779,15 @@ fn test_ls_order_time() {
 
     // ctime was changed at write, so the order is 4 3 2 1
     let result = scene.ucmd().arg("-t").succeeds();
-    #[cfg(not(windows))]
-    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-    #[cfg(windows)]
     result.stdout_only("test-4  test-3  test-2  test-1\n");
 
     let result = scene.ucmd().arg("--sort=time").succeeds();
-    #[cfg(not(windows))]
-    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-    #[cfg(windows)]
     result.stdout_only("test-4  test-3  test-2  test-1\n");
 
     let result = scene.ucmd().arg("-tr").succeeds();
-    #[cfg(not(windows))]
-    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
-    #[cfg(windows)]
     result.stdout_only("test-1  test-2  test-3  test-4\n");
 
     let result = scene.ucmd().arg("--sort=time").arg("-r").succeeds();
-    #[cfg(not(windows))]
-    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
-    #[cfg(windows)]
     result.stdout_only("test-1  test-2  test-3  test-4\n");
 
     // 3 was accessed last in the read
@@ -826,19 +799,11 @@ fn test_ls_order_time() {
 
         // It seems to be dependent on the platform whether the access time is actually set
         if file3_access > file4_access {
-            if cfg!(not(windows)) {
-                result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n");
-            } else {
-                result.stdout_only("test-3  test-4  test-2  test-1\n");
-            }
+            result.stdout_only("test-3  test-4  test-2  test-1\n");
         } else {
             // Access time does not seem to be set on Windows and some other
             // systems so the order is 4 3 2 1
-            if cfg!(not(windows)) {
-                result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-            } else {
-                result.stdout_only("test-4  test-3  test-2  test-1\n");
-            }
+            result.stdout_only("test-4  test-3  test-2  test-1\n");
         }
     }
 
@@ -847,7 +812,7 @@ fn test_ls_order_time() {
     #[cfg(unix)]
     {
         let result = scene.ucmd().arg("-tc").succeeds();
-        result.stdout_only("test-2\ntest-4\ntest-3\ntest-1\n");
+        result.stdout_only("test-2  test-4  test-3  test-1\n");
     }
 }
 
@@ -2009,11 +1974,7 @@ fn test_ls_path() {
     };
     scene.ucmd().arg(&abs_path).run().stdout_is(expected_stdout);
 
-    let expected_stdout = if cfg!(windows) {
-        format!("{}  {}\n", path, file1)
-    } else {
-        format!("{}\n{}\n", path, file1)
-    };
+    let expected_stdout = format!("{}  {}\n", path, file1);
     scene
         .ucmd()
         .arg(file1)

From 988cc49d4a171e76c46e9ea25a4daf37c32196b1 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Wed, 11 Aug 2021 15:53:39 +0200
Subject: [PATCH 25/29] ls: print a single line when width is set to 0

This means that we treat a width=0 as infinite width.
---
 src/uu/ls/src/ls.rs      | 48 +++++++++++++++++++++++++-------------
 tests/by-util/test_ls.rs | 50 +++++++++++++++++++++++++---------------
 2 files changed, 64 insertions(+), 34 deletions(-)

diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index 44a374dbe..8183f3bb8 100644
--- a/src/uu/ls/src/ls.rs
+++ b/src/uu/ls/src/ls.rs
@@ -1440,7 +1440,8 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
                 }
                 for name in names {
                     let name_width = name.width as u16;
-                    if current_col + name_width + 1 > config.width {
+                    // If the width is 0 we print one single line
+                    if config.width != 0 && current_col + name_width + 1 > config.width {
                         current_col = name_width + 2;
                         let _ = write!(out, ",\n{}", name.contents);
                     } else {
@@ -1492,22 +1493,37 @@ fn display_grid(
     direction: Direction,
     out: &mut BufWriter<Stdout>,
 ) {
-    let mut grid = Grid::new(GridOptions {
-        filling: Filling::Spaces(2),
-        direction,
-    });
-
-    for name in names {
-        grid.add(name);
-    }
-
-    match grid.fit_into_width(width as usize) {
-        Some(output) => {
-            let _ = write!(out, "{}", output);
+    if width == 0 {
+        // If the width is 0 we print one single line
+        let mut printed_something = false;
+        for name in names {
+            if printed_something {
+                let _ = write!(out, "  ");
+            }
+            printed_something = true;
+            let _ = write!(out, "{}", name.contents);
         }
-        // Width is too small for the grid, so we fit it in one column
-        None => {
-            let _ = write!(out, "{}", grid.fit_into_columns(1));
+        if printed_something {
+            let _ = writeln!(out);
+        }
+    } else {
+        let mut grid = Grid::new(GridOptions {
+            filling: Filling::Spaces(2),
+            direction,
+        });
+
+        for name in names {
+            grid.add(name);
+        }
+
+        match grid.fit_into_width(width as usize) {
+            Some(output) => {
+                let _ = write!(out, "{}", output);
+            }
+            // Width is too small for the grid, so we fit it in one column
+            None => {
+                let _ = write!(out, "{}", grid.fit_into_columns(1));
+            }
         }
     }
 }
diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs
index 0a19a44fa..bc3d89c23 100644
--- a/tests/by-util/test_ls.rs
+++ b/tests/by-util/test_ls.rs
@@ -140,16 +140,7 @@ fn test_ls_width() {
             .stdout_only("test-width-1  test-width-3\ntest-width-2  test-width-4\n");
     }
 
-    for option in &[
-        "-w 25",
-        "-w=25",
-        "--width=25",
-        "--width 25",
-        "-w 0",
-        "-w=0",
-        "--width=0",
-        "--width 0",
-    ] {
+    for option in &["-w 25", "-w=25", "--width=25", "--width 25"] {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
@@ -157,6 +148,14 @@ fn test_ls_width() {
             .stdout_only("test-width-1\ntest-width-2\ntest-width-3\ntest-width-4\n");
     }
 
+    for option in &["-w 0", "-w=0", "--width=0", "--width 0"] {
+        scene
+            .ucmd()
+            .args(&option.split(' ').collect::<Vec<_>>())
+            .succeeds()
+            .stdout_only("test-width-1  test-width-2  test-width-3  test-width-4\n");
+    }
+
     scene
         .ucmd()
         .arg("-w=bad")
@@ -200,21 +199,36 @@ fn test_ls_columns() {
             .stdout_only("test-columns-1  test-columns-3\ntest-columns-2  test-columns-4\n");
     }
 
-    for option in &["-C", "--format=columns"] {
+    // On windows we are always able to get the terminal size, so we can't simulate falling back to the
+    // environment variable.
+    #[cfg(not(windows))]
+    {
+        for option in &["-C", "--format=columns"] {
+            scene
+                .ucmd()
+                .env("COLUMNS", "40")
+                .arg(option)
+                .succeeds()
+                .stdout_only("test-columns-1  test-columns-3\ntest-columns-2  test-columns-4\n");
+        }
+
         scene
             .ucmd()
-            .env("COLUMNS", "40")
-            .arg(option)
+            .env("COLUMNS", "garbage")
             .succeeds()
-            .stdout_only("test-columns-1  test-columns-3\ntest-columns-2  test-columns-4\n");
+            .stdout_is("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n")
+            .stderr_is("ls: ignoring invalid width in environment variable COLUMNS: 'garbage'");
     }
-
     scene
         .ucmd()
-        .env("COLUMNS", "garbage")
+        .arg("-w0")
         .succeeds()
-        .stdout_is("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n")
-        .stderr_is("ls: ignoring invalid width in environment variable COLUMNS: 'garbage'");
+        .stdout_only("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n");
+    scene
+        .ucmd()
+        .arg("-mw0")
+        .succeeds()
+        .stdout_only("test-columns-1, test-columns-2, test-columns-3, test-columns-4\n");
 }
 
 #[test]

From 0af244ac426f4cb85ef9cab7f722b0c734694c5a Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Wed, 11 Aug 2021 22:03:41 +0200
Subject: [PATCH 26/29] ls: default to one-line output if stdout is not a tty

---
 src/uu/ls/src/ls.rs      | 19 +++++++++++--------
 tests/by-util/test_ls.rs | 40 ++++++++++++++++++++++++----------------
 2 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index 8183f3bb8..5c58bac7c 100644
--- a/src/uu/ls/src/ls.rs
+++ b/src/uu/ls/src/ls.rs
@@ -260,16 +260,20 @@ impl Config {
                     // below should never happen as clap already restricts the values.
                     _ => unreachable!("Invalid field for --format"),
                 },
-                options::FORMAT,
+                Some(options::FORMAT),
             )
         } else if options.is_present(options::format::LONG) {
-            (Format::Long, options::format::LONG)
+            (Format::Long, Some(options::format::LONG))
         } else if options.is_present(options::format::ACROSS) {
-            (Format::Across, options::format::ACROSS)
+            (Format::Across, Some(options::format::ACROSS))
         } else if options.is_present(options::format::COMMAS) {
-            (Format::Commas, options::format::COMMAS)
+            (Format::Commas, Some(options::format::COMMAS))
+        } else if options.is_present(options::format::COLUMNS) {
+            (Format::Columns, Some(options::format::COLUMNS))
+        } else if atty::is(atty::Stream::Stdout) {
+            (Format::Columns, None)
         } else {
-            (Format::Columns, options::format::COLUMNS)
+            (Format::OneLine, None)
         };
 
         // The -o, -n and -g options are tricky. They cannot override with each
@@ -288,9 +292,8 @@ impl Config {
         // options, but manually whether they have an index that's greater than
         // the other format options. If so, we set the appropriate format.
         if format != Format::Long {
-            let idx = options
-                .indices_of(opt)
-                .map(|x| x.max().unwrap())
+            let idx = opt
+                .and_then(|opt| options.indices_of(opt).map(|x| x.max().unwrap()))
                 .unwrap_or(0);
             if [
                 options::format::LONG_NO_OWNER,
diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs
index bc3d89c23..a3372050a 100644
--- a/tests/by-util/test_ls.rs
+++ b/tests/by-util/test_ls.rs
@@ -128,6 +128,7 @@ fn test_ls_width() {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .succeeds()
             .stdout_only("test-width-1  test-width-2  test-width-3  test-width-4\n");
     }
@@ -136,6 +137,7 @@ fn test_ls_width() {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .succeeds()
             .stdout_only("test-width-1  test-width-3\ntest-width-2  test-width-4\n");
     }
@@ -144,6 +146,7 @@ fn test_ls_width() {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .succeeds()
             .stdout_only("test-width-1\ntest-width-2\ntest-width-3\ntest-width-4\n");
     }
@@ -152,6 +155,7 @@ fn test_ls_width() {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .succeeds()
             .stdout_only("test-width-1  test-width-2  test-width-3  test-width-4\n");
     }
@@ -159,6 +163,7 @@ fn test_ls_width() {
     scene
         .ucmd()
         .arg("-w=bad")
+        .arg("-C")
         .fails()
         .stderr_contains("invalid line width");
 
@@ -166,6 +171,7 @@ fn test_ls_width() {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .fails()
             .stderr_only("ls: invalid line width: '1a'");
     }
@@ -183,7 +189,7 @@ fn test_ls_columns() {
     // Columns is the default
     let result = scene.ucmd().succeeds();
 
-    result.stdout_only("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n");
+    result.stdout_only("test-columns-1\ntest-columns-2\ntest-columns-3\ntest-columns-4\n");
 
     for option in &["-C", "--format=columns"] {
         let result = scene.ucmd().arg(option).succeeds();
@@ -215,13 +221,14 @@ fn test_ls_columns() {
         scene
             .ucmd()
             .env("COLUMNS", "garbage")
+            .arg("-C")
             .succeeds()
             .stdout_is("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n")
             .stderr_is("ls: ignoring invalid width in environment variable COLUMNS: 'garbage'");
     }
     scene
         .ucmd()
-        .arg("-w0")
+        .arg("-Cw0")
         .succeeds()
         .stdout_only("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n");
     scene
@@ -591,7 +598,7 @@ fn test_ls_sort_name() {
         .ucmd()
         .arg("--sort=name")
         .succeeds()
-        .stdout_is("test-1  test-2  test-3\n");
+        .stdout_is("test-1\ntest-2\ntest-3\n");
 
     let scene_dot = TestScenario::new(util_name!());
     let at = &scene_dot.fixtures;
@@ -605,7 +612,7 @@ fn test_ls_sort_name() {
         .arg("--sort=name")
         .arg("-A")
         .succeeds()
-        .stdout_is(".a  .b  a  b\n");
+        .stdout_is(".a\n.b\na\nb\n");
 }
 
 #[test]
@@ -626,16 +633,16 @@ fn test_ls_order_size() {
     scene.ucmd().arg("-al").succeeds();
 
     let result = scene.ucmd().arg("-S").succeeds();
-    result.stdout_only("test-4  test-3  test-2  test-1\n");
+    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
 
     let result = scene.ucmd().arg("-S").arg("-r").succeeds();
-    result.stdout_only("test-1  test-2  test-3  test-4\n");
+    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
 
     let result = scene.ucmd().arg("--sort=size").succeeds();
-    result.stdout_only("test-4  test-3  test-2  test-1\n");
+    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
 
     let result = scene.ucmd().arg("--sort=size").arg("-r").succeeds();
-    result.stdout_only("test-1  test-2  test-3  test-4\n");
+    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
 }
 
 #[test]
@@ -793,16 +800,16 @@ fn test_ls_order_time() {
 
     // ctime was changed at write, so the order is 4 3 2 1
     let result = scene.ucmd().arg("-t").succeeds();
-    result.stdout_only("test-4  test-3  test-2  test-1\n");
+    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
 
     let result = scene.ucmd().arg("--sort=time").succeeds();
-    result.stdout_only("test-4  test-3  test-2  test-1\n");
+    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
 
     let result = scene.ucmd().arg("-tr").succeeds();
-    result.stdout_only("test-1  test-2  test-3  test-4\n");
+    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
 
     let result = scene.ucmd().arg("--sort=time").arg("-r").succeeds();
-    result.stdout_only("test-1  test-2  test-3  test-4\n");
+    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
 
     // 3 was accessed last in the read
     // So the order should be 2 3 4 1
@@ -813,11 +820,11 @@ fn test_ls_order_time() {
 
         // It seems to be dependent on the platform whether the access time is actually set
         if file3_access > file4_access {
-            result.stdout_only("test-3  test-4  test-2  test-1\n");
+            result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n");
         } else {
             // Access time does not seem to be set on Windows and some other
             // systems so the order is 4 3 2 1
-            result.stdout_only("test-4  test-3  test-2  test-1\n");
+            result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
         }
     }
 
@@ -826,7 +833,7 @@ fn test_ls_order_time() {
     #[cfg(unix)]
     {
         let result = scene.ucmd().arg("-tc").succeeds();
-        result.stdout_only("test-2  test-4  test-3  test-1\n");
+        result.stdout_only("test-2\ntest-4\ntest-3\ntest-1\n");
     }
 }
 
@@ -970,6 +977,7 @@ fn test_ls_color() {
         .ucmd()
         .arg("--color")
         .arg("-w=15")
+        .arg("-C")
         .succeeds()
         .stdout_only(format!(
             "{}  test-color\nb  {}\n",
@@ -1988,7 +1996,7 @@ fn test_ls_path() {
     };
     scene.ucmd().arg(&abs_path).run().stdout_is(expected_stdout);
 
-    let expected_stdout = format!("{}  {}\n", path, file1);
+    let expected_stdout = format!("{}\n{}\n", path, file1);
     scene
         .ucmd()
         .arg(file1)

From 6283dbe451ba6ea2a9ecb40ab4ae3200be60f6bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Danis?= <frederic.danis@collabora.com>
Date: Mon, 9 Aug 2021 10:50:54 +0200
Subject: [PATCH 27/29] csplit: fix suffix support without flag

csplit fails when suffix has no flags:
$ csplit result.expected -f /tmp/EXPECT -b "%d" "/^## alternative ##$/" {*}
csplit: error: incorrect conversion specification in suffix

This is supported by original csplit
---
 src/uu/csplit/src/split_name.rs | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/uu/csplit/src/split_name.rs b/src/uu/csplit/src/split_name.rs
index 758216414..44ea2a5af 100644
--- a/src/uu/csplit/src/split_name.rs
+++ b/src/uu/csplit/src/split_name.rs
@@ -47,7 +47,7 @@ impl SplitName {
             }),
             Some(custom) => {
                 let spec =
-                    Regex::new(r"(?P<ALL>%(?P<FLAG>[0#-])(?P<WIDTH>\d+)?(?P<TYPE>[diuoxX]))")
+                    Regex::new(r"(?P<ALL>%((?P<FLAG>[0#-])(?P<WIDTH>\d+)?)?(?P<TYPE>[diuoxX]))")
                         .unwrap();
                 let mut captures_iter = spec.captures_iter(&custom);
                 let custom_fn: Box<dyn Fn(usize) -> String> = match captures_iter.next() {
@@ -60,6 +60,21 @@ impl SplitName {
                             Some(m) => m.as_str().parse::<usize>().unwrap(),
                         };
                         match (captures.name("FLAG"), captures.name("TYPE")) {
+                            (None, Some(ref t)) => match t.as_str() {
+                                "d" | "i" | "u" => Box::new(move |n: usize| -> String {
+                                    format!("{}{}{}{}", prefix, before, n, after)
+                                }),
+                                "o" => Box::new(move |n: usize| -> String {
+                                    format!("{}{}{:o}{}", prefix, before, n, after)
+                                }),
+                                "x" => Box::new(move |n: usize| -> String {
+                                    format!("{}{}{:x}{}", prefix, before, n, after)
+                                }),
+                                "X" => Box::new(move |n: usize| -> String {
+                                    format!("{}{}{:X}{}", prefix, before, n, after)
+                                }),
+                                _ => return Err(CsplitError::SuffixFormatIncorrect),
+                            },
                             (Some(ref f), Some(ref t)) => {
                                 match (f.as_str(), t.as_str()) {
                                     /*
@@ -276,6 +291,12 @@ mod tests {
         assert_eq!(split_name.get(2), "xx00002");
     }
 
+    #[test]
+    fn no_padding_decimal() {
+        let split_name = SplitName::new(None, Some(String::from("cst-%d-")), None).unwrap();
+        assert_eq!(split_name.get(2), "xxcst-2-");
+    }
+
     #[test]
     fn zero_padding_decimal1() {
         let split_name = SplitName::new(None, Some(String::from("cst-%03d-")), None).unwrap();

From 1725bf6a365df9fd2d5d891da1c901958dd5f7cd Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Thu, 12 Aug 2021 17:03:06 +0200
Subject: [PATCH 28/29] pr: reduce test flakiness

Since some tests run multiple commands, we have to re-calculate the
expected result for every run.
This is because the expected results depend on file timestamps, but the
files are re-created for every new TestScenario.
---
 tests/by-util/test_pr.rs | 187 ++++++++++++++++-----------------------
 1 file changed, 78 insertions(+), 109 deletions(-)

diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs
index 4a79a3eda..5c16e7acc 100644
--- a/tests/by-util/test_pr.rs
+++ b/tests/by-util/test_pr.rs
@@ -68,41 +68,36 @@ fn test_with_numbering_option_with_number_width() {
 fn test_with_long_header_option() {
     let test_file_path = "test_one_page.log";
     let expected_test_file_path = "test_one_page_header.log.expected";
-    let mut scenario = new_ucmd!();
-    let value = file_last_modified_time(&scenario, test_file_path);
     let header = "new file";
-    scenario
-        .args(&["--header=new file", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(
-            expected_test_file_path,
-            &[("{last_modified_time}", &value), ("{header}", header)],
-        );
-
-    new_ucmd!()
-        .args(&["-h", header, test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(
-            expected_test_file_path,
-            &[("{last_modified_time}", &value), ("{header}", header)],
-        );
+    for args in &[&["-h", header][..], &["--header=new file"][..]] {
+        let mut scenario = new_ucmd!();
+        let value = file_last_modified_time(&scenario, test_file_path);
+        scenario
+            .args(args)
+            .arg(test_file_path)
+            .succeeds()
+            .stdout_is_templated_fixture(
+                expected_test_file_path,
+                &[("{last_modified_time}", &value), ("{header}", header)],
+            );
+    }
 }
 
 #[test]
 fn test_with_double_space_option() {
     let test_file_path = "test_one_page.log";
     let expected_test_file_path = "test_one_page_double_line.log.expected";
-    let mut scenario = new_ucmd!();
-    let value = file_last_modified_time(&scenario, test_file_path);
-    scenario
-        .args(&["-d", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
-
-    new_ucmd!()
-        .args(&["--double-space", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
+    for &arg in &["-d", "--double-space"] {
+        let mut scenario = new_ucmd!();
+        let value = file_last_modified_time(&scenario, test_file_path);
+        scenario
+            .args(&[arg, test_file_path])
+            .succeeds()
+            .stdout_is_templated_fixture(
+                expected_test_file_path,
+                &[("{last_modified_time}", &value)],
+            );
+    }
 }
 
 #[test]
@@ -188,33 +183,28 @@ fn test_with_page_range() {
     let test_file_path = "test.log";
     let expected_test_file_path = "test_page_range_1.log.expected";
     let expected_test_file_path1 = "test_page_range_2.log.expected";
-    let mut scenario = new_ucmd!();
-    let value = file_last_modified_time(&scenario, test_file_path);
-    scenario
-        .args(&["--pages=15", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
-
-    new_ucmd!()
-        .args(&["+15", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
-
-    new_ucmd!()
-        .args(&["--pages=15:17", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(
-            expected_test_file_path1,
-            &[("{last_modified_time}", &value)],
-        );
-
-    new_ucmd!()
-        .args(&["+15:17", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(
-            expected_test_file_path1,
-            &[("{last_modified_time}", &value)],
-        );
+    for &arg in &["--pages=15", "+15"] {
+        let mut scenario = new_ucmd!();
+        let value = file_last_modified_time(&scenario, test_file_path);
+        scenario
+            .args(&[arg, test_file_path])
+            .succeeds()
+            .stdout_is_templated_fixture(
+                expected_test_file_path,
+                &[("{last_modified_time}", &value)],
+            );
+    }
+    for &arg in &["--pages=15:17", "+15:17"] {
+        let mut scenario = new_ucmd!();
+        let value = file_last_modified_time(&scenario, test_file_path);
+        scenario
+            .args(&[arg, test_file_path])
+            .succeeds()
+            .stdout_is_templated_fixture(
+                expected_test_file_path1,
+                &[("{last_modified_time}", &value)],
+            );
+    }
 }
 
 #[test]
@@ -232,19 +222,17 @@ fn test_with_no_header_trailer_option() {
 #[test]
 fn test_with_page_length_option() {
     let test_file_path = "test.log";
-    let expected_test_file_path = "test_page_length.log.expected";
-    let expected_test_file_path1 = "test_page_length1.log.expected";
-    let mut scenario = new_ucmd!();
-    let value = file_last_modified_time(&scenario, test_file_path);
-    scenario
-        .args(&["--pages=2:3", "-l", "100", "-n", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
-
-    new_ucmd!()
-        .args(&["--pages=2:3", "-l", "5", "-n", test_file_path])
-        .succeeds()
-        .stdout_is_fixture(expected_test_file_path1);
+    for (arg, expected) in &[
+        ("100", "test_page_length.log.expected"),
+        ("5", "test_page_length1.log.expected"),
+    ] {
+        let mut scenario = new_ucmd!();
+        let value = file_last_modified_time(&scenario, test_file_path);
+        scenario
+            .args(&["--pages=2:3", "-l", arg, "-n", test_file_path])
+            .succeeds()
+            .stdout_is_templated_fixture(expected, &[("{last_modified_time}", &value)]);
+    }
 }
 
 #[test]
@@ -277,17 +265,17 @@ fn test_with_stdin() {
 fn test_with_column() {
     let test_file_path = "column.log";
     let expected_test_file_path = "column.log.expected";
-    let mut scenario = new_ucmd!();
-    let value = file_last_modified_time(&scenario, test_file_path);
-    scenario
-        .args(&["--pages=3:5", "--column=3", "-n", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
-
-    new_ucmd!()
-        .args(&["--pages=3:5", "-3", "-n", test_file_path])
-        .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
+    for arg in &["-3", "--column=3"] {
+        let mut scenario = new_ucmd!();
+        let value = file_last_modified_time(&scenario, test_file_path);
+        scenario
+            .args(&["--pages=3:5", arg, "-n", test_file_path])
+            .succeeds()
+            .stdout_is_templated_fixture(
+                expected_test_file_path,
+                &[("{last_modified_time}", &value)],
+            );
+    }
 }
 
 #[test]
@@ -305,36 +293,17 @@ fn test_with_column_across_option() {
 #[test]
 fn test_with_column_across_option_and_column_separator() {
     let test_file_path = "column.log";
-    let expected_test_file_path = "column_across_sep.log.expected";
-    let expected_test_file_path1 = "column_across_sep1.log.expected";
-    let mut scenario = new_ucmd!();
-    let value = file_last_modified_time(&scenario, test_file_path);
-    scenario
-        .args(&[
-            "--pages=3:5",
-            "--column=3",
-            "-s|",
-            "-a",
-            "-n",
-            test_file_path,
-        ])
-        .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
-
-    new_ucmd!()
-        .args(&[
-            "--pages=3:5",
-            "--column=3",
-            "-Sdivide",
-            "-a",
-            "-n",
-            test_file_path,
-        ])
-        .succeeds()
-        .stdout_is_templated_fixture(
-            expected_test_file_path1,
-            &[("{last_modified_time}", &value)],
-        );
+    for (arg, expected) in &[
+        ("-s|", "column_across_sep.log.expected"),
+        ("-Sdivide", "column_across_sep1.log.expected"),
+    ] {
+        let mut scenario = new_ucmd!();
+        let value = file_last_modified_time(&scenario, test_file_path);
+        scenario
+            .args(&["--pages=3:5", "--column=3", arg, "-a", "-n", test_file_path])
+            .succeeds()
+            .stdout_is_templated_fixture(expected, &[("{last_modified_time}", &value)]);
+    }
 }
 
 #[test]

From ce1323ce1cac263b2f7ee42ac8ec43f232692838 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Thu, 12 Aug 2021 22:42:40 +0200
Subject: [PATCH 29/29] cp: do not set the current directory in tests

Setting the current directory in tests affects other tests, even if the
change is reverted after, because tests are run in parallel.

This should fix the flaky cp tests.
---
 tests/by-util/test_cp.rs | 80 +++++++++++-----------------------------
 1 file changed, 22 insertions(+), 58 deletions(-)

diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs
index 19f93e499..541e6b5d9 100644
--- a/tests/by-util/test_cp.rs
+++ b/tests/by-util/test_cp.rs
@@ -16,8 +16,6 @@ use std::os::windows::fs::symlink_file;
 use filetime::FileTime;
 #[cfg(target_os = "linux")]
 use rlimit::Resource;
-#[cfg(not(windows))]
-use std::env;
 #[cfg(target_os = "linux")]
 use std::fs as std_fs;
 #[cfg(target_os = "linux")]
@@ -743,20 +741,16 @@ fn test_cp_deref_folder_to_folder() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
 
-    let cwd = env::current_dir().unwrap();
+    let path_to_new_symlink = at.plus(TEST_COPY_FROM_FOLDER);
 
-    let path_to_new_symlink = at.subdir.join(TEST_COPY_FROM_FOLDER);
-
-    // Change the cwd to have a correct symlink
-    assert!(env::set_current_dir(&path_to_new_symlink).is_ok());
-
-    #[cfg(not(windows))]
-    let _r = fs::symlink(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK);
-    #[cfg(windows)]
-    let _r = symlink_file(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK);
-
-    // Back to the initial cwd (breaks the other tests)
-    assert!(env::set_current_dir(&cwd).is_ok());
+    at.symlink_file(
+        &path_to_new_symlink
+            .join(TEST_HELLO_WORLD_SOURCE)
+            .to_string_lossy(),
+        &path_to_new_symlink
+            .join(TEST_HELLO_WORLD_SOURCE_SYMLINK)
+            .to_string_lossy(),
+    );
 
     //using -P -R option
     scene
@@ -843,20 +837,16 @@ fn test_cp_no_deref_folder_to_folder() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
 
-    let cwd = env::current_dir().unwrap();
+    let path_to_new_symlink = at.plus(TEST_COPY_FROM_FOLDER);
 
-    let path_to_new_symlink = at.subdir.join(TEST_COPY_FROM_FOLDER);
-
-    // Change the cwd to have a correct symlink
-    assert!(env::set_current_dir(&path_to_new_symlink).is_ok());
-
-    #[cfg(not(windows))]
-    let _r = fs::symlink(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK);
-    #[cfg(windows)]
-    let _r = symlink_file(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK);
-
-    // Back to the initial cwd (breaks the other tests)
-    assert!(env::set_current_dir(&cwd).is_ok());
+    at.symlink_file(
+        &path_to_new_symlink
+            .join(TEST_HELLO_WORLD_SOURCE)
+            .to_string_lossy(),
+        &path_to_new_symlink
+            .join(TEST_HELLO_WORLD_SOURCE_SYMLINK)
+            .to_string_lossy(),
+    );
 
     //using -P -R option
     scene
@@ -969,10 +959,9 @@ fn test_cp_archive() {
 }
 
 #[test]
-#[cfg(target_os = "unix")]
+#[cfg(unix)]
 fn test_cp_archive_recursive() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let cwd = env::current_dir().unwrap();
 
     // creates
     // dir/1
@@ -988,26 +977,13 @@ fn test_cp_archive_recursive() {
     at.touch(&file_1.to_string_lossy());
     at.touch(&file_2.to_string_lossy());
 
-    // Change the cwd to have a correct symlink
-    assert!(env::set_current_dir(&at.subdir.join(TEST_COPY_TO_FOLDER)).is_ok());
-
-    #[cfg(not(windows))]
-    {
-        let _r = fs::symlink("1", &file_1_link);
-        let _r = fs::symlink("2", &file_2_link);
-    }
-    #[cfg(windows)]
-    {
-        let _r = symlink_file("1", &file_1_link);
-        let _r = symlink_file("2", &file_2_link);
-    }
-    // Back to the initial cwd (breaks the other tests)
-    assert!(env::set_current_dir(&cwd).is_ok());
+    at.symlink_file("1", &file_1_link.to_string_lossy());
+    at.symlink_file("2", &file_2_link.to_string_lossy());
 
     ucmd.arg("--archive")
         .arg(TEST_COPY_TO_FOLDER)
         .arg(TEST_COPY_TO_FOLDER_NEW)
-        .fails(); // fails for now
+        .succeeds();
 
     let scene2 = TestScenario::new("ls");
     let result = scene2
@@ -1025,18 +1001,6 @@ fn test_cp_archive_recursive() {
         .run();
 
     println!("ls dest {}", result.stdout_str());
-    assert!(at.file_exists(
-        &at.subdir
-            .join(TEST_COPY_TO_FOLDER_NEW)
-            .join("1.link")
-            .to_string_lossy()
-    ));
-    assert!(at.file_exists(
-        &at.subdir
-            .join(TEST_COPY_TO_FOLDER_NEW)
-            .join("2.link")
-            .to_string_lossy()
-    ));
     assert!(at.file_exists(
         &at.subdir
             .join(TEST_COPY_TO_FOLDER_NEW)