Merge pull request #7129 from sylvestre/chgrp

 chgrp: add option --from

Author: cakebaker

src/uu/chgrp/src/chgrp.rs

@@ -19,8 +19,26 @@ use std::os::unix::fs::MetadataExt;
 const ABOUT: &str = help_about!("chgrp.md");
 const USAGE: &str = help_usage!("chgrp.md");
 
-fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<GidUidOwnerFilter> {
-    let mut raw_group: String = String::new();
+fn parse_gid_from_str(group: &str) -> Result<u32, String> {
+    if let Some(gid_str) = group.strip_prefix(':') {
+        // Handle :gid format
+        gid_str
+            .parse::<u32>()
+            .map_err(|_| format!("invalid group id: '{}'", gid_str))
+    } else {
+        // Try as group name first
+        match entries::grp2gid(group) {
+            Ok(g) => Ok(g),
+            // If group name lookup fails, try parsing as raw number
+            Err(_) => group
+                .parse::<u32>()
+                .map_err(|_| format!("invalid group: '{}'", group)),
+        }
+    }
+}
+
+fn get_dest_gid(matches: &ArgMatches) -> UResult<(Option<u32>, String)> {
+    let mut raw_group = String::new();
     let dest_gid = if let Some(file) = matches.get_one::<String>(options::REFERENCE) {
         fs::metadata(file)
             .map(|meta| {
@@ -38,22 +56,38 @@ fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<GidUidOwnerFilter> {
         if group.is_empty() {
             None
         } else {
-            match entries::grp2gid(group) {
+            match parse_gid_from_str(group) {
                 Ok(g) => Some(g),
-                _ => {
-                    return Err(USimpleError::new(
-                        1,
-                        format!("invalid group: {}", group.quote()),
-                    ))
-                }
+                Err(e) => return Err(USimpleError::new(1, e)),
+            }
+        }
+    };
+    Ok((dest_gid, raw_group))
+}
+
+fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<GidUidOwnerFilter> {
+    let (dest_gid, raw_group) = get_dest_gid(matches)?;
+
+    // Handle --from option
+    let filter = if let Some(from_group) = matches.get_one::<String>(options::FROM) {
+        match parse_gid_from_str(from_group) {
+            Ok(g) => IfFrom::Group(g),
+            Err(_) => {
+                return Err(USimpleError::new(
+                    1,
+                    format!("invalid user: '{}'", from_group),
+                ))
             }
         }
+    } else {
+        IfFrom::All
     };
+
     Ok(GidUidOwnerFilter {
         dest_gid,
         dest_uid: None,
         raw_owner: raw_group,
-        filter: IfFrom::All,
+        filter,
     })
 }
 
@@ -120,6 +154,12 @@ pub fn uu_app() -> Command {
                 .value_hint(clap::ValueHint::FilePath)
                 .help("use RFILE's group rather than specifying GROUP values"),
         )
+        .arg(
+            Arg::new(options::FROM)
+                .long(options::FROM)
+                .value_name("GROUP")
+                .help("change the group only if its current group matches GROUP"),
+        )
         .arg(
             Arg::new(options::RECURSIVE)
                 .short('R')

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

@@ -446,29 +446,21 @@ impl ChownExecutor {
 
     fn print_verbose_ownership_retained_as(&self, path: &Path, uid: u32, gid: Option<u32>) {
         if self.verbosity.level == VerbosityLevel::Verbose {
-            match (self.dest_uid, self.dest_gid, gid) {
-                (Some(_), Some(_), Some(gid)) => {
-                    println!(
-                        "ownership of {} retained as {}:{}",
-                        path.quote(),
-                        entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()),
-                        entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string()),
-                    );
-                }
+            let ownership = match (self.dest_uid, self.dest_gid, gid) {
+                (Some(_), Some(_), Some(gid)) => format!(
+                    "{}:{}",
+                    entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()),
+                    entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string())
+                ),
                 (None, Some(_), Some(gid)) => {
-                    println!(
-                        "ownership of {} retained as {}",
-                        path.quote(),
-                        entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string()),
-                    );
-                }
-                (_, _, _) => {
-                    println!(
-                        "ownership of {} retained as {}",
-                        path.quote(),
-                        entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()),
-                    );
+                    entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string())
                 }
+                _ => entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()),
+            };
+            if self.verbosity.groups_only {
+                println!("group of {} retained as {}", path.quote(), ownership);
+            } else {
+                println!("ownership of {} retained as {}", path.quote(), ownership);
             }
         }
     }

tests/by-util/test_chgrp.rs

@@ -56,7 +56,7 @@ fn test_invalid_group() {
 }
 
 #[test]
