holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

When using redirection, if a command generates non-zero exit code, the script should stop running (#11191)

Browse Source 
# Description
Fixes: #11153

To make sure scripts stop from running on non-zero exit code, we need to
invoke `might_consume_external_result` on
`PipelineData::ExternalStream`, so it can tell nushell if this command
exists with non-zero exit code.

And this pr also adjusts some test cases.

# User-Facing Changes
```nushell
^false out> /dev/null; print "ok"
```

After this pr, it shouldn't print ok.

# Tests + Formatting
Done
This commit is contained in:
WindSoilder 2023-12-01 01:52:02 +08:00 committed by GitHub
parent 250ee62e16
commit 80881c75f9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 47 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
crates/nu-command/tests/commands/redirection.rs
View File

@ -161,29 +161,32 @@ fn same_target_redirection_with_too_much_stderr_not_hang_nushell() {
#[test] #[test]
fn redirection_keep_exit_codes() { fn redirection_keep_exit_codes() {
Playground::setup( let out = nu!("do -i { nu --testbin fail e> a.txt } | complete | get exit_code");
"redirection should keep exit code the same", // needs to use contains "1", because it complete will output `Some(RawStream)`.
|dirs, sandbox| { assert!(out.out.contains('1'));
let script_body = r#"exit 10"#; }
#[cfg(not(windows))]
let output = { #[test]
sandbox.with_files(vec![FileWithContent("test.sh", script_body)]); fn redirection_with_non_zero_exit_code_should_stop_from_running() {
nu!( Playground::setup("redirection with non zero exit code", |dirs, _| {
cwd: dirs.test(), for redirection in ["o>", "o>>", "e>", "e>>", "o+e>", "o+e>>"] {
"bash test.sh out> out.txt err> err.txt; echo $env.LAST_EXIT_CODE" let output = nu!(
) cwd: dirs.test(),
}; &format!("nu --testbin fail {redirection} log.txt; echo 3")
#[cfg(windows)] );
let output = { assert!(!output.out.contains('3'));
sandbox.with_files(vec![FileWithContent("test.bat", script_body)]); }
nu!( });
cwd: dirs.test(),
"cmd /D /c test.bat out> out.txt err> err.txt; echo $env.LAST_EXIT_CODE" Playground::setup("redirection with non zero exit code", |dirs, _| {
) for (out, err) in [("o>", "e>"), ("o>>", "e>"), ("o>", "e>>"), ("o>>", "e>>")] {
}; let output = nu!(
assert_eq!(output.out, "10") cwd: dirs.test(),
}, &format!("nu --testbin fail {out} log.txt {err} err_log.txt; echo 3")
) );
assert!(!output.out.contains('3'));
}
})
} }
#[test] #[test]

49
crates/nu-engine/src/eval.rs
View File

@ -828,17 +828,17 @@ fn eval_element_with_input(
// save is internal command, normally it exists with non-ExternalStream // save is internal command, normally it exists with non-ExternalStream
// but here in redirection context, we make it returns ExternalStream // but here in redirection context, we make it returns ExternalStream
// So nu handles exit_code correctly // So nu handles exit_code correctly
( //
PipelineData::ExternalStream { // Also, we don't want to run remaining commands if this command exits with non-zero
stdout: None, // exit code, so we need to consume and check exit_code too
stderr: None, might_consume_external_result(PipelineData::ExternalStream {
exit_code, stdout: None,
span: *span, stderr: None,
metadata: None, exit_code,
trim_end_newline: false, span: *span,
}, metadata: None,
false, trim_end_newline: false,
) })
}) })
} }
Some(out_stream) => { Some(out_stream) => {
@ -854,7 +854,7 @@ fn eval_element_with_input(
input, input,
)); ));
Ok(( Ok(might_consume_external_result(
PipelineData::ExternalStream { PipelineData::ExternalStream {
stdout: out_stream, stdout: out_stream,
stderr: None, stderr: None,
@ -863,7 +863,6 @@ fn eval_element_with_input(
metadata: None, metadata: None,
trim_end_newline: false, trim_end_newline: false,
}, },
false,
)) ))
} }
} }
@ -903,17 +902,14 @@ fn eval_element_with_input(
// save is internal command, normally it exists with non-ExternalStream // save is internal command, normally it exists with non-ExternalStream
// but here in redirection context, we make it returns ExternalStream // but here in redirection context, we make it returns ExternalStream
// So nu handles exit_code correctly // So nu handles exit_code correctly
( might_consume_external_result(PipelineData::ExternalStream {
PipelineData::ExternalStream { stdout: None,
stdout: None, stderr: None,
stderr: None, exit_code,
exit_code, span: *out_span,
span: *out_span, metadata: None,
metadata: None, trim_end_newline: false,
trim_end_newline: false, })
},
false,
)
}) })
} else { } else {
Err(ShellError::CommandNotFound { span: *out_span }) Err(ShellError::CommandNotFound { span: *out_span })
@ -1098,9 +1094,6 @@ pub fn eval_block(
); );
match (eval_result, redirect_stderr) { match (eval_result, redirect_stderr) {
(Ok((pipeline_data, _)), true) => {
input = pipeline_data;
}
(Err(error), true) => { (Err(error), true) => {
input = PipelineData::Value( input = PipelineData::Value(
Value::error( Value::error(
@ -1110,7 +1103,7 @@ pub fn eval_block(
None, None,
) )
} }
(output, false) => { (output, _) => {
let output = output?; let output = output?;
input = output.0; input = output.0;
// external command may runs to failed // external command may runs to failed