Merge pull request #3692 from jfinkels/cp-preserve-perm-link

cp: correctly copy attributes of a dangling symbolic link

src/uu/cp/src/cp.rs

@@ -1097,12 +1097,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)?;
+            // The `chmod()` system call that underlies the
-            // FIXME: Implement this for windows as well
+            // `fs::set_permissions()` call is unable to change the
-            #[cfg(feature = "feat_acl")]
+            // permissions of a symbolic link. In that case, we just
-            exacl::getfacl(source, None)
+            // do nothing, since every symbolic link has the same
-                .and_then(|acl| exacl::setfacl(&[dest], &acl, None))
+            // permissions.
-                .map_err(|err| Error::Error(err.to_string()))?;
+            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 => {
@@ -1128,11 +1135,13 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu
             .map_err(Error::Error)?;
         }
         Attribute::Timestamps => {
-            filetime::set_file_times(
+            let atime = FileTime::from_last_access_time(&source_metadata);
-                Path::new(dest),
+            let mtime = FileTime::from_last_modification_time(&source_metadata);
-                FileTime::from_last_access_time(&source_metadata),
+            if is_symlink(dest) {
-                FileTime::from_last_modification_time(&source_metadata),
+                filetime::set_symlink_file_times(dest, atime, mtime)?;
-            )?;
+            } else {
+                filetime::set_file_times(dest, atime, mtime)?;
+            }
         }
         #[cfg(feature = "feat_selinux")]
         Attribute::Context => {

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)]
@@ -1538,6 +1540,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]
 fn test_copy_through_dangling_symlink_no_dereference_2() {
     let (at, mut ucmd) = at_and_ucmd!();

tests/common/util.rs

@@ -772,6 +772,14 @@ impl AtPath {
         }
     }
 
+    /// Decide whether the named symbolic link exists in the test directory.
+    pub fn symlink_exists(&self, path: &str) -> bool {
+        match fs::symlink_metadata(&self.plus(path)) {
+            Ok(m) => m.file_type().is_symlink(),
+            Err(_) => false,
+        }
+    }
+
     pub fn dir_exists(&self, path: &str) -> bool {
         match fs::metadata(&self.plus(path)) {
             Ok(m) => m.is_dir(),