rm: skip prompt when stdin is not interactive; Fix #7326 by bitspill · Pull Request #7492 · uutils/coreutils

bitspill · 2025-03-19T07:52:37Z

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 · 2025-03-19T08:09:05Z

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

bitspill · 2025-03-19T08:26:57Z

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 · 2025-03-19T08:28:46Z

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 · 2025-03-20T06:02:55Z

@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 · 2025-03-20T06:38:00Z

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 · 2025-03-20T09:51:47Z

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 · 2025-03-20T10:26:38Z

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 · 2025-03-24T11:15:52Z

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 · 2025-03-24T13:17:40Z

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 · 2025-04-08T13:11:52Z

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 · 2025-04-08T13:16:13Z

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 · 2025-04-08T14:09:21Z

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 · 2025-04-08T23:22:31Z

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 · 2025-04-09T05:58:20Z

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 · 2025-04-09T08:45:01Z

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 · 2025-04-27T07:40:19Z

Rebased

github-actions · 2025-04-27T08:22:10Z

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 · 2025-04-27T08:37:37Z

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 · 2025-04-27T21:54:20Z

kudos

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

bitspill mentioned this pull request Mar 19, 2025

rm: shouldn't prompt when stdin is not interactive #7326

Closed

bitspill force-pushed the rm branch from 553f24e to a993ebd Compare March 20, 2025 06:13

cakebaker linked an issue Mar 20, 2025 that may be closed by this pull request

rm: shouldn't prompt when stdin is not interactive #7326

Closed

bitspill force-pushed the rm branch from a993ebd to d97e74b Compare March 24, 2025 11:08

bitspill force-pushed the rm branch from d97e74b to 43bf7dc Compare April 8, 2025 12:40

bitspill force-pushed the rm branch from 7958fd0 to a53cd29 Compare April 8, 2025 23:20

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

d732064

bitspill force-pushed the rm branch from a53cd29 to d732064 Compare April 27, 2025 07:39

bitspill mentioned this pull request Apr 27, 2025

test_tail::test_retry9 is failing intermittently #3929

Closed

sylvestre merged commit d957e64 into uutils:main Apr 27, 2025

bitspill deleted the rm branch April 27, 2025 22:22

BrewTestBot mentioned this pull request May 24, 2025

uutils-coreutils 0.1.0 Homebrew/homebrew-core#224645

Merged

moonfruit mentioned this pull request May 26, 2025

uutils-selected 0.1.0 moonfruit/homebrew-tap#243

Closed

nickorlow pushed a commit to nickorlow/coreutils that referenced this pull request Jul 17, 2025

Merge pull request uutils#7492 from bitspill/rm

3872e02

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

siraben mentioned this pull request Mar 14, 2026

mv: prompts to overwrite read-only file when stdin is not a terminal #11321

Open

venoosoo mentioned this pull request Mar 16, 2026

mv: don't prompt to overwrite read-only file when stdin is not a terminal #11346

Closed

venoosoo mentioned this pull request Oct 2, 2026

mv: don't prompt to overwrite read-only file when stdin is not a terminal (recovered PR #11346) #15035

Open

Uh oh!

Conversation

bitspill commented Mar 19, 2025

Uh oh!

sylvestre commented Mar 19, 2025

Uh oh!

bitspill commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Mar 19, 2025

Uh oh!

bitspill commented Mar 20, 2025

Uh oh!

github-actions Bot commented Mar 20, 2025

Uh oh!

kaathewisegit commented Mar 20, 2025

Uh oh!

github-actions Bot commented Mar 20, 2025

Uh oh!

bitspill commented Mar 24, 2025

Uh oh!

github-actions Bot commented Mar 24, 2025

Uh oh!

bitspill commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 8, 2025

Uh oh!

github-actions Bot commented Apr 8, 2025

Uh oh!

bitspill commented Apr 8, 2025

Uh oh!

github-actions Bot commented Apr 9, 2025

Uh oh!

bitspill commented Apr 9, 2025

Uh oh!

bitspill commented Apr 27, 2025

Uh oh!

github-actions Bot commented Apr 27, 2025

Uh oh!

bitspill commented Apr 27, 2025

Uh oh!

sylvestre commented Apr 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bitspill commented Mar 19, 2025 •

edited

Loading

bitspill commented Apr 8, 2025 •

edited

Loading