diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index b11a5a2bd37..f05777cb485 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 2cdd79255cf..e826ac3c470 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -2446,7 +2446,7 @@ fn copy_file( copy_attributes(&src, dest, &options.attributes)?; } } - } else if source_is_stream && source.exists() { + } else if source_is_stream && !source.exists() { // Some stream files may not exist after we have copied it, // like anonymous pipes. Thus, we can't really copy its // attributes. However, this is already handled in the stream @@ -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 52da95cd1bf..72fb0b27caa 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 8575d1959e1..03727365292 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 +// 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}; @@ -3087,13 +3087,89 @@ fn test_cp_link_backup() { fn test_cp_fifo() { let (at, mut ucmd) = at_and_ucmd!(); at.mkfifo("fifo"); - ucmd.arg("-r") + // Also test that permissions are preserved + at.set_mode("fifo", 0o731); + ucmd.arg("--preserve=mode") + .arg("-r") .arg("fifo") .arg("fifo2") .succeeds() .no_stderr() .no_stdout(); assert!(at.is_fifo("fifo2")); + + let metadata = std::fs::metadata(at.subdir.join("fifo2")).unwrap(); + let permission = uucore::fs::display_permissions(&metadata, true); + assert_eq!(permission, "prwx-wx--x".to_string()); +} + +#[cfg(all(unix, 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]