-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rm: when error during recursive, stops prematurely #3722
Comments
I noticed that this issue with the test added above can be passed at the current main branch (615b562). ❯ cargo test test_rm
Compiling coreutils v0.0.22 (/home/arch/code/tommady/coreutils)
Finished test [unoptimized + debuginfo] target(s) in 2.67s
Running unittests src/bin/coreutils.rs (target/debug/deps/coreutils-057d678475d87ea4)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Running tests/test_util_name.rs (target/debug/deps/test_util_name-befa9301860f5b3e)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 61 filtered out; finished in 0.00s
Running tests/tests.rs (target/debug/deps/tests-49e604e72fb6a35c)
running 44 tests
test test_rm::test_rm_descend_directory ... ok
test test_rm::test_fifo_removal ... ok
test test_rm::test_rm_directory_without_flag ... ok
test test_rm::test_rm_force_multiple ... ok
test test_rm::test_rm_force ... ok
test test_rm::test_rm_empty_directory_verbose ... ok
test test_rm::test_invalid_arg ... ok
test test_rm::test_rm_interactive_never ... ok
test test_rm::test_rm_empty_directory ... ok
test test_rm::test_rm_force_no_operand ... ok
test test_rm::test_non_utf8 ... ok
test test_rm::test_rm_directory_rights_rm1 ... ok ---> here
test test_rm::test_rm_interactive_missing_value ... ok
test test_rm::test_rm_failed ... ok
test test_rm::test_rm_force_prompts_order ... ok
test test_rm::test_rm_invalid_symlink ... ok
test test_rm::test_rm_no_operand ... ok
test test_rm::test_rm_interactive_once_recursive_prompt ... ok
test test_rm::test_rm_interactive_once_prompt ... ok
test test_rm::test_rm_multiple_files ... ok
test test_rm::test_rm_recursive ... ok
test test_rm::test_rm_symlink_dir ... ok
test test_rm::test_rm_non_empty_directory ... ok
test test_rm::test_rm_silently_accepts_presume_input_tty2 ... ok
test test_rm::test_rm_verbose_slash ... ok
test test_rmdir::test_invalid_arg ... ok
test test_rm::test_rm_recursive_multiple ... ok
test test_rm::test_rm_verbose ... ok
test test_rm::test_rm_one_file ... ok
test test_rmdir::test_rmdir_remove_symlink_dangling ... ok
test test_rmdir::test_rmdir_empty_directory_with_parents ... ok
test test_rmdir::test_rmdir_ignore_nonempty_directory_no_parents ... ok
test test_rmdir::test_rmdir_remove_symlink_file ... ok
test test_rmdir::test_rmdir_empty_directory_no_parents ... ok
test test_rmdir::test_rmdir_ignore_nonempty_no_permissions ... ok
test test_rmdir::test_rmdir_not_a_directory ... ok
test test_rmdir::test_rmdir_ignore_nonempty_directory_with_parents ... ok
test test_rm::test_rm_interactive ... ok
test test_rmdir::test_rmdir_nonempty_directory_no_parents ... ok
test test_rmdir::test_rmdir_nonempty_directory_with_parents ... ok
test test_rmdir::test_verbose_multi ... ok
test test_rmdir::test_rmdir_remove_symlink_dir ... ok
test test_rmdir::test_verbose_single ... ok
test test_rmdir::test_verbose_nested_failure ... ok
test result: ok. 44 passed; 0 failed; 0 ignored; 0 measured; 2504 filtered out; finished in 0.06s so could we consider removing the ignore flag on the test case and closing this PR? |
I presume that was due to this change? #5403 If that's the case let's add the test you made and then close this! |
Hm, while the test passes when removing the ignore flag, doing the steps manually doesn't remove the folders here on my machine:
|
ya interesting the CI also failed... |
Hi, I have a question at rm.rs:331 331: 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);
}
}
}
349: } else {
...
} why does the code need to be separated into two parts, is it because of the performance? |
nope, the comment tries to explain it
mostly because remove_dir_all is doing a better job than remove_dir on files with incorrect permissions |
Sorry, my presentation was not clear enough. the way and the thoughts when I see those codes, so what I tried to ask is why separate them?
Those minor conditions separate A and B:
please guide me, thanks! |
uutils doesn't remove
b/d
in the following example$ mkdir -p b/a/p $ mkdir b/c b/d $ chmod -w b/a $ rm -rf b rm: cannot remove 'b': Permission denied $ ls b a d
but GNU removes
b/d
That's probably because of
fs::remove_dir_all
behaviorThe text was updated successfully, but these errors were encountered: