Merge pull request #4859 from shinhs0506/cp-perm

cp: preserve permissions on -r -p --parents

src/uu/cp/src/copydir.rs

@@ -404,8 +404,18 @@ pub(crate) fn copy_directory(
             Err(e) => show_error!("{}", e),
         }
     }
+
     // Copy the attributes from the root directory to the target directory.
-    copy_attributes(root, target, &options.attributes)?;
+    if options.parents {
+        let dest = target.join(root.file_name().unwrap());
+        copy_attributes(root, dest.as_path(), &options.attributes)?;
+        for (x, y) in aligned_ancestors(root, dest.as_path()) {
+            copy_attributes(x, y, &options.attributes)?;
+        }
+    } else {
+        copy_attributes(root, target, &options.attributes)?;
+    }
+
     Ok(())
 }
 

src/uu/cp/src/cp.rs

@@ -1151,14 +1151,20 @@ fn copy_source(
     } else {
         // Copy as file
         let dest = construct_dest_path(source_path, target, target_type, options)?;
-        copy_file(
+        let res = copy_file(
             progress_bar,
             source_path,
             dest.as_path(),
             options,
             symlinked_files,
             true,
-        )
+        );
+        if options.parents {
+            for (x, y) in aligned_ancestors(source, dest.as_path()) {
+                copy_attributes(x, y, &options.attributes)?;
+            }
+        }
+        res
     }
 }
 
@@ -1704,11 +1710,6 @@ fn copy_file(
     }
 
     copy_attributes(source, dest, &options.attributes)?;
-    if options.parents && should_preserve_attribute(options) {
-        for (x, y) in aligned_ancestors(source, dest) {
-            copy_attributes(x, y, &options.attributes)?;
-        }
-    }
 
     if let Some(progress_bar) = progress_bar {
         progress_bar.inc(fs::metadata(source)?.len());
@@ -1758,27 +1759,6 @@ fn copy_helper(
     Ok(())
 }
 
-fn should_preserve_attribute(options: &Options) -> bool {
-    let checks = [
-        &options.attributes.mode,
-        &options.attributes.timestamps,
-        &options.attributes.links,
-        &options.attributes.context,
-        &options.attributes.xattr,
-    ];
-
-    #[cfg(unix)]
-    let checks = [
-        checks.as_slice(),
-        [&options.attributes.ownership].as_slice(),
-    ]
-    .concat();
-
-    checks
-        .iter()
-        .any(|attr| matches!(attr, Preserve::Yes { .. }))
-}
-
 // "Copies" a FIFO by creating a new one. This workaround is because Rust's
 // built-in fs::copy does not handle FIFOs (see rust-lang/rust/issues/79390).
 #[cfg(unix)]

tests/by-util/test_cp.rs

@@ -1115,6 +1115,48 @@ fn test_cp_parents_with_permissions_copy_file() {
     }
 }
 
+#[test]
+fn test_cp_parents_with_permissions_copy_dir() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    let dir1 = "dir";
+    let dir2 = "p1/p2";
+    let file = "p1/p2/file";
+
+    at.mkdir(dir1);
+    at.mkdir_all(dir2);
+    at.touch(file);
+
+    let p1_mode = 0o0777;
+    let p2_mode = 0o0711;
+    let file_mode = 0o0702;
+
+    #[cfg(unix)]
+    {
+        at.set_mode("p1", p1_mode);
+        at.set_mode("p1/p2", p2_mode);
+        at.set_mode(file, file_mode);
+    }
+
+    ucmd.arg("-p")
+        .arg("--parents")
+        .arg("-r")
+        .arg(dir2)
+        .arg(dir1)
+        .succeeds();
+
+    #[cfg(all(unix, not(target_os = "freebsd")))]
+    {
+        let p1_metadata = at.metadata("p1");
+        let p2_metadata = at.metadata("p1/p2");
+        let file_metadata = at.metadata(file);
+
+        assert_metadata_eq!(p1_metadata, at.metadata("dir/p1"));
+        assert_metadata_eq!(p2_metadata, at.metadata("dir/p1/p2"));
+        assert_metadata_eq!(file_metadata, at.metadata("dir/p1/p2/file"));
+    }
+}
+
 #[test]
 #[cfg(unix)]
 fn test_cp_writable_special_file_permissions() {