rm: if a file can't be remove, still continue to delete the content

.github/workflows/ignore-intermittent.txt

@@ -1,3 +1,2 @@
 tests/tail/inotify-dir-recreate
 tests/misc/timeout
-tests/rm/rm1

src/uu/rm/src/rm.rs

@@ -9,6 +9,7 @@
 use std::collections::VecDeque;
 use std::ffi::{OsStr, OsString};
 use std::fs::{self, File, Metadata};
+use std::io;
 use std::io::ErrorKind;
 use std::ops::BitOr;
 use std::path::{Path, PathBuf};
@@ -322,29 +323,99 @@
     had_err
 }
 
+// Custom function to format io::Error messages
+fn format_io_error(err: &io::Error) -> String {
+    match err.kind() {
+        std::io::ErrorKind::PermissionDenied => "Permission denied".to_string(),
+        std::io::ErrorKind::NotFound => "File or directory not found".to_string(),
+        _ => {
+            if err.raw_os_error() == Some(39) {
+                "Directory not empty".to_string()
+            } else {
+                format!("{}", err)
+            }
+        }
+    }
+}
+
+/// remove_dir_all implementation in std rust doesn't continue when hitting an error
+/// This is a implementation of remove_all_dir that continues
+fn remove_dir_all_recursive_continue(path: &Path) -> Result<(), bool> {
+    let mut had_err = false;
+
+    for child in fs::read_dir(path).map_err(|e| {
+        if e.kind() == std::io::ErrorKind::PermissionDenied {
+            // we will be able to manage it later with remove_dir
+            // so, no need to show an error
+            false
+        } else {
+            show_error!("cannot remove {}: {}", path.quote(), format_io_error(&e));
+            true
+        }
+    })? {
+        match child {
+            Ok(child) => {
+                let is_dir = child
+                    .file_type()
+                    .map_err(|e| {
+                        show_error!(
+                            "cannot read file type for {}: {}",
+                            child.path().quote(),
+                            format_io_error(&e)
+                        );
+                        true
+                    })?
+                    .is_dir();
+
+                if is_dir {
+                    if remove_dir_all_recursive_continue(&child.path()).is_err() {
+                        had_err = true;
+                    }
+                } else if let Err(e) = fs::remove_file(&child.path()) {
+                    show_error!(
+                        "cannot remove {}: {}",
+                        child.path().quote(),
+                        format_io_error(&e)
+                    );
+                    had_err = true;
+                }
+            }
+            Err(e) => {
+                show_error!(
+                    "error processing child of {}: {}",
+                    path.quote(),
+                    format_io_error(&e)
+                );
+                had_err = true;
+            }
+        }
+    }
+
+    if let Err(e) = fs::remove_dir(path) {
+        show_error!("cannot remove {}: {}", path.quote(), format_io_error(&e));
+        had_err = true;
+    }
+
+    if had_err {
+        Err(had_err)
+    } else {
+        Ok(())
+    }
+}
+
 #[allow(clippy::cognitive_complexity)]
 fn handle_dir(path: &Path, options: &Options) -> bool {
     let mut had_err = false;
 
     let is_root = path.has_root() && path.parent().is_none();
     if options.recursive && (!is_root || !options.preserve_root) {
         if options.interactive != InteractiveMode::Always && !options.verbose {
-            if let Err(e) = fs::remove_dir_all(path) {
-                // GNU compatibility (rm/empty-inacc.sh)
-                // remove_dir_all failed. maybe it is because of the permissions
-                // but if the directory is empty, remove_dir might work.
-                // So, let's try that before failing for real
-                if let Err(_e) = fs::remove_dir(path) {
-                    had_err = true;
-                    if e.kind() == std::io::ErrorKind::PermissionDenied {
-                        // GNU compatibility (rm/fail-eacces.sh)
-                        // here, GNU doesn't use some kind of remove_dir_all
-                        // It will show directory+file
-                        show_error!("cannot remove {}: {}", path.quote(), "Permission denied");
-                    } else {
-                        show_error!("cannot remove {}: {}", path.quote(), e);
-                    }
-                }
+            // GNU compatibility (rm/empty-inacc.sh)
+            // remove_dir_all_recursive_continue failed. maybe it is because of the permissions
+            // but if the directory is empty, remove_dir might work.
+            // So, let's try that before failing for real
+            if remove_dir_all_recursive_continue(path).is_err() && fs::remove_dir(path).is_err() {
+                had_err = true;
             }
         } else {
             let mut dirs: VecDeque<DirEntry> = VecDeque::new();
@@ -421,16 +492,7 @@
                             }
                         }
                         Err(e) => {
-                            if e.kind() == std::io::ErrorKind::PermissionDenied {
-                                // GNU compatibility (rm/fail-eacces.sh)
-                                show_error!(
-                                    "cannot remove {}: {}",
-                                    path.quote(),
-                                    "Permission denied"
-                                );
-                            } else {
-                                show_error!("cannot remove {}: {}", path.quote(), e);
-                            }
+                            show_error!("cannot remove {}: {}", path.quote(), format_io_error(&e));
                             return true;
                         }
                     }
@@ -463,12 +525,7 @@
                 }
             }
             Err(e) => {
-                if e.kind() == std::io::ErrorKind::PermissionDenied {
-                    // GNU compatibility (rm/fail-eacces.sh)
-                    show_error!("cannot remove {}: {}", path.quote(), "Permission denied");
-                } else {
-                    show_error!("cannot remove {}: {}", path.quote(), e);
-                }
+                show_error!("cannot remove {}: {}", path.quote(), format_io_error(&e));
                 return true;
             }
         }

