rm: skip prompt when stdin is not interactive; Fix #7326

[bitspill]

There's a currently redundant check in prompt_file_permission_readonly which is unreachable due to being under prompt_file but we add the extra check just in case the prompt should ever be used in a different codepath

[sylvestre]

Could you please add a test to make sure we don't regress? Thanks

[bitspill]

Could you please add a test to make sure we don't regress? Thanks

I'm going through the tests now which previously used a pipe to stdin and thus failing since they don't get the tested prompts by adding .terminal_simulation(true) I'll come up with a regression test to fit in as well.

This brings me to a larger question.... Windows (where I'm developing) doesn't support .terminal_simulation(true) on UCommand so we're in a bit of a pickle. Should interactive tests only be executed on unix systems, or shall I modify the public interface Options to add the -presume-input-tty option; Unix systems can simulate a terminal in their tests, while windows systems can presume input tty? And of course add some new tests for presume input tty on unix platforms set and unset. Feels a little off to put an "undocumented" argument into the public API but not sure how else to pass it along

Edit: going to call it a night and circle back tomorrow after some thoughts on presume-input-tty, suggestions or comment very much welcome

[github-actions]

GNU testsuite comparison:

GNU test failed: rm/d-3.log. rm/d-3.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/i-1.log. rm/i-1.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/interactive-always.log. rm/interactive-always.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/interactive-once.log. rm/interactive-once.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/ir-1.log. rm/ir-1.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/isatty.log. rm/isatty.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/rm3.log. rm/rm3.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/rm5.log. rm/rm5.log is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test misc/stdbuf.log is no longer failing!

[bitspill]

@kaathewisegit Since it was your comment about the public interface, do you have thoughts on the addition of __presume_input_tty in the Options struct?

I don't see any dependents described at crates.io and searched through NuShell, they're not yet using uu_rm so this doesn't appear to break other projects yet

[github-actions]

GNU testsuite comparison:

GNU test failed: rm/d-3.log. rm/d-3.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/i-1.log. rm/i-1.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/interactive-always.log. rm/interactive-always.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/interactive-once.log. rm/interactive-once.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/ir-1.log. rm/ir-1.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/rm3.log. rm/rm3.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/rm5.log. rm/rm5.log is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test rm/empty-inacc.log is no longer failing!

[kaathewisegit]

Huh, that was quite awhile ago, let me see.

Nope, that'll be a problem. Since Options and its fields are fully public, adding a new field will break constructing options via Options { /* all fields */ }, which is technically a breaking change. Not sure how much of a problem it is if nobody uses it.

One thing I'd suggest in general is implementing Default and putting non_exhaustive on those structs, so that the option list can be expanded without breaking backwards-compatibility. But that in itself would be a breaking change, too

[github-actions]

GNU testsuite comparison:

GNU test failed: rm/d-3.log. rm/d-3.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/i-1.log. rm/i-1.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/interactive-always.log. rm/interactive-always.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/interactive-once.log. rm/interactive-once.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/ir-1.log. rm/ir-1.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/rm3.log. rm/rm3.log is passing on 'main'. Maybe you have to rebase?
GNU test failed: rm/rm5.log. rm/rm5.log is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test rm/empty-inacc.log is no longer failing!

[bitspill]

For now I've added an implementation for Default but held off on adding non_exhaustive as that results in a much more involved reworking to add a new function and/or some setter functions to build out an Options instance which need more thought with regards to a proper design. For now I'll contribute Default and the tty fix but defer addition of non_exhaustive to someone with a deeper understanding of the uutils architecture/design for a pattern that would be desired across all utilities

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/rm/interactive-always. tests/rm/interactive-always is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/interactive-once. tests/rm/interactive-once is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/rm/empty-inacc is no longer failing!
Congrats! The gnu test tests/timeout/timeout is no longer failing!

[bitspill]

Greatly simplified the changes here after realizing I had read the man page incorrectly. It only skips the prompts while non-interactive and InteractiveMode.PromptProtected, if either -i/--interactive=always or -I/--interactive=once is passed it'll always prompt, I mistakenly read that as only applying to -i/--interactive=always

Not sure what that's about with the rustfmt style failing I got the opposite order on my windows machine, setup an Ubuntu 24.04 vm with fresh install of Rust to check and see if there was a version difference but I see the same, opposite that in the CI runner.

vboxuser@uutils:~/Desktop/coreutils/src/uu/rm/src$ rustfmt --check rm.rs --verbose
Using rustfmt config file /home/vboxuser/Desktop/coreutils/.rustfmt.toml for /home/vboxuser/Desktop/coreutils/src/uu/rm/src/rm.rs
Formatting /home/vboxuser/Desktop/coreutils/src/uu/rm/src/rm.rs
Diff in /home/vboxuser/Desktop/coreutils/src/uu/rm/src/rm.rs:8:
 use clap::{builder::ValueParser, parser::ValueSource, Arg, ArgAction, Command};⏎
 use std::ffi::{OsStr, OsString};⏎
 use std::fs::{self, Metadata};⏎
-use std::io::{IsTerminal, stdin};⏎
+use std::io::{stdin, IsTerminal};⏎
 use std::ops::BitOr;⏎
 #[cfg(not(windows))]⏎
 use std::os::unix::ffi::OsStrExt;⏎
Spent 0.003 secs in the parsing phase, and 0.008 secs in the formatting phase
vboxuser@uutils:~/Desktop/coreutils/src/uu/rm/src$ rustfmt --version
rustfmt 1.8.0-stable (05f9846f89 2025-03-31)
vboxuser@uutils:~/Desktop/coreutils/src/uu/rm/src$ cargo --version
cargo 1.86.0 (adf9b6ad1 2025-02-28)
vboxuser@uutils:~/Desktop/coreutils/src/uu/rm/src$ rustc +stable --version --verbose
rustc 1.86.0 (05f9846f8 2025-03-31)
binary: rustc
commit-hash: 05f9846f893b09a1be1fc8560e33fc3c815cfecb
commit-date: 2025-03-31
host: x86_64-unknown-linux-gnu
release: 1.86.0
LLVM version: 19.1.7

Also looks like still got one of the GNU test cases failing, will correct that tomorrow.

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/rm/interactive-once. tests/rm/interactive-once 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)
Congrats! The gnu test tests/rm/empty-inacc is no longer failing!

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/rm/empty-inacc is no longer failing!

