fix exit code when stdin is /dev/null (#1011)

Author: valoq

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
 
 

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()?;

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"
+    );
+}

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)> {