rm: fix for -rf ./ and variants silently delete current directory con…

[jacek-kurlit]

I have added checks for ./ ad ../ as I can see other variants are already covered.

Did I missed any other cases?
I have asked AI (both gemini and gpt) but it seems to provide covered use cases like:

Input
././.
../..
dir/.
dir/..

I'm not sure about preventing removing root like '/' or '//' because I don't have enough courage to check if coreutils will react same as gnu rm

rm: it is dangerous to operate recursively on '/'
rm: use --no-preserve-root to override this failsafe

[github-actions]

GNU testsuite comparison:

Congrats! The gnu test tests/tty/tty-eof is no longer failing!

[cerdelen]

I was also looking at this issue but saw it was already taken.

I think it would be good if you added a regression test too. The test you edited just checks that the the message output is right but not if they do not silently delete the files/directories.

also i think you missed 1 case but im not 100% sure
|| path_bytes.ends_with(&[dir_separator, b'.', b'.', dir_separator])

[jacek-kurlit]

Hi @cerdelen
I think this tests this case .arg("d/../")
Also it may be good idea to add this test to check if no file were removed

[cerdelen]

the tests are okay but shouldn't in the coud in file src/uu/rm/src/rm.rs in the if in line 850+ it checks for

            || path_bytes.ends_with(&[dir_separator, b'.'])
            || path_bytes.ends_with(&[dir_separator, b'.', b'.'])
            || path_bytes.ends_with(&[dir_separator, b'.', dir_separator])

so

xyz/.
xyz/..
xyz/./

isnt xyz/../ missing

this is what i refered to with

|| path_bytes.ends_with(&[dir_separator, b'.', b'.', dir_separator])

[jacek-kurlit]

Checks for silent removal added

[jacek-kurlit]

@cerdelen it's already there at line 860 or I'm missing something?

[cerdelen]

Oh yes i with the github interface it got cropped out and i didnt see it. My bad!

[github-actions]

GNU testsuite comparison:

Congrats! The gnu test tests/tty/tty-eof is no longer failing!

[jacek-kurlit]

I'm confused why pipeline failed, I don't see any error message. I see that mac os build fails for other PRs too

[oech3]

I'm not 100% sure. But I think random failure of features::backup_control::tests::* is happening after 939ab03

[jacek-kurlit]

@oech3 seems like some issue with concurrency on Mac OS

[jacek-kurlit]

Ok I think that I have broken pipeline because I didn't add unsafe { env::remove_var(ENV_VERSION_CONTROL) }; in my previous PR. I will try to fix it

[github-actions]

GNU testsuite comparison:

Congrats! The gnu test tests/tty/tty-eof is no longer failing!

[sylvestre]

please add a comment to explain why you have to do that

[jacek-kurlit]

Comment in the code? No other removals have it, add it to all of them?

[sylvestre]

yeah, it isn't clear to me why this variable has to be removed here :)

[jacek-kurlit]

Because it can affect other tests that are using env, but this is fragile though because if test fails this will not be cleaned up

[sylvestre]

please add this information in the code

[jacek-kurlit]

added

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/sort/sort-stale-thread-mem. tests/sort/sort-stale-thread-mem is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/dd/no-allocate was skipped on 'main' but is now failing.
Note: The gnu test tests/misc/write-errors was skipped on 'main' but is now failing.

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/tty/tty-eof. tests/tty/tty-eof is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/printf/printf-surprise is now passing!
Note: The gnu test tests/dd/no-allocate was skipped on 'main' but is now failing.
Note: The gnu test tests/misc/write-errors was skipped on 'main' but is now failing.

[jacek-kurlit]

I don't know why other tests are failing, they don't seem to be related with this PR at all. I'm missing something here @sylvestre ?

[sylvestre]

yeah, just ignore that :)

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)