From cd0d0364ecc759bf7401f8d762611ceeb0e8c909 Mon Sep 17 00:00:00 2001
From: Ian Manske <ian.manske@pm.me>
Date: Sun, 22 Sep 2024 07:26:32 -0700
Subject: [PATCH] Fix `do -p` not waiting for external commands (#13881)

# Description
Similar to #13870 (thanks @WindSoilder), this PR adds a boolean which
determines whether to ignore any errors from an external command. This
is in order to fix #13876. I.e., `do -p` does not wait for externals to
complete before continuing.

# User-Facing Changes
Bug fix.

# Tests + Formatting
Added a test.
---
 crates/nu-cmd-lang/src/core_commands/do_.rs   |  2 +-
 crates/nu-command/tests/commands/do_.rs       |  7 +++++++
 crates/nu-protocol/src/process/child.rs       | 17 ++++++++++++-----
 crates/nu-protocol/src/process/exit_status.rs |  8 +++++---
 4 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/crates/nu-cmd-lang/src/core_commands/do_.rs b/crates/nu-cmd-lang/src/core_commands/do_.rs
index b05de81d57..742bc3d218 100644
--- a/crates/nu-cmd-lang/src/core_commands/do_.rs
+++ b/crates/nu-cmd-lang/src/core_commands/do_.rs
@@ -169,7 +169,7 @@ impl Command for Do {
                     && !matches!(caller_stack.stdout(), OutDest::Pipe | OutDest::Capture) =>
             {
                 if let ByteStreamSource::Child(child) = stream.source_mut() {
-                    child.set_exit_code(0)
+                    child.ignore_error();
                 }
                 Ok(PipelineData::ByteStream(stream, metadata))
             }
diff --git a/crates/nu-command/tests/commands/do_.rs b/crates/nu-command/tests/commands/do_.rs
index 645421c15f..fe7959acb4 100644
--- a/crates/nu-command/tests/commands/do_.rs
+++ b/crates/nu-command/tests/commands/do_.rs
@@ -73,3 +73,10 @@ fn run_closure_with_it_using() {
     assert!(actual.err.is_empty());
     assert_eq!(actual.out, "3");
 }
+
+#[test]
+fn waits_for_external() {
+    let actual = nu!(r#"do -p { nu -c 'sleep 1sec; print before; exit 1'}; print after"#);
+    assert!(actual.err.is_empty());
+    assert_eq!(actual.out, "beforeafter");
+}
diff --git a/crates/nu-protocol/src/process/child.rs b/crates/nu-protocol/src/process/child.rs
index 320427a819..9ab92b331c 100644
--- a/crates/nu-protocol/src/process/child.rs
+++ b/crates/nu-protocol/src/process/child.rs
@@ -29,7 +29,7 @@ impl ExitStatusFuture {
                             core_dumped: true, ..
                         },
                     )) => {
-                        status.check_ok(span)?;
+                        status.check_ok(false, span)?;
                         Ok(status)
                     }
                     Ok(Ok(status)) => Ok(status),
@@ -109,6 +109,7 @@ pub struct ChildProcess {
     pub stdout: Option<ChildPipe>,
     pub stderr: Option<ChildPipe>,
     exit_status: ExitStatusFuture,
+    ignore_error: bool,
     span: Span,
 }
 
@@ -155,12 +156,14 @@ impl ChildProcess {
             exit_status: exit_status
                 .map(ExitStatusFuture::Running)
                 .unwrap_or(ExitStatusFuture::Finished(Ok(ExitStatus::Exited(0)))),
+            ignore_error: false,
             span,
         }
     }
 
-    pub fn set_exit_code(&mut self, exit_code: i32) {
-        self.exit_status = ExitStatusFuture::Finished(Ok(ExitStatus::Exited(exit_code)));
+    pub fn ignore_error(&mut self) -> &mut Self {
+        self.ignore_error = true;
+        self
     }
 
     pub fn span(&self) -> Span {
@@ -182,7 +185,9 @@ impl ChildProcess {
             Vec::new()
         };
 
-        self.exit_status.wait(self.span)?.check_ok(self.span)?;
+        self.exit_status
+            .wait(self.span)?
+            .check_ok(self.ignore_error, self.span)?;
 
         Ok(bytes)
     }
@@ -223,7 +228,9 @@ impl ChildProcess {
             consume_pipe(stderr).err_span(self.span)?;
         }
 
-        self.exit_status.wait(self.span)?.check_ok(self.span)
+        self.exit_status
+            .wait(self.span)?
+            .check_ok(self.ignore_error, self.span)
     }
 
     pub fn try_wait(&mut self) -> Result<Option<ExitStatus>, ShellError> {
diff --git a/crates/nu-protocol/src/process/exit_status.rs b/crates/nu-protocol/src/process/exit_status.rs
index 0ea6b0e72b..8f37902aae 100644
--- a/crates/nu-protocol/src/process/exit_status.rs
+++ b/crates/nu-protocol/src/process/exit_status.rs
@@ -20,10 +20,12 @@ impl ExitStatus {
         }
     }
 
-    pub fn check_ok(self, span: Span) -> Result<(), ShellError> {
+    pub fn check_ok(self, ignore_error: bool, span: Span) -> Result<(), ShellError> {
         match self {
             ExitStatus::Exited(exit_code) => {
-                if let Ok(exit_code) = exit_code.try_into() {
+                if ignore_error {
+                    Ok(())
+                } else if let Ok(exit_code) = exit_code.try_into() {
                     Err(ShellError::NonZeroExitCode { exit_code, span })
                 } else {
                     Ok(())
@@ -38,7 +40,7 @@ impl ExitStatus {
 
                 let sig = Signal::try_from(signal);
 
-                if sig == Ok(Signal::SIGPIPE) {
+                if sig == Ok(Signal::SIGPIPE) || (ignore_error && !core_dumped) {
                     // Processes often exit with SIGPIPE, but this is not an error condition.
                     Ok(())
                 } else {