diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index 6c7d3239572..4c8c1104189 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -239,7 +239,7 @@ pub fn safe_remove_dir_recursive( // Before trying to remove the directory, check if it's actually empty // This handles the case where some children weren't removed due to user "no" responses - if !is_dir_empty(path) { + if !is_dir_empty(path).unwrap_or(false) { // Directory is not empty, so we can't/shouldn't remove it // In interactive mode, this might be expected if user said "no" to some children // In non-interactive mode, this indicates an error (some children couldn't be removed) @@ -292,7 +292,7 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt if is_dir { // Ask user if they want to descend into this directory if options.interactive == InteractiveMode::Always - && !is_dir_empty(&entry_path) + && !is_dir_empty(&entry_path).unwrap_or(false) && !prompt_descend(&entry_path) { continue; diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index a20a57d7f36..082e118cce8 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -527,8 +527,8 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool { /// /// `path` must be a directory. If there is an error reading the /// contents of the directory, this returns `false`. -fn is_dir_empty(path: &Path) -> bool { - fs::read_dir(path).is_ok_and(|mut iter| iter.next().is_none()) +fn is_dir_empty(path: &Path) -> Result { + fs::read_dir(path).map(|mut iter| iter.next().is_none()) } #[cfg(unix)] @@ -599,7 +599,7 @@ fn remove_dir_recursive( // Base case 2: this is a non-empty directory, but the user // doesn't want to descend into it. if options.interactive == InteractiveMode::Always - && !is_dir_empty(path) + && !is_dir_empty(path).unwrap_or(false) && !prompt_descend(path) { return false; @@ -685,35 +685,52 @@ fn remove_dir_recursive( } fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool { - let mut had_err = false; - let path = clean_trailing_slashes(path); - if path_is_current_or_parent_directory(path) { + + // Error priority: Is directory > Dir not empty > preserve-root > current/parent directory + + // Refuse to remove a directory without -r/-R or -d + if !options.recursive && !options.dir { show_error!( "{}", - RmError::RefusingToRemoveDirectory(path.as_os_str().to_os_string()) + RmError::CannotRemoveIsDirectory(path.as_os_str().to_os_string()) + ); + return true; + } + + // Refuse to remove non-empty directory without -r/-R + // If we can't read the directory, fall through to let remove_dir handle the error + if !options.recursive && !is_dir_empty(path).unwrap_or(true) { + show_error!( + "{}: Directory not empty", + translate!("rm-error-cannot-remove", "file" => path.quote()) ); return true; } + // Preserve root directory let is_root = path.has_root() && path.parent().is_none(); - if options.recursive && (!is_root || !options.preserve_root) { - had_err = remove_dir_recursive(path, options, progress_bar); - } else if options.dir && (!is_root || !options.preserve_root) { - had_err = remove_dir(path, options, progress_bar).bitor(had_err); - } else if options.recursive { + if is_root && options.preserve_root { show_error!("{}", RmError::DangerousRecursiveOperation); show_error!("{}", RmError::UseNoPreserveRoot); - had_err = true; - } else { + return true; + } + + // Refuse to remove current or parent directories + if path_is_current_or_parent_directory(path) { show_error!( "{}", - RmError::CannotRemoveIsDirectory(path.as_os_str().to_os_string()) + RmError::RefusingToRemoveDirectory(path.as_os_str().to_os_string()) ); - had_err = true; + return true; } - had_err + // Now we can safely remove the directory + if options.recursive { + remove_dir_recursive(path, options, progress_bar) + } else { + remove_dir(path, options, progress_bar) + } } /// Remove the given directory, asking the user for permission if necessary. diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 38230f2ad36..8017e2b2c3d 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -1217,3 +1217,33 @@ fn test_progress_no_output_on_error() { .stderr_contains("cannot remove") .stderr_contains("No such file or directory"); } + +/// Test error 'Is directory' comes before 'Dir not empty' +#[test] +fn test_error_message_priority1() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("d"); + at.touch("d/f"); + + ts.ucmd() + .arg("d") + .fails() + .stderr_is("rm: cannot remove 'd': Is a directory\n"); +} + +/// Test error 'Dir not empty' comes before 'Refusing to remove . or ..' +#[test] +fn test_error_message_priority2() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.touch("f"); + + ts.ucmd() + .arg("-d") + .arg(".") + .fails() + .stderr_is("rm: cannot remove '.': Directory not empty\n"); +}