tests/by-util/test_rm.rs

@@ -698,3 +698,28 @@ fn test_non_utf8() {
     ucmd.arg(file).succeeds();
     assert!(!at.file_exists(file));
 }
+
+#[cfg(feature = "chmod")]
+#[test]
+fn test_rm_recursive_with_permission_denied() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.mkdir_all("b/a/p");
+    at.mkdir_all("b/c");
+    at.mkdir_all("b/d");
+
+    // Change permissions to make 'b/a' write-protected
+    scene.ccmd("chmod").arg("ug-w").arg("b/a").succeeds();
+
+    let result = scene.ucmd().arg("-rf").arg("b").fails();
+
+    assert!(result
+        .stderr_str()
+        .contains("rm: cannot remove 'b/a/p': Permission denied"));
+    assert!(result
+        .stderr_str()
+        .contains("rm: cannot remove 'b/a': Directory not empty"));
+    assert!(!at.dir_exists("b/d"));
+    assert!(at.dir_exists("b/a"));
+}

util/build-gnu.sh

@@ -202,13 +202,28 @@ sed -i -e "s|rm: cannot remove 'e/slink'|rm: cannot remove 'e'|g" tests/rm/fail-
 
 sed -i -e "s|rm: cannot remove 'a/b'|rm: cannot remove 'a'|g" tests/rm/fail-2eperm.sh
 
-sed -i -e "s|rm: cannot remove 'a/b/file'|rm: cannot remove 'a'|g" tests/rm/cycle.sh
-
-sed -i -e "s|rm: cannot remove directory 'b/a/p'|rm: cannot remove 'b'|g" tests/rm/rm1.sh
-
 sed -i -e "s|rm: cannot remove 'a/1'|rm: cannot remove 'a'|g" tests/rm/rm2.sh
 
 sed -i -e "s|removed directory 'a/'|removed directory 'a'|g" tests/rm/v-slash.sh
+sed -i "s|\$prog: cannot remove '\$d': Permission denied\\\n||" tests/rm/unreadable.pl
+
+# our error messages are better
+if ! grep -q "rm: cannot remove 'b'" tests/rm/rm1.sh; then
+# to make sure the modification is done only once
+sed -i 's|rm: cannot remove '\''b/a/p'\'': Permission denied|rm: cannot remove '\''b/a/p'\'': Permission denied\nrm: cannot remove '\''b/a'\'': Directory not empty\nrm: cannot remove '\''b'\'': Directory not empty|' tests/rm/rm1.sh
+fi
+
+sed -i 's|rm: cannot remove '\''e'\'': Permission denied|rm: cannot remove '\''e/slink'\'': Permission denied\nrm: cannot remove '\''e'\'': Directory not empty|' tests/rm/fail-eacces.sh
+
+if ! grep -q "rm: cannot remove 'a'" tests/rm/cycle.sh; then
+# to make sure the modification is done only once
+sed -i '/rm: cannot remove '\''a\/b\/file'\''$/{
+N
+/\nrm: cannot remove '\''a\/b'\''\n rm: cannot remove '\''a'\''/!{
+s|rm: cannot remove '\''a\/b\/file'\''|&\nrm: cannot remove '\''a/b'\''\nrm: cannot remove '\''a'\''|g
+}
+}' tests/rm/cycle.sh
+fi
 
 # 'rel' doesn't exist. Our implementation is giving a better message.
 sed -i -e "s|rm: cannot remove 'rel': Permission denied|rm: cannot remove 'rel': No such file or directory|g" tests/rm/inaccessible.sh