chgrp: forward to chown

chgrp does mostly the same as chown.
By making chown a bit more configurable we can reuse its code for chgrp.

Cargo.lock

@@ -2126,9 +2126,9 @@ name = "uu_chgrp"
 version = "0.0.7"
 dependencies = [
  "clap",
+ "uu_chown",
  "uucore",
  "uucore_procs",
- "walkdir",
 ]
 
 [[package]]

src/uu/chgrp/Cargo.toml

@@ -18,7 +18,7 @@ path = "src/chgrp.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" }
-walkdir = "2.2"
+uu_chown = { version=">=0.0.6", package="uu_chown", path="../chown"}
 
 [[bin]]
 name = "chgrp"

src/uu/chgrp/src/chgrp.rs

@@ -5,25 +5,20 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) COMFOLLOW Chgrper RFILE RFILE's derefer dgid nonblank nonprint nonprinting
+// spell-checker:ignore (ToDO) COMFOLLOW Chowner RFILE RFILE's derefer dgid nonblank nonprint nonprinting
 
 #[macro_use]
 extern crate uucore;
+use uu_chown::{Chowner, IfFrom};
 pub use uucore::entries;
-use uucore::fs::resolve_relative_path;
+use uucore::error::{FromIo, UResult, USimpleError};
-use uucore::libc::gid_t;
+use uucore::perms::{Verbosity, VerbosityLevel};
-use uucore::perms::{wrap_chgrp, Verbosity};
 
 use clap::{App, Arg};
 
-extern crate walkdir;
-use walkdir::WalkDir;
-
 use std::fs;
-use std::fs::Metadata;
 use std::os::unix::fs::MetadataExt;
 
-use std::path::Path;
 use uucore::InvalidEncodingHandling;
 
 static ABOUT: &str = "Change the group of each FILE to GROUP.";
@@ -66,7 +61,8 @@ fn usage() -> String {
     )
 }
 
