diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index c38daecf6..3a9f06ac6 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1146,12 +1146,19 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu let source_metadata = fs::symlink_metadata(source).context(context)?; match *attribute { Attribute::Mode => { - fs::set_permissions(dest, source_metadata.permissions()).context(context)?; - // FIXME: Implement this for windows as well - #[cfg(feature = "feat_acl")] - exacl::getfacl(source, None) - .and_then(|acl| exacl::setfacl(&[dest], &acl, None)) - .map_err(|err| Error::Error(err.to_string()))?; + // The `chmod()` system call that underlies the + // `fs::set_permissions()` call is unable to change the + // permissions of a symbolic link. In that case, we just + // do nothing, since every symbolic link has the same + // permissions. + if !is_symlink(dest) { + fs::set_permissions(dest, source_metadata.permissions()).context(context)?; + // FIXME: Implement this for windows as well + #[cfg(feature = "feat_acl")] + exacl::getfacl(source, None) + .and_then(|acl| exacl::setfacl(&[dest], &acl, None)) + .map_err(|err| Error::Error(err.to_string()))?; + } } #[cfg(unix)] Attribute::Ownership => { @@ -1177,11 +1184,13 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu .map_err(Error::Error)?; } Attribute::Timestamps => { - filetime::set_file_times( - Path::new(dest), - FileTime::from_last_access_time(&source_metadata), - FileTime::from_last_modification_time(&source_metadata), - )?; + let atime = FileTime::from_last_access_time(&source_metadata); + let mtime = FileTime::from_last_modification_time(&source_metadata); + if is_symlink(dest) { + filetime::set_symlink_file_times(dest, atime, mtime)?; + } else { + filetime::set_file_times(dest, atime, mtime)?; + } } #[cfg(feature = "feat_selinux")] Attribute::Context => { diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index c246d0ef9..098d73178 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -9,6 +9,8 @@ use std::os::unix::fs; #[cfg(unix)] use std::os::unix::fs::symlink as symlink_file; +#[cfg(unix)] +use std::os::unix::fs::MetadataExt; #[cfg(all(unix, not(target_os = "freebsd")))] use std::os::unix::fs::PermissionsExt; #[cfg(windows)] @@ -1536,6 +1538,37 @@ fn test_copy_through_dangling_symlink_no_dereference() { .no_stdout(); } +/// Test for copying a dangling symbolic link and its permissions. +#[test] +fn test_copy_through_dangling_symlink_no_dereference_permissions() { + let (at, mut ucmd) = at_and_ucmd!(); + // target name link name + at.symlink_file("no-such-file", "dangle"); + // don't dereference the link + // | copy permissions, too + // | | from the link + // | | | to new file d2 + // | | | | + // V V V V + ucmd.args(&["-P", "-p", "dangle", "d2"]) + .succeeds() + .no_stderr() + .no_stdout(); + assert!(at.symlink_exists("d2")); + + // `-p` means `--preserve=mode,ownership,timestamps` + #[cfg(unix)] + { + let metadata1 = at.symlink_metadata("dangle"); + let metadata2 = at.symlink_metadata("d2"); + assert_eq!(metadata1.mode(), metadata2.mode()); + assert_eq!(metadata1.uid(), metadata2.uid()); + assert_eq!(metadata1.atime(), metadata2.atime()); + assert_eq!(metadata1.mtime(), metadata2.mtime()); + assert_eq!(metadata1.ctime(), metadata2.ctime()); + } +} + #[test] #[cfg(unix)] fn test_cp_archive_on_nonexistent_file() {