rm: added write-protected check for files

[stefins]

Hi,

This fixes #3813. also this doesn't check the permission of directories right now.

[sylvestre]

Looks like it broke a test:

---- test_rm::test_rm_interactive_never stdout ----
current_directory_resolved: 
touch: /tmp/.tmpgyUHXS/test_rm_interactive
run: /home/runner/work/coreutils/coreutils/target/debug/coreutils chmod 0 test_rm_interactive
run: /home/runner/work/coreutils/coreutils/target/debug/coreutils rm --interactive=never test_rm_interactive
thread 'test_rm::test_rm_interactive_never' panicked at 'assertion failed: !at.file_exists(file_2)', /home/runner/work/coreutils/coreutils/tests/by-util/test_rm.rs:348:5

And please add a new test showing that the initial issue is fixed
thanks

[stefins]

In this line

coreutils/src/uu/rm/src/rm.rs

Line 115 in 2aede03

InteractiveMode::Never

we are making InteractiveMode::Never as default without any arguments but in case of #3813 we need to be interactive in default mode .

Maybe we can add a new InteractiveMode?

[sylvestre]

Sure, you can do that :)

[tertsdiepraam]

I'm not sure about this, but it sounds like this issue is separate from the existing InteractiveMode. I.e. there is prompting for recursive removal and for removal of protected files. If they are linked I would like the naming to be a bit more descriptive than Default, maybe PromptProtected or something like that (I'm sure there's a better name).

[stefins]

@tertsdiepraam Ok I'll change the name to PromptProtected

[sylvestre]

Fails on windows

failures:

---- test_rm::test_prompt_write_protected_no stdout ----
current_directory_resolved: 
touch: C:\Users\RUNNER~1\AppData\Local\Temp\.tmp0yZTql\test_rm_prompt_write_protected_2
run: D:\a\coreutils\coreutils\target\debug\coreutils.exe rm test_rm_prompt_write_protected_2
thread 'test_rm::test_prompt_write_protected_no' panicked at 'assertion failed: at.file_exists(file_2)', D:\a\coreutils\coreutils\tests\by-util\test_rm.rs:394:5


failures:
    test_rm::test_prompt_write_protected_no

[sylvestre]

To test on Windows:
https://developer.microsoft.com/windows/downloads/virtual-machines/

[stefins]

@sylvestre removed the failing test!

[sylvestre]

@stefins why did you remove it ?

[stefins]

It was failing on all the OS's!

Edit: Also we have another test to test the function

[sylvestre]

sure but it is an issue with the test or with the code ?
was it failing on linux too ?

[stefins]

Yeah It was failing on ubuntu also but only on these jobs

Build/stable (ubuntu-latest, feat_os_unix)
Build/stable (macos-latest, feat_os_macos)
Build/stable (windows-latest, feat_os_windows)
Build/nightly (ubuntu-latest, feat_os_unix)
Build/nightly (macos-latest, feat_os_macos)
Build/nightly (windows-latest, feat_os_windows)

https://github.com/uutils/coreutils/actions/runs/2913039044

[tertsdiepraam]

Seems like the cfg(feature = chmod) should apply to the test signature instead of just the single line. Otherwise it will fail on all platforms that don't have chmod enabled. Maybe that fixes the test?

[stefins]

@tertsdiepraam done

[tertsdiepraam]

Looks like it worked! I think it looks good now but I'll leave the final check to Sylvestre

[sylvestre]

Almost ready, the error message isn't exactly the same in the CI with the GNU testsuite:

2022-08-27T15:00:12.1685929Z 
2022-08-27T15:00:12.1686024Z FAIL: tests/rm/isatty
2022-08-27T15:00:12.1686231Z =====================
2022-08-27T15:00:12.1686342Z 
2022-08-27T15:00:12.1686519Z --- exp	2022-08-27 14:25:17.743717230 +0000
2022-08-27T15:00:12.1686821Z +++ out	2022-08-27 14:25:17.743717230 +0000
2022-08-27T15:00:12.1687062Z @@ -1 +1 @@
2022-08-27T15:00:12.1687367Z -rm: remove write-protected regular empty file 'f'? x
2022-08-27T15:00:12.1687716Z +rm: remove write-protected file 'f'? x
2022-08-27T15:00:12.1687985Z FAIL tests/rm/isatty.sh (exit status: 1)

https://pipelines.actions.githubusercontent.com/serviceHosts/91055393-6e6f-4662-ba97-3acee74a8c7c/_apis/pipelines/1/runs/16689/signedlogcontent/4?urlExpires=2022-08-29T11%3A35%3A26.1596317Z&urlSigningMethod=HMACV1&urlSignature=vjqV0OfnErkx%2BuWWQjyk6BXKVEkNlSy9i7NYUjHwAnc%3D

[stefins]

@sylvestre the test seems to be failing in the main branch also

[sylvestre]

Not sure what you mean but it works here:

Warning: Congrats! The gnu test tests/rm/isatty is no longer failing!

[stefins]

This test https://github.com/uutils/coreutils/runs/8071421179?check_suite_focus=true

[sylvestre]

oh that, don't bother ;)
we have intermittent issues in the CI

[sylvestre]

well done ;)