cp: Preserve mode for directories when copying by default.

Author: nikolalukovic

src/uu/cp/src/cp.rs

@@ -1664,6 +1664,13 @@ pub(crate) fn copy_attributes(
     let source_metadata =
         fs::symlink_metadata(source).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
 
+    // if --no-preserve wasn't explicitly passed and we're copying a directory by default we should preserve mode
+    let mode = if dest.is_dir() && attributes.mode == (Preserve::No { explicit: false }) {
+        Preserve::Yes { required: false }
+    } else {
+        attributes.mode
+    };
+
     // Ownership must be changed first to avoid interfering with mode change.
     #[cfg(unix)]
     handle_preserve(&attributes.ownership, || -> CopyResult<()> {
@@ -1701,7 +1708,7 @@ pub(crate) fn copy_attributes(
         Ok(())
     })?;
 
-    handle_preserve(&attributes.mode, || -> CopyResult<()> {
+    handle_preserve(&mode, || -> CopyResult<()> {
         // 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

tests/by-util/test_cp.rs

@@ -7400,3 +7400,71 @@ fn test_cp_recurse_verbose_output_with_symlink_already_exists() {
         .no_stderr()
         .stdout_is(output);
 }
+
+#[test]
+#[cfg(not(target_os = "windows"))]
+fn test_cp_preserve_directory_permissions_by_default() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let dir = "a/b/c/d";
+    let file = "foo.txt";
+
+    at.mkdir_all(dir);
+
+    let file_path = format!("{dir}/{file}");
+
+    at.touch(file_path);
+
+    scene.cmd("chmod").arg("-R").arg("555").arg("a").succeeds();
+    scene.cmd("cp").arg("-r").arg("a").arg("b").succeeds();
+
+    scene.ucmd().arg("-r").arg("a").arg("c").succeeds();
+
+    assert_eq!(at.get_mode("b"), 0o40555);
+    assert_eq!(at.get_mode("b/b"), 0o40555);
+    assert_eq!(at.get_mode("b/b/c"), 0o40555);
+    assert_eq!(at.get_mode("b/b/c/d"), 0o40555);
+
+    assert_eq!(at.get_mode("c"), 0o40555);
+    assert_eq!(at.get_mode("c/b"), 0o40555);
+    assert_eq!(at.get_mode("c/b/c"), 0o40555);
+    assert_eq!(at.get_mode("c/b/c/d"), 0o40555);
+}
+
+#[test]
+#[cfg(not(target_os = "windows"))]
+fn test_cp_no_preserve_directory_permissions() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let dir = "a/b/c/d";
+    let file = "foo.txt";
+
+    at.mkdir_all(dir);
+
+    let file_path = format!("{dir}/{file}");
+
+    at.touch(file_path);
+
+    scene.cmd("chmod").arg("-R").arg("555").arg("a").succeeds();
+    scene.cmd("cp").arg("-r").arg("a").arg("b").succeeds();
+
+    scene
+        .ucmd()
+        .arg("-r")
+        .arg("--no-preserve=mode")
+        .arg("a")
+        .arg("c")
+        .succeeds();
+
+    assert_eq!(at.get_mode("b"), 0o40555);
+    assert_eq!(at.get_mode("b/b"), 0o40555);
+    assert_eq!(at.get_mode("b/b/c"), 0o40555);
+    assert_eq!(at.get_mode("b/b/c/d"), 0o40555);
+
+    assert_eq!(at.get_mode("c"), 0o40755);
+    assert_eq!(at.get_mode("c/b"), 0o40755);
+    assert_eq!(at.get_mode("c/b/c"), 0o40755);
+    assert_eq!(at.get_mode("c/b/c/d"), 0o40755);
+}

tests/uutests/src/lib/util.rs

@@ -1324,6 +1324,18 @@ impl AtPath {
         perms.set_mode(mode);
         std::fs::set_permissions(&path, perms).unwrap();
     }
+
+    /// Get the permissions of the specified file.
+    ///
+    /// # Panics
+    ///
+    /// This function panics if there is an error loading the metadata
+    #[cfg(not(windows))]
+    pub fn get_mode(&self, filename: &str) -> u32 {
+        let path = self.plus(filename);
+        let perms = std::fs::metadata(&path).unwrap().permissions();
+        perms.mode()
+    }
 }
 
 /// An environment for running a single uutils test case, serves three functions: