From 4b11692db30d1501511a548ecdcece6383433941 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Wed, 30 Jul 2025 16:08:42 +0800 Subject: [PATCH] cp: Also copy attributes in copy_symlink Most attributes can actually be applied to symlinks (mode and xattr are exceptions), so try to copy them, if possible. Fort those 2 exceptions, we don't need any special handling as: - Mode errors would be ignored anyway. - The source symlink cannot have any xattr anyway, so no attribute is attempted to be set on the destination. --- src/uu/cp/src/copydir.rs | 2 +- src/uu/cp/src/cp.rs | 6 ++- src/uucore/src/lib/features/perms.rs | 9 ++++ tests/by-util/test_cp.rs | 72 +++++++++++++++++++++++++++- 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index b11a5a2bd..f05777cb4 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -227,7 +227,7 @@ fn copy_direntry( // If the source is a symbolic link and the options tell us not to // dereference the link, then copy the link object itself. if source_absolute.is_symlink() && !options.dereference { - return copy_link(&source_absolute, &local_to_target, symlinked_files); + return copy_link(&source_absolute, &local_to_target, symlinked_files, options); } // If the source is a directory and the destination does not diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index a4d0cff4d..e826ac3c4 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -2563,7 +2563,7 @@ fn copy_helper( #[cfg(unix)] copy_fifo(dest, options.overwrite, options.debug)?; } else if source_is_symlink { - copy_link(source, dest, symlinked_files)?; + copy_link(source, dest, symlinked_files, options)?; } else { let copy_debug = copy_on_write( source, @@ -2600,6 +2600,7 @@ fn copy_link( source: &Path, dest: &Path, symlinked_files: &mut HashSet, + options: &Options, ) -> CopyResult<()> { // Here, we will copy the symlink itself (actually, just recreate it) let link = fs::read_link(source)?; @@ -2608,7 +2609,8 @@ fn copy_link( if dest.is_symlink() || dest.is_file() { fs::remove_file(dest)?; } - symlink_file(&link, dest, symlinked_files) + symlink_file(&link, dest, symlinked_files)?; + copy_attributes(source, dest, &options.attributes) } /// Generate an error message if `target` is not the correct `target_type` diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 52da95cd1..72fb0b27c 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -41,6 +41,15 @@ pub struct Verbosity { pub level: VerbosityLevel, } +impl Default for Verbosity { + fn default() -> Self { + Self { + groups_only: false, + level: VerbosityLevel::Normal, + } + } +} + /// Actually perform the change of owner on a path fn chown>(path: P, uid: uid_t, gid: gid_t, follow: bool) -> IOResult<()> { let path = path.as_ref(); diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 730c7a87e..432cff313 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. // spell-checker:ignore (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile uufs xattrs -// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx +// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist use uucore::display::Quotable; use uutests::util::TestScenario; use uutests::{at_and_ucmd, new_ucmd, path_concat, util_name}; @@ -3103,6 +3103,76 @@ fn test_cp_fifo() { assert_eq!(permission, "prwx-wx--x".to_string()); } +#[cfg(unix)] +#[cfg(not(target_vendor = "apple"))] +fn find_other_group(current: u32) -> Option { + // Get the first group that doesn't match current + nix::unistd::getgroups().ok()?.iter().find_map(|group| { + let gid = group.as_raw(); + (gid != current).then_some(gid) + }) +} + +#[cfg(target_vendor = "apple")] +fn find_other_group(_current: u32) -> Option { + None +} + +#[test] +#[cfg(unix)] +fn test_cp_r_symlink() { + let (at, mut ucmd) = at_and_ucmd!(); + // Specifically test copying a link in a subdirectory, as the internal path + // is slightly different. + at.mkdir("tmp"); + // Create a symlink to a non-existent file to make sure + // we don't try to resolve it. + at.symlink_file("doesnotexist", "tmp/symlink"); + let symlink = at.subdir.join("tmp").join("symlink"); + + // If we can find such a group, change the owner to a non-default to test + // that (group) ownership is preserved. + let metadata = std::fs::symlink_metadata(&symlink).unwrap(); + let other_gid = find_other_group(metadata.gid()); + if let Some(gid) = other_gid { + uucore::perms::wrap_chown( + &symlink, + &metadata, + None, + Some(gid), + false, + uucore::perms::Verbosity::default(), + ) + .expect("Cannot chgrp symlink."); + } else { + println!("Cannot find a second group to chgrp to."); + } + + // Use -r to make sure we copy the symlink itself + // --preserve will include ownership + ucmd.arg("--preserve") + .arg("-r") + .arg("tmp") + .arg("tmp2") + .succeeds() + .no_stderr() + .no_stdout(); + + // Is symlink2 still a symlink, and does it point at the same place? + assert!(at.is_symlink("tmp2/symlink")); + let symlink2 = at.subdir.join("tmp2/symlink"); + assert_eq!( + std::fs::read_link(&symlink).unwrap(), + std::fs::read_link(&symlink2).unwrap(), + ); + + // If we found a suitable group, is the group correct after the copy. + if let Some(gid) = other_gid { + let metadata2 = std::fs::symlink_metadata(&symlink2).unwrap(); + assert_eq!(metadata2.gid(), gid); + } +} + #[test] fn test_dir_recursive_copy() { let scene = TestScenario::new(util_name!());