diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 466a8d6c1..033a1a4aa 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -233,7 +233,7 @@ fn remove(files: Vec, options: Options) -> bool { // (e.g., permission), even rm -f should fail with // outputting the error, but there's no easy eay. if !options.force { - show_error!("no such file or directory '{}'", filename); + show_error!("cannot remove '{}': No such file or directory", filename); true } else { false @@ -289,7 +289,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool { had_err = true; } else { show_error!( - "could not remove directory '{}' (did you mean to pass '-r' or '-R'?)", + "cannot remove '{}': Is a directory", // GNU's rm error message does not include help path.display() ); had_err = true; @@ -305,16 +305,34 @@ fn remove_dir(path: &Path, options: &Options) -> bool { true }; if response { - match fs::remove_dir(path) { - Ok(_) => { - if options.verbose { - println!("removed '{}'", path.display()); + if let Ok(mut read_dir) = fs::read_dir(path) { + if options.dir || options.recursive { + if read_dir.next().is_none() { + match fs::remove_dir(path) { + Ok(_) => { + if options.verbose { + println!("removed directory '{}'", path.display()); + } + } + Err(e) => { + show_error!("cannot remove '{}': {}", path.display(), e); + return true; + } + } + } else { + // directory can be read but is not empty + show_error!("cannot remove '{}': Directory not empty", path.display()); + return true; } - } - Err(e) => { - show_error!("removing '{}': {}", path.display(), e); + } else { + // called to remove a symlink_dir (windows) without "-r"/"-R" or "-d" + show_error!("cannot remove '{}': Is a directory", path.display()); return true; } + } else { + // GNU's rm shows this message if directory is empty but not readable + show_error!("cannot remove '{}': Directory not empty", path.display()); + return true; } } diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index c3635d202..40cc6839a 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -17,7 +17,12 @@ fn test_rm_failed() { let (_at, mut ucmd) = at_and_ucmd!(); let file = "test_rm_one_file"; - ucmd.arg(file).fails(); // Doesn't exist + let result = ucmd.arg(file).fails(); // Doesn't exist + + assert!(result.stderr.contains(&format!( + "cannot remove '{}': No such file or directory", + file + ))); } #[test] @@ -115,6 +120,39 @@ fn test_rm_empty_directory() { assert!(!at.dir_exists(dir)); } +#[test] +fn test_rm_empty_directory_verbose() { + let (at, mut ucmd) = at_and_ucmd!(); + let dir = "test_rm_empty_directory_verbose"; + + at.mkdir(dir); + + ucmd.arg("-d") + .arg("-v") + .arg(dir) + .succeeds() + .stdout_only(format!("removed directory '{}'\n", dir)); + + assert!(!at.dir_exists(dir)); +} + +#[test] +fn test_rm_non_empty_directory() { + let (at, mut ucmd) = at_and_ucmd!(); + let dir = "test_rm_non_empty_dir"; + let file_a = &format!("{}/test_rm_non_empty_file_a", dir); + + at.mkdir(dir); + at.touch(file_a); + + let result = ucmd.arg("-d").arg(dir).fails(); + assert!(result + .stderr + .contains(&format!("cannot remove '{}': Directory not empty", dir))); + assert!(at.file_exists(file_a)); + assert!(at.dir_exists(dir)); +} + #[test] fn test_rm_recursive() { let (at, mut ucmd) = at_and_ucmd!(); @@ -134,22 +172,17 @@ fn test_rm_recursive() { } #[test] -fn test_rm_errors() { +fn test_rm_directory_without_flag() { let (at, mut ucmd) = at_and_ucmd!(); - let dir = "test_rm_errors_directory"; - let file_a = "test_rm_errors_directory/test_rm_errors_file_a"; - let file_b = "test_rm_errors_directory/test_rm_errors_file_b"; + let dir = "test_rm_directory_without_flag_dir"; at.mkdir(dir); - at.touch(file_a); - at.touch(file_b); - - // $ rm test_rm_errors_directory - // rm: error: could not remove directory 'test_rm_errors_directory' (did you mean to pass '-r'?) - ucmd.arg(dir).fails().stderr_is( - "rm: error: could not remove directory 'test_rm_errors_directory' (did you mean \ - to pass '-r' or '-R'?)\n", - ); + + let result = ucmd.arg(dir).fails(); + println!("{}", result.stderr); + assert!(result + .stderr + .contains(&format!("cannot remove '{}': Is a directory", dir))); } #[test] @@ -169,10 +202,13 @@ fn test_rm_verbose() { } #[test] -fn test_rm_dir_symlink() { +#[cfg(not(windows))] +// on unix symlink_dir is a file +fn test_rm_symlink_dir() { let (at, mut ucmd) = at_and_ucmd!(); - let dir = "test_rm_dir_symlink_dir"; - let link = "test_rm_dir_symlink_link"; + + let dir = "test_rm_symlink_dir_directory"; + let link = "test_rm_symlink_dir_link"; at.mkdir(dir); at.symlink_dir(dir, link); @@ -180,6 +216,29 @@ fn test_rm_dir_symlink() { ucmd.arg(link).succeeds(); } +#[test] +#[cfg(windows)] +// on windows removing symlink_dir requires "-r" or "-d" +fn test_rm_symlink_dir() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let dir = "test_rm_symlink_dir_directory"; + let link = "test_rm_symlink_dir_link"; + + at.mkdir(dir); + at.symlink_dir(dir, link); + + let result = scene.ucmd().arg(link).fails(); + assert!(result + .stderr + .contains(&format!("cannot remove '{}': Is a directory", link))); + + assert!(at.dir_exists(link)); + + scene.ucmd().arg("-r").arg(link).succeeds(); +} + #[test] fn test_rm_invalid_symlink() { let (at, mut ucmd) = at_and_ucmd!();