From f03ba6793e673fe7967fc09a4090ad72c3a39054 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Wed, 2 Oct 2024 04:04:18 -0700 Subject: [PATCH] Fix non-zero exit code errors in middle of pipeline (#13899) # Description Fixes #13868. Should come after #13885. # User-Facing Changes Bug fix. # Tests + Formatting Added a test. --- crates/nu-cli/src/print.rs | 8 +++++++- crates/nu-cmd-lang/src/core_commands/do_.rs | 3 ++- crates/nu-cmd-lang/src/core_commands/ignore.rs | 9 +++++++-- crates/nu-command/src/filesystem/save.rs | 2 ++ crates/nu-command/src/system/run_external.rs | 9 ++++++++- crates/nu-protocol/src/process/child.rs | 4 ++-- tests/shell/pipeline/mod.rs | 10 +++++++++- 7 files changed, 37 insertions(+), 8 deletions(-) diff --git a/crates/nu-cli/src/print.rs b/crates/nu-cli/src/print.rs index 66e736ca4f..ffb0366242 100644 --- a/crates/nu-cli/src/print.rs +++ b/crates/nu-cli/src/print.rs @@ -1,4 +1,5 @@ use nu_engine::command_prelude::*; +use nu_protocol::ByteStreamSource; #[derive(Clone)] pub struct Print; @@ -50,7 +51,7 @@ Since this command has no output, there is no point in piping it with other comm engine_state: &EngineState, stack: &mut Stack, call: &Call, - input: PipelineData, + mut input: PipelineData, ) -> Result { let args: Vec = call.rest(engine_state, stack, 0)?; let no_newline = call.has_flag(engine_state, stack, "no-newline")?; @@ -69,6 +70,11 @@ Since this command has no output, there is no point in piping it with other comm } } } else if !input.is_nothing() { + if let PipelineData::ByteStream(stream, _) = &mut input { + if let ByteStreamSource::Child(child) = stream.source_mut() { + child.ignore_error(true); + } + } if raw { input.print_raw(engine_state, no_newline, to_stderr)?; } else { diff --git a/crates/nu-cmd-lang/src/core_commands/do_.rs b/crates/nu-cmd-lang/src/core_commands/do_.rs index 3a48c66745..4fee08a79c 100644 --- a/crates/nu-cmd-lang/src/core_commands/do_.rs +++ b/crates/nu-cmd-lang/src/core_commands/do_.rs @@ -147,6 +147,7 @@ impl Command for Do { None }; + child.ignore_error(false); child.wait()?; let mut child = ChildProcess::from_raw(None, None, None, span); @@ -172,7 +173,7 @@ impl Command for Do { ) => { if let ByteStreamSource::Child(child) = stream.source_mut() { - child.ignore_error(); + child.ignore_error(true); } Ok(PipelineData::ByteStream(stream, metadata)) } diff --git a/crates/nu-cmd-lang/src/core_commands/ignore.rs b/crates/nu-cmd-lang/src/core_commands/ignore.rs index 2b3d31ba9f..382dc4fedd 100644 --- a/crates/nu-cmd-lang/src/core_commands/ignore.rs +++ b/crates/nu-cmd-lang/src/core_commands/ignore.rs @@ -1,5 +1,5 @@ use nu_engine::command_prelude::*; -use nu_protocol::{engine::StateWorkingSet, OutDest}; +use nu_protocol::{engine::StateWorkingSet, ByteStreamSource, OutDest}; #[derive(Clone)] pub struct Ignore; @@ -32,8 +32,13 @@ impl Command for Ignore { _engine_state: &EngineState, _stack: &mut Stack, _call: &Call, - input: PipelineData, + mut input: PipelineData, ) -> Result { + if let PipelineData::ByteStream(stream, _) = &mut input { + if let ByteStreamSource::Child(child) = stream.source_mut() { + child.ignore_error(true); + } + } input.drain()?; Ok(PipelineData::empty()) } diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs index f14ed25471..ec7e4140ce 100644 --- a/crates/nu-command/src/filesystem/save.rs +++ b/crates/nu-command/src/filesystem/save.rs @@ -182,6 +182,8 @@ impl Command for Save { } (None, None) => {} }; + + child.wait()?; } } diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index 4a790fc9b4..8d6b4e2bf3 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -180,12 +180,19 @@ impl Command for External { } // Wrap the output into a `PipelineData::ByteStream`. - let child = ChildProcess::new( + let mut child = ChildProcess::new( child, merged_stream, matches!(stderr, OutDest::Pipe), call.head, )?; + + if matches!(stdout, OutDest::Pipe | OutDest::PipeSeparate) + || matches!(stderr, OutDest::Pipe | OutDest::PipeSeparate) + { + child.ignore_error(true); + } + Ok(PipelineData::ByteStream( ByteStream::child(child, call.head), None, diff --git a/crates/nu-protocol/src/process/child.rs b/crates/nu-protocol/src/process/child.rs index 930015742b..8c4fb55c0d 100644 --- a/crates/nu-protocol/src/process/child.rs +++ b/crates/nu-protocol/src/process/child.rs @@ -202,8 +202,8 @@ impl ChildProcess { } } - pub fn ignore_error(&mut self) -> &mut Self { - self.ignore_error = true; + pub fn ignore_error(&mut self, ignore: bool) -> &mut Self { + self.ignore_error = ignore; self } diff --git a/tests/shell/pipeline/mod.rs b/tests/shell/pipeline/mod.rs index 243dc27475..5a58134b98 100644 --- a/tests/shell/pipeline/mod.rs +++ b/tests/shell/pipeline/mod.rs @@ -6,6 +6,14 @@ use pretty_assertions::assert_eq; #[test] fn doesnt_break_on_utf8() { let actual = nu!("echo ö"); - assert_eq!(actual.out, "ö", "'{}' should contain ö", actual.out); } + +#[test] +fn non_zero_exit_code_in_middle_of_pipeline_ignored() { + let actual = nu!("nu -c 'print a b; exit 42' | collect"); + assert_eq!(actual.out, "ab"); + + let actual = nu!("nu -c 'print a b; exit 42' | nu --stdin -c 'collect'"); + assert_eq!(actual.out, "ab"); +}