diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index fe5aee872..07d34071c 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/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 { - let mut raw_group: String = String::new(); +fn parse_gid_from_str(group: &str) -> Result { + if let Some(gid_str) = group.strip_prefix(':') { + // Handle :gid format + gid_str + .parse::() + .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::() + .map_err(|_| format!("invalid group: '{}'", group)), + } + } +} + +fn get_dest_gid(matches: &ArgMatches) -> UResult<(Option, String)> { + let mut raw_group = String::new(); let dest_gid = if let Some(file) = matches.get_one::(options::REFERENCE) { fs::metadata(file) .map(|meta| { @@ -38,22 +56,38 @@ fn parse_gid_and_uid(matches: &ArgMatches) -> UResult { 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 { + let (dest_gid, raw_group) = get_dest_gid(matches)?; + + // Handle --from option + let filter = if let Some(from_group) = matches.get_one::(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') diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 3d10dfd07..3879b7337 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/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) { 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); } } } diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index eca5ba0ed..cd40d80be 100644 --- a/tests/by-util/test_chgrp.rs +++ b/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()); +}