chown/chgrp: share more code

Also share argument parsing code between `chgrp` and `chown`

src/uu/chgrp/src/chgrp.rs

@@ -11,46 +11,16 @@
 extern crate uucore;
 pub use uucore::entries;
 use uucore::error::{FromIo, UResult, USimpleError};
-use uucore::perms::{
+use uucore::perms::{chown_base, options, IfFrom};
-    ChownExecutor, IfFrom, Verbosity, VerbosityLevel, FTS_COMFOLLOW, FTS_LOGICAL, FTS_PHYSICAL,
-};
 
-use clap::{App, Arg};
+use clap::{App, Arg, ArgMatches};
 
 use std::fs;
 use std::os::unix::fs::MetadataExt;
 
-use uucore::InvalidEncodingHandling;
-
 static ABOUT: &str = "Change the group of each FILE to GROUP.";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
 
-pub mod options {
-    pub mod verbosity {
-        pub static CHANGES: &str = "changes";
-        pub static QUIET: &str = "quiet";
-        pub static SILENT: &str = "silent";
-        pub static VERBOSE: &str = "verbose";
-    }
-    pub mod preserve_root {
-        pub static PRESERVE: &str = "preserve-root";
-        pub static NO_PRESERVE: &str = "no-preserve-root";
-    }
-    pub mod dereference {
-        pub static DEREFERENCE: &str = "dereference";
-        pub static NO_DEREFERENCE: &str = "no-dereference";
-    }
-    pub static RECURSIVE: &str = "recursive";
-    pub mod traverse {
-        pub static TRAVERSE: &str = "H";
-        pub static NO_TRAVERSE: &str = "P";
-        pub static EVERY: &str = "L";
-    }
-    pub static REFERENCE: &str = "reference";
-    pub static ARG_GROUP: &str = "GROUP";
-    pub static ARG_FILES: &str = "FILE";
-}
-
 fn get_usage() -> String {
     format!(
         "{0} [OPTION]... GROUP FILE...\n    {0} [OPTION]... --reference=RFILE FILE...",
@@ -58,101 +28,7 @@ fn get_usage() -> String {
     )
 }
 
-#[uucore_procs::gen_uumain]
+fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<(Option<u32>, Option<u32>, IfFrom)> {
-pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    let args = args
-        .collect_str(InvalidEncodingHandling::ConvertLossy)
-        .accept_any();
-
-    let usage = get_usage();
-
-    let mut app = uu_app().usage(&usage[..]);
-
-    // we change the positional args based on whether
-    // --reference was used.
-    let mut reference = false;
-    let mut help = false;
-    // stop processing options on --
-    for arg in args.iter().take_while(|s| *s != "--") {
-        if arg.starts_with("--reference=") || arg == "--reference" {
-            reference = true;
-        } else if arg == "--help" {
-            // we stop processing once we see --help,
-            // as it doesn't matter if we've seen reference or not
-            help = true;
-            break;
-        }
-    }
-
-    if help || !reference {
-        // add both positional arguments
-        app = app.arg(
-            Arg::with_name(options::ARG_GROUP)
-                .value_name(options::ARG_GROUP)
-                .required(true)
-                .takes_value(true)
-                .multiple(false),
-        )
-    }
-    app = app.arg(
-        Arg::with_name(options::ARG_FILES)
-            .value_name(options::ARG_FILES)
-            .multiple(true)
-            .takes_value(true)
-            .required(true)
-            .min_values(1),
-    );
-
-    let matches = app.get_matches_from(args);
-
-    /* Get the list of files */
-    let files: Vec<String> = matches
-        .values_of(options::ARG_FILES)
-        .map(|v| v.map(ToString::to_string).collect())
-        .unwrap_or_default();
-
-    let preserve_root = matches.is_present(options::preserve_root::PRESERVE);
-
-    let mut derefer = if matches.is_present(options::dereference::DEREFERENCE) {
-        1
-    } else if matches.is_present(options::dereference::NO_DEREFERENCE) {
-        0
-    } else {
-        -1
-    };
-
-    let mut bit_flag = if matches.is_present(options::traverse::TRAVERSE) {
-        FTS_COMFOLLOW | FTS_PHYSICAL
-    } else if matches.is_present(options::traverse::EVERY) {
-        FTS_LOGICAL
-    } else {
-        FTS_PHYSICAL
-    };
-
-    let recursive = matches.is_present(options::RECURSIVE);
-    if recursive {
-        if bit_flag == FTS_PHYSICAL {
-            if derefer == 1 {
-                return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
-            }
-            derefer = 0;
-        }
-    } else {
-        bit_flag = FTS_PHYSICAL;
-    }
-
-    let verbosity_level = if matches.is_present(options::verbosity::CHANGES) {
-        VerbosityLevel::Changes
-    } else if matches.is_present(options::verbosity::SILENT)
-        || matches.is_present(options::verbosity::QUIET)
-    {
-        VerbosityLevel::Silent
-    } else if matches.is_present(options::verbosity::VERBOSE) {
-        VerbosityLevel::Verbose
-    } else {
-        VerbosityLevel::Normal
-    };
-
     let dest_gid = if let Some(file) = matches.value_of(options::REFERENCE) {
         fs::metadata(&file)
             .map(|meta| Some(meta.gid()))
@@ -168,22 +44,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             }
         }
     };
