Merge pull request #3122 from xxyzz/chmod

chmod: replace walkdir with std::fs

Cargo.lock

@@ -2385,7 +2385,6 @@ dependencies = [
  "clap 3.0.10",
  "libc",
  "uucore",
- "walkdir",
 ]
 
 [[package]]

src/uu/chmod/Cargo.toml

@@ -18,7 +18,6 @@ path = "src/chmod.rs"
 clap = { version = "3.0", features = ["wrap_help", "cargo"] }
 libc = "0.2.42"
 uucore = { version=">=0.0.11", package="uucore", path="../../uucore", features=["fs", "mode"] }
-walkdir = "2.2"
 
 [[bin]]
 name = "chmod"

src/uu/chmod/src/chmod.rs

@@ -18,7 +18,6 @@ use uucore::libc::mode_t;
 #[cfg(not(windows))]
 use uucore::mode;
 use uucore::{format_usage, show_error, InvalidEncodingHandling};
-use walkdir::WalkDir;
 
 static ABOUT: &str = "Change the mode of each FILE to MODE.
  With --reference, change the mode of each FILE to that of RFILE.";
@@ -227,9 +226,19 @@ impl Chmoder {
             if !self.recursive {
                 r = self.chmod_file(file).and(r);
             } else {
-                for entry in WalkDir::new(&filename).into_iter().filter_map(|e| e.ok()) {
+                r = self.walk_dir(file);
-                    let file = entry.path();
+            }
-                    r = self.chmod_file(file).and(r);
+        }
+        r
+    }
+
+    fn walk_dir(&self, file_path: &Path) -> UResult<()> {
+        let mut r = self.chmod_file(file_path);
+        if !is_symlink(file_path) && file_path.is_dir() {
+            for dir_entry in file_path.read_dir()? {
+                let path = dir_entry?.path();
+                if !is_symlink(&path) {
+                    r = self.walk_dir(path.as_path());
                 }
             }
         }

tests/by-util/test_chmod.rs

@@ -334,19 +334,20 @@ fn test_chmod_recursive() {
     make_file(&at.plus_as_string("a/b/c/c"), 0o100444);
     make_file(&at.plus_as_string("z/y"), 0o100444);
 
+    // only the permissions of folder `a` and `z` are changed
+    // folder can't be read after read permission is removed
     ucmd.arg("-R")
         .arg("--verbose")
         .arg("-r,a+w")
         .arg("a")
         .arg("z")
-        .succeeds()
+        .fails()
-        .stdout_contains(&"to 0333 (-wx-wx-wx)")
+        .stderr_is("chmod: Permission denied");
-        .stdout_contains(&"to 0222 (-w--w--w-)");
 
-    assert_eq!(at.metadata("z/y").permissions().mode(), 0o100222);
+    assert_eq!(at.metadata("z/y").permissions().mode(), 0o100444);
-    assert_eq!(at.metadata("a/a").permissions().mode(), 0o100222);
+    assert_eq!(at.metadata("a/a").permissions().mode(), 0o100444);
-    assert_eq!(at.metadata("a/b/b").permissions().mode(), 0o100222);
+    assert_eq!(at.metadata("a/b/b").permissions().mode(), 0o100444);
-    assert_eq!(at.metadata("a/b/c/c").permissions().mode(), 0o100222);
+    assert_eq!(at.metadata("a/b/c/c").permissions().mode(), 0o100444);
     println!("mode {:o}", at.metadata("a").permissions().mode());
     assert_eq!(at.metadata("a").permissions().mode(), 0o40333);
     assert_eq!(at.metadata("z").permissions().mode(), 0o40333);
@@ -356,6 +357,23 @@ fn test_chmod_recursive() {
     }
 }
 
+#[test]
+#[allow(clippy::unreadable_literal)]
+fn test_chmod_recursive_read_permission() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir("a");
+    at.mkdir("a/b");
+    let mut perms = at.metadata("a/b").permissions();
+    perms.set_mode(0o311);
+    set_permissions(at.plus_as_string("a/b"), perms.clone()).unwrap();
+    set_permissions(at.plus_as_string("a"), perms).unwrap();
+
+    ucmd.arg("-R").arg("u+r").arg("a").succeeds();
+
+    assert_eq!(at.metadata("a").permissions().mode(), 0o40711);
+    assert_eq!(at.metadata("a/b").permissions().mode(), 0o40711);
+}
+
 #[test]
 fn test_chmod_non_existing_file() {
     new_ucmd!()
@@ -455,8 +473,8 @@ fn test_chmod_symlink_non_existing_file_recursive() {
 
     let expected_stdout = &format!(
         // spell-checker:disable-next-line
-        "mode of '{}' retained as 0755 (rwxr-xr-x)\nneither symbolic link '{}/{}' nor referent has been changed",
+        "mode of '{}' retained as 0755 (rwxr-xr-x)",
-        test_directory, test_directory, test_symlink
+        test_directory
     );
 
     // '-v': this should succeed without stderr