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.
This commit is contained in:
Ian Manske 2024-09-22 07:26:32 -07:00 committed by GitHub
parent cf5fec63c0
commit cd0d0364ec
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 25 additions and 9 deletions

View file

@ -169,7 +169,7 @@ impl Command for Do {
&& !matches!(caller_stack.stdout(), OutDest::Pipe | OutDest::Capture) => && !matches!(caller_stack.stdout(), OutDest::Pipe | OutDest::Capture) =>
{ {
if let ByteStreamSource::Child(child) = stream.source_mut() { if let ByteStreamSource::Child(child) = stream.source_mut() {
child.set_exit_code(0) child.ignore_error();
} }
Ok(PipelineData::ByteStream(stream, metadata)) Ok(PipelineData::ByteStream(stream, metadata))
} }

View file

@ -73,3 +73,10 @@ fn run_closure_with_it_using() {
assert!(actual.err.is_empty()); assert!(actual.err.is_empty());
assert_eq!(actual.out, "3"); 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");
}

View file

@ -29,7 +29,7 @@ impl ExitStatusFuture {
core_dumped: true, .. core_dumped: true, ..
}, },
)) => { )) => {
status.check_ok(span)?; status.check_ok(false, span)?;
Ok(status) Ok(status)
} }
Ok(Ok(status)) => Ok(status), Ok(Ok(status)) => Ok(status),
@ -109,6 +109,7 @@ pub struct ChildProcess {
pub stdout: Option<ChildPipe>, pub stdout: Option<ChildPipe>,
pub stderr: Option<ChildPipe>, pub stderr: Option<ChildPipe>,
exit_status: ExitStatusFuture, exit_status: ExitStatusFuture,
ignore_error: bool,
span: Span, span: Span,
} }
@ -155,12 +156,14 @@ impl ChildProcess {
exit_status: exit_status exit_status: exit_status
.map(ExitStatusFuture::Running) .map(ExitStatusFuture::Running)
.unwrap_or(ExitStatusFuture::Finished(Ok(ExitStatus::Exited(0)))), .unwrap_or(ExitStatusFuture::Finished(Ok(ExitStatus::Exited(0)))),
ignore_error: false,
span, span,
} }
} }
pub fn set_exit_code(&mut self, exit_code: i32) { pub fn ignore_error(&mut self) -> &mut Self {
self.exit_status = ExitStatusFuture::Finished(Ok(ExitStatus::Exited(exit_code))); self.ignore_error = true;
self
} }
pub fn span(&self) -> Span { pub fn span(&self) -> Span {
@ -182,7 +185,9 @@ impl ChildProcess {
Vec::new() 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) Ok(bytes)
} }
@ -223,7 +228,9 @@ impl ChildProcess {
consume_pipe(stderr).err_span(self.span)?; 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> { pub fn try_wait(&mut self) -> Result<Option<ExitStatus>, ShellError> {

View file

@ -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 { match self {
ExitStatus::Exited(exit_code) => { 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 }) Err(ShellError::NonZeroExitCode { exit_code, span })
} else { } else {
Ok(()) Ok(())
@ -38,7 +40,7 @@ impl ExitStatus {
let sig = Signal::try_from(signal); 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. // Processes often exit with SIGPIPE, but this is not an error condition.
Ok(()) Ok(())
} else { } else {