+    Ok((dest_gid, None, IfFrom::All))
+}
 
-    let executor = ChownExecutor {
+#[uucore_procs::gen_uumain]
-        bit_flag,
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-        dest_gid,
+    let usage = get_usage();
-        verbosity: Verbosity {
+
-            groups_only: true,
+    chown_base(
-            level: verbosity_level,
+        uu_app().usage(&usage[..]),
-        },
+        args,
-        recursive,
+        options::ARG_GROUP,
-        dereference: derefer != 0,
+        parse_gid_and_uid,
-        preserve_root,
+        true,
-        files,
+    )
-        filter: IfFrom::All,
-        dest_uid: None,
-    };
-    executor.exec()
 }
 
 pub fn uu_app() -> App<'static, 'static> {

src/uu/chown/src/chown.rs

@@ -10,49 +10,17 @@
 #[macro_use]
 extern crate uucore;
 pub use uucore::entries::{self, Group, Locate, Passwd};
-use uucore::perms::{
+use uucore::perms::{chown_base, options, IfFrom};
-    ChownExecutor, IfFrom, Verbosity, VerbosityLevel, FTS_COMFOLLOW, FTS_LOGICAL, FTS_PHYSICAL,
-};
 
 use uucore::error::{FromIo, UResult, USimpleError};
 
-use clap::{crate_version, App, Arg};
+use clap::{crate_version, App, Arg, ArgMatches};
 
 use std::fs;
 use std::os::unix::fs::MetadataExt;
 
-use uucore::InvalidEncodingHandling;
-
 static ABOUT: &str = "change file owner and group";
 
-pub mod options {
-    pub mod verbosity {
-        pub static CHANGES: &str = "changes";
-        pub static QUIET: &str = "quiet";
-        pub static SILENT: &str = "silent";
-        pub static VERBOSE: &str = "verbose";
-    }
-    pub mod preserve_root {
-        pub static PRESERVE: &str = "preserve-root";
-        pub static NO_PRESERVE: &str = "no-preserve-root";
-    }
-    pub mod dereference {
-        pub static DEREFERENCE: &str = "dereference";
-        pub static NO_DEREFERENCE: &str = "no-dereference";
-    }
-    pub static FROM: &str = "from";
-    pub static RECURSIVE: &str = "recursive";
-    pub mod traverse {
-        pub static TRAVERSE: &str = "H";
-        pub static NO_TRAVERSE: &str = "P";
-        pub static EVERY: &str = "L";
-    }
-    pub static REFERENCE: &str = "reference";
-}
-
-static ARG_OWNER: &str = "owner";
-static ARG_FILES: &str = "files";
-
 fn get_usage() -> String {
     format!(
         "{0} [OPTION]... [OWNER][:[GROUP]] FILE...\n{0} [OPTION]... --reference=RFILE FILE...",
@@ -60,65 +28,7 @@ fn get_usage() -> String {
     )
 }
 
-#[uucore_procs::gen_uumain]
+fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option<u32>, Option<u32>, IfFrom)> {
-pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    let args = args
-        .collect_str(InvalidEncodingHandling::Ignore)
-        .accept_any();
-
-    let usage = get_usage();
-
-    let matches = uu_app().usage(&usage[..]).get_matches_from(args);
-
-    /* First arg is the owner/group */
-    let owner = matches.value_of(ARG_OWNER).unwrap();
-
-    /* Then the list of files */
-    let files: Vec<String> = matches
-        .values_of(ARG_FILES)
-        .map(|v| v.map(ToString::to_string).collect())
-        .unwrap_or_default();
-
-    let preserve_root = matches.is_present(options::preserve_root::PRESERVE);
-
-    let mut derefer = if matches.is_present(options::dereference::NO_DEREFERENCE) {
-        1
-    } else {
-        0
-    };
-
-    let mut bit_flag = if matches.is_present(options::traverse::TRAVERSE) {
-        FTS_COMFOLLOW | FTS_PHYSICAL
-    } else if matches.is_present(options::traverse::EVERY) {
-        FTS_LOGICAL
-    } else {
-        FTS_PHYSICAL
-    };
-
-    let recursive = matches.is_present(options::RECURSIVE);
-    if recursive {
-        if bit_flag == FTS_PHYSICAL {
-            if derefer == 1 {
-                return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
-            }
-            derefer = 0;
-        }
-    } else {
-        bit_flag = FTS_PHYSICAL;
-    }
-
-    let verbosity = if matches.is_present(options::verbosity::CHANGES) {
-        VerbosityLevel::Changes
-    } else if matches.is_present(options::verbosity::SILENT)
-        || matches.is_present(options::verbosity::QUIET)
-    {
-        VerbosityLevel::Silent
-    } else if matches.is_present(options::verbosity::VERBOSE) {
-        VerbosityLevel::Verbose
-    } else {
-        VerbosityLevel::Normal
-    };
-
     let filter = if let Some(spec) = matches.value_of(options::FROM) {
         match parse_spec(spec)? {
             (Some(uid), None) => IfFrom::User(uid),
@@ -138,25 +48,24 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         dest_gid = Some(meta.gid());
         dest_uid = Some(meta.uid());
     } else {
-        let (u, g) = parse_spec(owner)?;
+        let (u, g) = parse_spec(matches.value_of(options::ARG_OWNER).unwrap())?;
         dest_uid = u;
         dest_gid = g;
     }
-    let executor = ChownExecutor {
+    Ok((dest_gid, dest_uid, filter))
-        bit_flag,
+}
-        dest_uid,
+
-        dest_gid,
+#[uucore_procs::gen_uumain]
-        verbosity: Verbosity {
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-            groups_only: false,
+    let usage = get_usage();
-            level: verbosity,
+
-        },
+    chown_base(
-        recursive,
+        uu_app().usage(&usage[..]),
-        dereference: derefer != 0,
+        args,
-        filter,
+        options::ARG_OWNER,
-        preserve_root,
+        parse_gid_uid_and_filter,
-        files,
+        false,
-    };
+    )
-    executor.exec()
 }
 
 pub fn uu_app() -> App<'static, 'static> {
@@ -169,22 +78,31 @@ pub fn uu_app() -> App<'static, 'static> {
                 .long(options::verbosity::CHANGES)
                 .help("like verbose but report only when a change is made"),
         )
-        .arg(Arg::with_name(options::dereference::DEREFERENCE).long(options::dereference::DEREFERENCE).help(
+        .arg(
-            "affect the referent of each symbolic link (this is the default), rather than the symbolic link itself",
+            Arg::with_name(options::dereference::DEREFERENCE)
-        ))
+                .long(options::dereference::DEREFERENCE)
+                .help(
+                    "affect the referent of each symbolic link (this is the default), \
+                    rather than the symbolic link itself",
+                ),
+        )
         .arg(
             Arg::with_name(options::dereference::NO_DEREFERENCE)
                 .short("h")
                 .long(options::dereference::NO_DEREFERENCE)
                 .help(
-                    "affect symbolic links instead of any referenced file (useful only on systems that can change the ownership of a symlink)",
+                    "affect symbolic links instead of any referenced file \
+                    (useful only on systems that can change the ownership of a symlink)",
                 ),
         )
         .arg(
             Arg::with_name(options::FROM)
                 .long(options::FROM)
                 .help(
-                    "change the owner and/or group of each file only if its current owner and/or group match those specified here. Either may be omitted, in which case a match is not required for the omitted attribute",
+                    "change the owner and/or group of each file only if its \
+                    current owner and/or group match those specified here. \
+                    Either may be omitted, in which case a match is not required \
+                    for the omitted attribute",
                 )
                 .value_name("CURRENT_OWNER:CURRENT_GROUP"),
         )
@@ -216,7 +134,11 @@ pub fn uu_app() -> App<'static, 'static> {
                 .value_name("RFILE")
                 .min_values(1),
         )
-        .arg(Arg::with_name(options::verbosity::SILENT).short("f").long(options::verbosity::SILENT))
+        .arg(
+            Arg::with_name(options::verbosity::SILENT)
+                .short("f")
+                .long(options::verbosity::SILENT),
+        )
         .arg(
             Arg::with_name(options::traverse::TRAVERSE)
                 .short(options::traverse::TRAVERSE)
@@ -240,19 +162,6 @@ pub fn uu_app() -> App<'static, 'static> {
                 .long(options::verbosity::VERBOSE)
                 .help("output a diagnostic for every file processed"),
         )
-        .arg(
-            Arg::with_name(ARG_OWNER)
-                .multiple(false)
-                .takes_value(true)
-                .required(true),
-        )
-        .arg(
-            Arg::with_name(ARG_FILES)
-                .multiple(true)
-                .takes_value(true)
-                .required(true)
-                .min_values(1),
-        )
 }
 
 fn parse_spec(spec: &str) -> UResult<(Option<u32>, Option<u32>)> {

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

@@ -7,10 +7,14 @@
 
 use crate::error::strip_errno;
 use crate::error::UResult;
+use crate::error::USimpleError;
 pub use crate::features::entries;
 use crate::fs::resolve_relative_path;
 use crate::show_error;
-use libc::{self, gid_t, lchown, uid_t};
+use clap::App;
+use clap::Arg;
+use clap::ArgMatches;
+use libc::{self, gid_t, uid_t};
 use walkdir::WalkDir;
 
 use std::io::Error as IOError;
@@ -45,7 +49,7 @@ fn chown<P: AsRef<Path>>(path: P, uid: uid_t, gid: gid_t, follow: bool) -> IORes
         if follow {
             libc::chown(s.as_ptr(), uid, gid)
         } else {
-            lchown(s.as_ptr(), uid, gid)
+            libc::lchown(s.as_ptr(), uid, gid)
         }
     };
     if ret == 0 {
@@ -359,3 +363,148 @@ impl ChownExecutor {
         }
     }
 }
+
+pub mod options {
+    pub mod verbosity {
+        pub const CHANGES: &str = "changes";
+        pub const QUIET: &str = "quiet";
+        pub const SILENT: &str = "silent";
+        pub const VERBOSE: &str = "verbose";
+    }
+    pub mod preserve_root {
+        pub const PRESERVE: &str = "preserve-root";
+        pub const NO_PRESERVE: &str = "no-preserve-root";
+    }
+    pub mod dereference {
+        pub const DEREFERENCE: &str = "dereference";
+        pub const NO_DEREFERENCE: &str = "no-dereference";
+    }
+    pub const FROM: &str = "from";
+    pub const RECURSIVE: &str = "recursive";
+    pub mod traverse {
+        pub const TRAVERSE: &str = "H";
+        pub const NO_TRAVERSE: &str = "P";
+        pub const EVERY: &str = "L";
+    }
+    pub const REFERENCE: &str = "reference";
+    pub const ARG_OWNER: &str = "OWNER";
+    pub const ARG_GROUP: &str = "GROUP";
+    pub const ARG_FILES: &str = "FILE";
+}
+
+type GidUidFilterParser<'a> = fn(&ArgMatches<'a>) -> UResult<(Option<u32>, Option<u32>, IfFrom)>;
+
+/// Base implementation for `chgrp` and `chown`.
+///
+/// An argument called `add_arg_if_not_reference` will be added to `app` if
+/// `args` does not contain the `--reference` option.
+/// `parse_gid_uid_and_filter` will be called to obtain the target gid and uid, and the filter,
+/// from `ArgMatches`.
+/// `groups_only` determines whether verbose output will only mention the group.
+pub fn chown_base<'a>(
+    mut app: App<'a, 'a>,
+    args: impl crate::Args,
+    add_arg_if_not_reference: &'a str,
+    parse_gid_uid_and_filter: GidUidFilterParser<'a>,
+    groups_only: bool,
+) -> UResult<()> {
+    let args: Vec<_> = args.collect();
+    let mut reference = false;
+    let mut help = false;
+    // stop processing options on --
+    for arg in args.iter().take_while(|s| *s != "--") {
+        if arg.to_string_lossy().starts_with("--reference=") || arg == "--reference" {
+            reference = true;
+        } else if arg == "--help" {
+            // we stop processing once we see --help,
+            // as it doesn't matter if we've seen reference or not
+            help = true;
+            break;
+        }
+    }
+
+    if help || !reference {
+        // add both positional arguments
+        // arg_group is only required if
+        app = app.arg(
+            Arg::with_name(add_arg_if_not_reference)
+                .value_name(add_arg_if_not_reference)
+                .required(true)
+                .takes_value(true)
+                .multiple(false),
+        )
+    }
+    app = app.arg(
+        Arg::with_name(options::ARG_FILES)
+            .value_name(options::ARG_FILES)
+            .multiple(true)
+            .takes_value(true)
+            .required(true)
+            .min_values(1),
+    );
+    let matches = app.get_matches_from(args);
+
+    let files: Vec<String> = matches
+        .values_of(options::ARG_FILES)
+        .map(|v| v.map(ToString::to_string).collect())
+        .unwrap_or_default();
+
+    let preserve_root = matches.is_present(options::preserve_root::PRESERVE);
+
+    let mut dereference = if matches.is_present(options::dereference::DEREFERENCE) {
+        1
+    } else if matches.is_present(options::dereference::NO_DEREFERENCE) {
+        0
+    } else {
+        -1
+    };
+
+    let mut bit_flag = if matches.is_present(options::traverse::TRAVERSE) {
+        FTS_COMFOLLOW | FTS_PHYSICAL
+    } else if matches.is_present(options::traverse::EVERY) {
+        FTS_LOGICAL
+    } else {
+        FTS_PHYSICAL
+    };
+
+    let recursive = matches.is_present(options::RECURSIVE);
+    if recursive {
+        if bit_flag == FTS_PHYSICAL {
+            if dereference == 1 {
+                return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
+            }
+            dereference = 0;
+        }
+    } else {
+        bit_flag = FTS_PHYSICAL;
+    }
+
+    let verbosity_level = if matches.is_present(options::verbosity::CHANGES) {
+        VerbosityLevel::Changes
+    } else if matches.is_present(options::verbosity::SILENT)
+        || matches.is_present(options::verbosity::QUIET)
+    {
+        VerbosityLevel::Silent
+    } else if matches.is_present(options::verbosity::VERBOSE) {
+        VerbosityLevel::Verbose
+    } else {
+        VerbosityLevel::Normal
+    };
+    let (dest_gid, dest_uid, filter) = parse_gid_uid_and_filter(&matches)?;
+
+    let executor = ChownExecutor {
+        bit_flag,
+        dest_gid,
+        dest_uid,
+        verbosity: Verbosity {
+            groups_only: true,
+            level: verbosity_level,
+        },
+        recursive,
+        dereference: dereference != 0,
+        preserve_root,
+        files,
+        filter,
+    };
+    executor.exec()
+}