From 2fb45c188e51f8558d6c72c4968486ee9f061232 Mon Sep 17 00:00:00 2001 From: Chris Dryden Date: Mon, 19 Jan 2026 16:39:25 -0500 Subject: [PATCH] rm: fix error reporting for -r on Linux fixing #9011 (#10111) * rm: fix error reporting for -r on Linux * rm: add stricter assertion for error tracking test --------- Co-authored-by: Alex Lyon --- src/uu/rm/src/platform/unix.rs | 12 ++++++------ tests/by-util/test_rm.rs | 7 ++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/uu/rm/src/platform/unix.rs b/src/uu/rm/src/platform/unix.rs index 5c8e0981b..e890ab158 100644 --- a/src/uu/rm/src/platform/unix.rs +++ b/src/uu/rm/src/platform/unix.rs @@ -371,7 +371,7 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt let entry_stat = match dir_fd.stat_at(&entry_name, false) { Ok(stat) => stat, Err(e) => { - error = handle_error_with_force(e, &entry_path, options); + error |= handle_error_with_force(e, &entry_path, options); continue; } }; @@ -395,21 +395,21 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt // If we can't open the subdirectory for safe traversal, // try to handle it as best we can with safe operations if e.kind() == std::io::ErrorKind::PermissionDenied { - error = handle_permission_denied( + error |= handle_permission_denied( dir_fd, entry_name.as_ref(), &entry_path, options, ); } else { - error = handle_error_with_force(e, &entry_path, options); + error |= handle_error_with_force(e, &entry_path, options); } continue; } }; let child_error = safe_remove_dir_recursive_impl(&entry_path, &child_dir_fd, options); - error = error || child_error; + error |= child_error; // Ask user permission if needed for this subdirectory if !child_error @@ -421,12 +421,12 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt // Remove the now-empty subdirectory using safe unlinkat if !child_error { - error = handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, true, options); + error |= handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, true, options); } } else { // Remove file - check if user wants to remove it first if prompt_file_with_stat(&entry_path, &entry_stat, options) { - error = handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, false, options); + error |= handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, false, options); } } } diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 3d3b1a9b1..20d4a9357 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -1140,9 +1140,10 @@ fn test_rm_directory_not_writable() { // Check for expected error message // When the parent directory (b/a) doesn't have write permission, - // we get "Permission denied" when trying to remove the subdirectory - let stderr = result.stderr_str(); - assert!(stderr.contains("rm: cannot remove 'b/a/p': Permission denied")); + // we get "Permission denied" when trying to remove the subdirectory. + // The error tracking must be correct so we don't attempt to remove the parent + // directory after child failure (which would produce extra "Directory not empty" errors). + result.stderr_only("rm: cannot remove 'b/a/p': Permission denied\n"); // Check which directories still exist assert!(at.dir_exists("b/a/p")); // Should still exist (parent not writable)