[bitspill]

Squashed to a single commit, rebased, and manually adjusted the order of the std::io import to match CI expectations

use std::io::{IsTerminal, stdin};
vs
use std::io::{stdin, IsTerminal};

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/rm/empty-inacc is no longer failing!

[bitspill]

Looks to me like CICD / Build/SELinux (Pull_request) experienced an unrelated (intermittent?) failure in test_tail::test_follow_when_files_are_pointing_to_same_relative_file_and_file_stays_same_size, can we trigger the run again?

[bitspill]

Rebased

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/rm/empty-inacc is no longer failing!

[bitspill]

Looks like an intermittent tail failure again #3929

thread 'test_tail::test_retry9' panicked at tests/by-util/test_tail.rs:1609:10:
assertion failed: `(left == right)`

Diff < left / right > :
 tail: 'parent_dir/watched_file' has become inaccessible: No such file or directory
>tail: directory containing watched file was removed
>tail: inotify cannot be used, reverting to polling
>tail: 'parent_dir/watched_file' has appeared;  following new file
>tail: 'parent_dir/watched_file' has become inaccessible: No such file or directory
>tail: 'parent_dir/watched_file' has appeared;  following new file
>tail: 'parent_dir/watched_file' has become inaccessible: No such file or directory
>tail: 'parent_dir/watched_file' has appeared;  following new file

failures:
    test_tail::test_retry9

[sylvestre]

kudos