-pub fn uumain(args: impl uucore::Args) -> i32 {
+#[uucore_procs::gen_uumain]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let args = args
         .collect_str(InvalidEncodingHandling::ConvertLossy)
         .accept_any();
@@ -140,8 +136,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     if recursive {
         if bit_flag == FTS_PHYSICAL {
             if derefer == 1 {
-                show_error!("-R --dereference requires -H or -L");
+                return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
-                return 1;
             }
             derefer = 0;
         }
@@ -149,26 +144,22 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         bit_flag = FTS_PHYSICAL;
     }
 
-    let verbosity = if matches.is_present(options::verbosity::CHANGES) {
+    let verbosity_level = if matches.is_present(options::verbosity::CHANGES) {
-        Verbosity::Changes
+        VerbosityLevel::Changes
     } else if matches.is_present(options::verbosity::SILENT)
         || matches.is_present(options::verbosity::QUIET)
     {
-        Verbosity::Silent
+        VerbosityLevel::Silent
     } else if matches.is_present(options::verbosity::VERBOSE) {
-        Verbosity::Verbose
+        VerbosityLevel::Verbose
     } else {
-        Verbosity::Normal
+        VerbosityLevel::Normal
     };
 
     let dest_gid = if let Some(file) = matches.value_of(options::REFERENCE) {
-        match fs::metadata(&file) {
+        fs::metadata(&file)
-            Ok(meta) => Some(meta.gid()),
+            .map(|meta| Some(meta.gid()))
-            Err(e) => {
+            .map_err_context(|| format!("failed to get attributes of '{}'", file))?
-                show_error!("failed to get attributes of '{}': {}", file, e);
-                return 1;
-            }
-        }
     } else {
         let group = matches.value_of(options::ARG_GROUP).unwrap_or_default();
         if group.is_empty() {
@@ -176,22 +167,24 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         } else {
             match entries::grp2gid(group) {
                 Ok(g) => Some(g),
-                _ => {
+                _ => return Err(USimpleError::new(1, format!("invalid group: '{}'", group))),
-                    show_error!("invalid group: {}", group);
-                    return 1;
-                }
             }
         }
     };
 
-    let executor = Chgrper {
+    let executor = Chowner {
         bit_flag,
         dest_gid,
-        verbosity,
+        verbosity: Verbosity {
+            groups_only: true,
+            level: verbosity_level,
+        },
         recursive,
         dereference: derefer != 0,
         preserve_root,
         files,
+        filter: IfFrom::All,
+        dest_uid: None,
     };
     executor.exec()
 }
@@ -275,172 +268,3 @@ pub fn uu_app() -> App<'static, 'static> {
                 .help("traverse every symbolic link to a directory encountered"),
         )
 }
-
-struct Chgrper {
-    dest_gid: Option<gid_t>,
-    bit_flag: u8,
-    verbosity: Verbosity,
-    files: Vec<String>,
-    recursive: bool,
-    preserve_root: bool,
-    dereference: bool,
-}
-
-macro_rules! unwrap {
-    ($m:expr, $e:ident, $err:block) => {
-        match $m {
-            Ok(meta) => meta,
-            Err($e) => $err,
-        }
-    };
-}
-
-impl Chgrper {
-    fn exec(&self) -> i32 {
-        let mut ret = 0;
-        for f in &self.files {
-            ret |= self.traverse(f);
-        }
-        ret
-    }
-
-    #[cfg(windows)]
-    fn is_bind_root<P: AsRef<Path>>(&self, root: P) -> bool {
-        // TODO: is there an equivalent on Windows?
-        false
-    }
-
-    #[cfg(unix)]
-    fn is_bind_root<P: AsRef<Path>>(&self, path: P) -> bool {
-        if let (Ok(given), Ok(root)) = (fs::metadata(path), fs::metadata("/")) {
-            given.dev() == root.dev() && given.ino() == root.ino()
-        } else {
-            // FIXME: not totally sure if it's okay to just ignore an error here
-            false
-        }
-    }
-
-    fn traverse<P: AsRef<Path>>(&self, root: P) -> i32 {
-        let follow_arg = self.dereference || self.bit_flag != FTS_PHYSICAL;
-        let path = root.as_ref();
-        let meta = match self.obtain_meta(path, follow_arg) {
-            Some(m) => m,
-            _ => return 1,
-        };
-
-        // Prohibit only if:
-        // (--preserve-root and -R present) &&
-        // (
-        //     (argument is not symlink && resolved to be '/') ||
-        //     (argument is symlink && should follow argument && resolved to be '/')
-        // )
-        if self.recursive && self.preserve_root {
-            let may_exist = if follow_arg {
-                path.canonicalize().ok()
-            } else {
-                let real = resolve_relative_path(path);
-                if real.is_dir() {
-                    Some(real.canonicalize().expect("failed to get real path"))
-                } else {
-                    Some(real.into_owned())
-                }
-            };
-
-            if let Some(p) = may_exist {
-                if p.parent().is_none() || self.is_bind_root(p) {
-                    show_error!("it is dangerous to operate recursively on '/'");
-                    show_error!("use --no-preserve-root to override this failsafe");
-                    return 1;
-                }
-            }
-        }
-
-        let ret = match wrap_chgrp(
-            path,
-            &meta,
-            self.dest_gid,
-            follow_arg,
-            self.verbosity.clone(),
-        ) {
-            Ok(n) => {
-                if !n.is_empty() {
-                    show_error!("{}", n);
-                }
-                0
-            }
-            Err(e) => {
-                if self.verbosity != Verbosity::Silent {
-                    show_error!("{}", e);
-                }
-                1
-            }
-        };
-
-        if !self.recursive {
-            ret
-        } else {
-            ret | self.dive_into(&root)
-        }
-    }
-
-    fn dive_into<P: AsRef<Path>>(&self, root: P) -> i32 {
-        let mut ret = 0;
-        let root = root.as_ref();
-        let follow = self.dereference || self.bit_flag & FTS_LOGICAL != 0;
-        for entry in WalkDir::new(root).follow_links(follow).min_depth(1) {
-            let entry = unwrap!(entry, e, {
-                ret = 1;
-                show_error!("{}", e);
-                continue;
-            });
-            let path = entry.path();
-            let meta = match self.obtain_meta(path, follow) {
-                Some(m) => m,
-                _ => {
-                    ret = 1;
-                    continue;
-                }
-            };
-
-            ret = match wrap_chgrp(path, &meta, self.dest_gid, follow, self.verbosity.clone()) {
-                Ok(n) => {
-                    if !n.is_empty() {
-                        show_error!("{}", n);
-                    }
-                    0
-                }
-                Err(e) => {
-                    if self.verbosity != Verbosity::Silent {
-                        show_error!("{}", e);
-                    }
-                    1
-                }
-            }
-        }
-
-        ret
-    }
-
-    fn obtain_meta<P: AsRef<Path>>(&self, path: P, follow: bool) -> Option<Metadata> {
-        use self::Verbosity::*;
-        let path = path.as_ref();
-        let meta = if follow {
-            unwrap!(path.metadata(), e, {
-                match self.verbosity {
-                    Silent => (),
-                    _ => show_error!("cannot access '{}': {}", path.display(), e),
-                }
-                return None;
-            })
-        } else {
-            unwrap!(path.symlink_metadata(), e, {
-                match self.verbosity {
-                    Silent => (),
-                    _ => show_error!("cannot dereference '{}': {}", path.display(), e),
-                }
-                return None;
-            })
-        };
-        Some(meta)
-    }
-}

src/uu/chown/src/chown.rs

@@ -12,7 +12,7 @@ extern crate uucore;
 pub use uucore::entries::{self, Group, Locate, Passwd};
 use uucore::fs::resolve_relative_path;
 use uucore::libc::{gid_t, uid_t};
-use uucore::perms::{wrap_chown, Verbosity};
+use uucore::perms::{wrap_chown, Verbosity, VerbosityLevel};
 
 use uucore::error::{FromIo, UResult, USimpleError};
 
@@ -116,15 +116,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     }
 
     let verbosity = if matches.is_present(options::verbosity::CHANGES) {
-        Verbosity::Changes
+        VerbosityLevel::Changes
     } else if matches.is_present(options::verbosity::SILENT)
         || matches.is_present(options::verbosity::QUIET)
     {
-        Verbosity::Silent
+        VerbosityLevel::Silent
     } else if matches.is_present(options::verbosity::VERBOSE) {
-        Verbosity::Verbose
+        VerbosityLevel::Verbose
     } else {
-        Verbosity::Normal
+        VerbosityLevel::Normal
     };
 
     let filter = if let Some(spec) = matches.value_of(options::FROM) {
@@ -154,7 +154,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         bit_flag,
         dest_uid,
         dest_gid,
-        verbosity,
+        verbosity: Verbosity {
+            groups_only: false,
+            level: verbosity,
+        },
         recursive,
         dereference: derefer != 0,
         filter,
@@ -286,23 +289,23 @@ fn parse_spec(spec: &str) -> UResult<(Option<u32>, Option<u32>)> {
     Ok((uid, gid))
 }
 
-enum IfFrom {
+pub enum IfFrom {
     All,
     User(u32),
     Group(u32),
     UserGroup(u32, u32),
 }
 
-struct Chowner {
+pub struct Chowner {
-    dest_uid: Option<u32>,
+    pub dest_uid: Option<u32>,
-    dest_gid: Option<u32>,
+    pub dest_gid: Option<u32>,
-    bit_flag: u8,
+    pub bit_flag: u8,
-    verbosity: Verbosity,
+    pub verbosity: Verbosity,
-    filter: IfFrom,
+    pub filter: IfFrom,
-    files: Vec<String>,
+    pub files: Vec<String>,
-    recursive: bool,
+    pub recursive: bool,
-    preserve_root: bool,
+    pub preserve_root: bool,
-    dereference: bool,
+    pub dereference: bool,
 }
 
 macro_rules! unwrap {
@@ -315,7 +318,7 @@ macro_rules! unwrap {
 }
 
 impl Chowner {
-    fn exec(&self) -> UResult<()> {
+    pub fn exec(&self) -> UResult<()> {
         let mut ret = 0;
         for f in &self.files {
             ret |= self.traverse(f);
@@ -377,7 +380,7 @@ impl Chowner {
                     0
                 }
                 Err(e) => {
-                    if self.verbosity != Verbosity::Silent {
+                    if self.verbosity.level != VerbosityLevel::Silent {
                         show_error!("{}", e);
                     }
                     1
@@ -432,7 +435,7 @@ impl Chowner {
                     0
                 }
                 Err(e) => {
-                    if self.verbosity != Verbosity::Silent {
+                    if self.verbosity.level != VerbosityLevel::Silent {
                         show_error!("{}", e);
                     }
                     1
@@ -443,20 +446,19 @@ impl Chowner {
     }
 
     fn obtain_meta<P: AsRef<Path>>(&self, path: P, follow: bool) -> Option<Metadata> {
-        use self::Verbosity::*;
         let path = path.as_ref();
         let meta = if follow {
             unwrap!(path.metadata(), e, {
-                match self.verbosity {
+                match self.verbosity.level {
-                    Silent => (),
+                    VerbosityLevel::Silent => (),
                     _ => show_error!("cannot access '{}': {}", path.display(), e),
                 }
                 return None;
             })
         } else {
             unwrap!(path.symlink_metadata(), e, {
-                match self.verbosity {
+                match self.verbosity.level {
-                    Silent => (),
+                    VerbosityLevel::Silent => (),
                     _ => show_error!("cannot dereference '{}': {}", path.display(), e),
                 }
                 return None;

src/uu/install/src/install.rs

@@ -18,7 +18,7 @@ use filetime::{set_file_times, FileTime};
 use uucore::backup_control::{self, BackupMode};
 use uucore::entries::{grp2gid, usr2uid};
 use uucore::error::{FromIo, UError, UIoError, UResult, USimpleError};
-use uucore::perms::{wrap_chgrp, wrap_chown, Verbosity};
+use uucore::perms::{wrap_chown, Verbosity, VerbosityLevel};
 
 use libc::{getegid, geteuid};
 use std::error::Error;
@@ -641,7 +641,10 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> {
             Some(owner_id),
             Some(gid),
             false,
-            Verbosity::Normal,
+            Verbosity {
+                groups_only: false,
+                level: VerbosityLevel::Normal,
+            },
         ) {
             Ok(n) => {
                 if !n.is_empty() {
@@ -662,7 +665,17 @@ 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, Some(group_id), false, Verbosity::Normal) {
+        match wrap_chown(
+            to,
+            &meta,
+            Some(group_id),
+            None,
+            false,
+            Verbosity {
+                groups_only: true,
+                level: VerbosityLevel::Normal,
+            },
+        ) {
             Ok(n) => {
                 if !n.is_empty() {
                     show_error!("{}", n);

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

@@ -18,86 +18,16 @@ use std::path::Path;
 
 /// The various level of verbosity
 #[derive(PartialEq, Clone, Debug)]
-pub enum Verbosity {
+pub enum VerbosityLevel {
     Silent,
     Changes,
     Verbose,
     Normal,
 }
-
+#[derive(PartialEq, Clone, Debug)]
-/// Actually perform the change of group on a path
+pub struct Verbosity {
-fn chgrp<P: AsRef<Path>>(path: P, gid: gid_t, follow: bool) -> IOResult<()> {
+    pub groups_only: bool,
-    let path = path.as_ref();
+    pub level: VerbosityLevel,
-    let s = CString::new(path.as_os_str().as_bytes()).unwrap();
-    let ret = unsafe {
-        if follow {
-            libc::chown(s.as_ptr(), 0_u32.wrapping_sub(1), gid)
-        } else {
-            lchown(s.as_ptr(), 0_u32.wrapping_sub(1), gid)
-        }
-    };
-    if ret == 0 {
-        Ok(())
-    } else {
-        Err(IOError::last_os_error())
-    }
-}
-
-/// Perform the change of group on a path
-/// with the various options
-/// and error messages management
-pub fn wrap_chgrp<P: AsRef<Path>>(
-    path: P,
-    meta: &Metadata,
-    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 {
-            Silent => (),
-            _ => {
-                out = format!("changing group of '{}': {}", path.display(), e);
-                if verbosity == Verbose {
-                    out = format!(
-                        "{}\nfailed to change group of '{}' from {} to {}",
-                        out,
-                        path.display(),
-                        entries::gid2grp(meta.gid()).unwrap(),
-                        entries::gid2grp(dest_gid).unwrap()
-                    );
-                };
-            }
-        }
-        return Err(out);
-    } else {
-        let changed = dest_gid != meta.gid();
-        if changed {
-            match verbosity {
-                Changes | Verbose => {
-                    out = format!(
-                        "changed group of '{}' from {} to {}",
-                        path.display(),
-                        entries::gid2grp(meta.gid()).unwrap(),
-                        entries::gid2grp(dest_gid).unwrap()
-                    );
-                }
-                _ => (),
-            };
-        } else if verbosity == Verbose {
-            out = format!(
-                "group of '{}' retained as {}",
-                path.display(),
-                entries::gid2grp(dest_gid).unwrap_or_default()
-            );
-        }
-    }
-    Ok(out)
 }
 
 /// Actually perform the change of owner on a path
@@ -129,19 +59,36 @@ pub fn wrap_chown<P: AsRef<Path>>(
     follow: bool,
     verbosity: Verbosity,
 ) -> Result<String, String> {
-    use self::Verbosity::*;
     let dest_uid = dest_uid.unwrap_or_else(|| meta.uid());
     let dest_gid = dest_gid.unwrap_or_else(|| meta.gid());
     let path = path.as_ref();
     let mut out: String = String::new();
 
     if let Err(e) = chown(path, dest_uid, dest_gid, follow) {
-        match verbosity {
+        match verbosity.level {
-            Silent => (),
+            VerbosityLevel::Silent => (),
-            _ => {
+            level => {
-                out = format!("changing ownership of '{}': {}", path.display(), e);
-                if verbosity == Verbose {
                 out = format!(
+                    "changing {} of '{}': {}",
+                    if verbosity.groups_only {
+                        "group"
+                    } else {
+                        "ownership"
+                    },
+                    path.display(),
+                    e
+                );
+                if level == VerbosityLevel::Verbose {
+                    out = if verbosity.groups_only {
+                        format!(
+                            "{}\nfailed to change group of '{}' from {} to {}",
+                            out,
+                            path.display(),
+                            entries::gid2grp(meta.gid()).unwrap(),
+                            entries::gid2grp(dest_gid).unwrap()
+                        )
+                    } else {
+                        format!(
                             "{}\nfailed to change ownership of '{}' from {}:{} to {}:{}",
                             out,
                             path.display(),
@@ -149,7 +96,8 @@ pub fn wrap_chown<P: AsRef<Path>>(
                             entries::gid2grp(meta.gid()).unwrap(),
                             entries::uid2usr(dest_uid).unwrap(),
                             entries::gid2grp(dest_gid).unwrap()
-                    );
+                        )
+                    };
                 };
             }
         }
@@ -157,26 +105,43 @@ pub fn wrap_chown<P: AsRef<Path>>(
     } else {
         let changed = dest_uid != meta.uid() || dest_gid != meta.gid();
         if changed {
-            match verbosity {
+            match verbosity.level {
-                Changes | Verbose => {
+                VerbosityLevel::Changes | VerbosityLevel::Verbose => {
-                    out = format!(
+                    out = if verbosity.groups_only {
+                        format!(
+                            "changed group of '{}' from {} to {}",
+                            path.display(),
+                            entries::gid2grp(meta.gid()).unwrap(),
+                            entries::gid2grp(dest_gid).unwrap()
+                        )
+                    } else {
+                        format!(
                             "changed ownership of '{}' from {}:{} to {}:{}",
                             path.display(),
                             entries::uid2usr(meta.uid()).unwrap(),
                             entries::gid2grp(meta.gid()).unwrap(),
                             entries::uid2usr(dest_uid).unwrap(),
                             entries::gid2grp(dest_gid).unwrap()
-                    );
+                        )
+                    };
                 }
                 _ => (),
             };
-        } else if verbosity == Verbose {
+        } else if verbosity.level == VerbosityLevel::Verbose {
-            out = format!(
+            out = if verbosity.groups_only {
+                format!(
+                    "group of '{}' retained as {}",
+                    path.display(),
+                    entries::gid2grp(dest_gid).unwrap_or_default()
+                )
+            } else {
+                format!(
                     "ownership of '{}' retained as {}:{}",
                     path.display(),
                     entries::uid2usr(dest_uid).unwrap(),
                     entries::gid2grp(dest_gid).unwrap()
-            );
+                )
+            };
         }
     }
     Ok(out)

tests/by-util/test_chgrp.rs

@@ -43,7 +43,7 @@ fn test_invalid_group() {
         .arg("__nosuchgroup__")
         .arg("/")
         .fails()
-        .stderr_is("chgrp: invalid group: __nosuchgroup__");
+        .stderr_is("chgrp: invalid group: '__nosuchgroup__'");
 }
 
 #[test]