rm: -v -r should match GNU's output

[sylvestre]

with rm from GNU:

$ mkdir a
$ rm -rf -v a
removed directory 'a'

With the Rust implementation:

$ mkdir a
$ rm -rf -v a
removed 'a'

[goodbadwolf]

I would like to take this up if no one else is

[billyb2]

Interestingly, when I use rm -rf -v a, I don't get any output at all (on the rust implementation).

Using latest version of Manjaro Linux, and the latest version of Rust stable.

[jhscheer]

Interestingly, when I use rm -rf -v a, I don't get any output at all (on the rust implementation).

Using latest version of Manjaro Linux, and the latest version of Rust stable.

I also noticed that. However, I think the println fix should be inside remove_dir() not in handle_dir().
The funny thing is, remove_dir() is never called with rm -rf -v a.
To me it looks like every remove happens inside remove_dir_all (which is external).

Also note, that this test https://github.com/coreutils/coreutils/blob/master/tests/rm/v-slash.sh needs to be successful.
I ported it to test_rm.rs, but couldn't test it because I tripped over the fact that remove_dir() isn't called.

#[test]
fn test_rm_verbose_slash() {
    let (at, mut ucmd) = at_and_ucmd!();
    let dir = "test_rm_verbose_slash_directory";
    let file_a = &format!("{}/test_rm_verbose_slash_file_a", dir);

    at.mkdir(dir);
    at.touch(file_a);

    ucmd.arg("-r")
        .arg("-f")
        .arg("-v")
        .arg(&format!("{}///", dir))
        .succeeds()
        .stdout_only(format!(
            "removed '{}'\nremoved directory '{}'\n",
            file_a, dir
        ));

    assert!(!at.dir_exists(dir));
    assert!(!at.file_exists(file_a));
}

[billyb2]

@jhscheer Yeah that tripped me up too when I was writing my pull, I want to look for where remove_dir is called in the program but unfortunately my editor isn't all that great

[jhscheer]

I looked at it some more.

If rm -rf is called on a directory, we get handle_dir -> remove_dir_all
If rm -rfi is called on a directory, we get handle_dir -> remove_dir

However, the comment L249-250, indicates that remove_dir_all is only used for windows compatibility.

// we need the extra crate because apparently fs::remove_dir_all() does not function
// correctly on Windows

1. I think the stdout for -v should be fixed in remove_dir.
2. To me it looks like not calling remove_dir with flag -f is a bug.
3. To keep the stdout between windows systems identical to non windows system, there should probably be a cfg(windows) in remove_dir and the remove_dir_all should be moved from handle_dir to remove_dir.

Is this the way to go?

[jhscheer]

This was fixed in #1839 and can be closed.

[JnRouvignac]

Another one that can be closed apparently :)

[sylvestre]

thanks
please contact me sylvestre@debian.org if you are looking for good first bug!