-fn test_1() {
+fn test_error_1() {
     if getegid() != 0 {
         new_ucmd!().arg("bin").arg(DIR).fails().stderr_contains(
             // linux fails with "Operation not permitted (os error 1)"
@@ -417,3 +417,179 @@ fn test_traverse_symlinks() {
         );
     }
 }
+
+#[test]
+#[cfg(not(target_vendor = "apple"))]
+fn test_from_option() {
+    use std::os::unix::fs::MetadataExt;
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    let groups = nix::unistd::getgroups().unwrap();
+    // Skip test if we don't have at least two different groups to work with
+    if groups.len() < 2 {
+        return;
+    }
+    let (first_group, second_group) = (groups[0], groups[1]);
+
+    at.touch("test_file");
+    scene
+        .ucmd()
+        .arg(first_group.to_string())
+        .arg("test_file")
+        .succeeds();
+
+    // Test successful group change with --from
+    scene
+        .ucmd()
+        .arg("--from")
+        .arg(first_group.to_string())
+        .arg(second_group.to_string())
+        .arg("test_file")
+        .succeeds()
+        .no_stderr();
+
+    // Verify the group was changed
+    let new_gid = at.plus("test_file").metadata().unwrap().gid();
+    assert_eq!(new_gid, second_group.as_raw());
+
+    scene
+        .ucmd()
+        .arg("--from")
+        .arg(first_group.to_string())
+        .arg(first_group.to_string())
+        .arg("test_file")
+        .succeeds()
+        .no_stderr();
+
+    let unchanged_gid = at.plus("test_file").metadata().unwrap().gid();
+    assert_eq!(unchanged_gid, second_group.as_raw());
+}
+
+#[test]
+#[cfg(not(any(target_os = "android", target_os = "macos")))]
+fn test_from_with_invalid_group() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.touch("test_file");
+    #[cfg(not(target_os = "android"))]
+    let err_msg = "chgrp: invalid user: 'nonexistent_group'\n";
+    #[cfg(target_os = "android")]
+    let err_msg = "chgrp: invalid user: 'staff'\n";
+
+    ucmd.arg("--from")
+        .arg("nonexistent_group")
+        .arg("staff")
+        .arg("test_file")
+        .fails()
+        .stderr_is(err_msg);
+}
+
+#[test]
+#[cfg(not(target_vendor = "apple"))]
+fn test_verbosity_messages() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    let groups = nix::unistd::getgroups().unwrap();
+    // Skip test if we don't have at least one group to work with
+    if groups.is_empty() {
+        return;
+    }
+
+    at.touch("ref_file");
+    at.touch("target_file");
+
+    scene
+        .ucmd()
+        .arg("-v")
+        .arg("--reference=ref_file")
+        .arg("target_file")
+        .succeeds()
+        .stderr_contains("group of 'target_file' retained as ");
+}
+
+#[test]
+#[cfg(not(target_vendor = "apple"))]
+fn test_from_with_reference() {
+    use std::os::unix::fs::MetadataExt;
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    let groups = nix::unistd::getgroups().unwrap();
+    if groups.len() < 2 {
+        return;
+    }
+    let (first_group, second_group) = (groups[0], groups[1]);
+
+    at.touch("ref_file");
+    at.touch("test_file");
+
+    scene
+        .ucmd()
+        .arg(first_group.to_string())
+        .arg("test_file")
+        .succeeds();
+
+    scene
+        .ucmd()
+        .arg(second_group.to_string())
+        .arg("ref_file")
+        .succeeds();
+
+    // Test --from with --reference
+    scene
+        .ucmd()
+        .arg("--from")
+        .arg(first_group.to_string())
+        .arg("--reference=ref_file")
+        .arg("test_file")
+        .succeeds()
+        .no_stderr();
+
+    let new_gid = at.plus("test_file").metadata().unwrap().gid();
+    let ref_gid = at.plus("ref_file").metadata().unwrap().gid();
+    assert_eq!(new_gid, ref_gid);
+}
+
+#[test]
+#[cfg(not(target_vendor = "apple"))]
+fn test_numeric_group_formats() {
+    use std::os::unix::fs::MetadataExt;
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let groups = nix::unistd::getgroups().unwrap();
+    if groups.len() < 2 {
+        return;
+    }
+    let (first_group, second_group) = (groups[0], groups[1]);
+
+    at.touch("test_file");
+
+    scene
+        .ucmd()
+        .arg(first_group.to_string())
+        .arg("test_file")
+        .succeeds();
+
+    // Test :gid format in --from
+    scene
+        .ucmd()
+        .arg(format!("--from=:{}", first_group.as_raw()))
+        .arg(second_group.to_string())
+        .arg("test_file")
+        .succeeds()
+        .no_stderr();
+
+    let new_gid = at.plus("test_file").metadata().unwrap().gid();
+    assert_eq!(new_gid, second_group.as_raw());
+
+    // Test :gid format in target group
+    scene
+        .ucmd()
+        .arg(format!("--from={}", second_group.as_raw()))
+        .arg(format!(":{}", first_group.as_raw()))
+        .arg("test_file")
+        .succeeds()
+        .no_stderr();
+
+    let final_gid = at.plus("test_file").metadata().unwrap().gid();
+    assert_eq!(final_gid, first_group.as_raw());
+}