From 57380511dfdf1d40e9624b35b7eff826fcbe3867 Mon Sep 17 00:00:00 2001 From: valoq Date: Sun, 21 Jun 2026 01:26:55 +0200 Subject: [PATCH] fix exit code --- CHANGELOG.md | 2 ++ src/utils/question.rs | 9 ++++++--- tests/integration.rs | 34 ++++++++++++++++++++++++++++++++++ tests/utils.rs | 21 +++++++++++++-------- 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dad2816b1..f5ccd596f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ Categories Used: ### Bug Fixes +- Decompress: exit with a non-zero code when a file conflict can't be resolved on non-interactive stdin, instead of silently skipping and reporting success (https://github.com/ouch-org/ouch/pull/1011) + ### Tweaks diff --git a/src/utils/question.rs b/src/utils/question.rs index 2462481ed..075e7f478 100644 --- a/src/utils/question.rs +++ b/src/utils/question.rs @@ -205,9 +205,12 @@ impl<'a, T: Default> ChoicePrompt<'a, T> { let message = self.prompt; if is_stdin_dev_null()? { - eprintln!("{message}"); - eprintln!("Stdin is null, can't read user input (bypass with --yes, but be careful)"); - return Ok(T::default()); + // stdin is /dev/null so the conflict prompt cannot be answered; fail instead of silently skipping + let error = FinalError::with_title("Cannot read input to resolve file conflict") + .detail(format!("Tried to ask: \"{message}\"")) + .detail("Stdin is connected to /dev/null, so the prompt cannot be answered.") + .hint("If using Ouch in scripting, consider using `--yes` and `--no`."); + return Err(error.into()); } let _locks = lock_and_flush_output_stdio()?; diff --git a/tests/integration.rs b/tests/integration.rs index 4c30533b7..2a2b8f7c6 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1569,3 +1569,37 @@ fn decompress_concatenated_lz4_frames() { encoder.finish().unwrap() }); } + +/// A conflict that cannot be resolved because stdin is /dev/null must fail, not exit 0. +#[test] +fn decompress_conflict_with_dev_null_stdin_exits_nonzero() { + use std::process::Stdio; + + let (_tempdir, dir) = testdir().unwrap(); + fs::create_dir(dir.join("d")).unwrap(); + fs::write(dir.join("d").join("f.txt"), b"hi").unwrap(); + + crate::utils::cargo_bin() + .current_dir(dir) + .args(["compress", "d", "a.zip", "--yes"]) + .assert() + .success(); + // first extraction creates ./a, so the next run hits a conflict + crate::utils::cargo_bin() + .current_dir(dir) + .args(["decompress", "a.zip", "--yes"]) + .assert() + .success(); + + // stdin is /dev/null so the conflict cannot be answered; the run must fail instead of exiting 0 + let status = crate::utils::cargo_bin_command() + .current_dir(dir) + .args(["decompress", "a.zip"]) + .stdin(Stdio::null()) + .status() + .unwrap(); + assert!( + !status.success(), + "decompress with an unresolvable conflict on /dev/null stdin must exit non-zero" + ); +} diff --git a/tests/utils.rs b/tests/utils.rs index 443dd38db..45fc1d2c9 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -23,16 +23,21 @@ macro_rules! ouch { } pub fn cargo_bin() -> Command { + Command::from_std(cargo_bin_command()) +} + +/// Like [`cargo_bin`] but returns a `std::process::Command`, so callers can set stdin/stdout. +pub fn cargo_bin_command() -> std::process::Command { + let bin = assert_cmd::cargo::cargo_bin("ouch"); env::vars() - .find_map(|(k, v)| { - (k.starts_with("CARGO_TARGET_") && k.ends_with("_RUNNER")).then(|| { - let mut runner = v.split_whitespace(); - let mut cmd = Command::new(runner.next().unwrap()); - cmd.args(runner).arg(assert_cmd::cargo::cargo_bin("ouch")); - cmd - }) + .find(|(k, _)| k.starts_with("CARGO_TARGET_") && k.ends_with("_RUNNER")) + .map(|(_, runner)| { + let mut parts = runner.split_whitespace(); + let mut cmd = std::process::Command::new(parts.next().unwrap()); + cmd.args(parts).arg(&bin); + cmd }) - .unwrap_or_else(|| Command::cargo_bin("ouch").expect("Failed to find ouch executable")) + .unwrap_or_else(|| std::process::Command::new(&bin)) } pub fn testdir() -> io::Result<(tempfile::TempDir, &'static Path)> {