extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2024-11-22 16:33:37 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix pipeline stall in do during capture and remove excessive redirections (#7204)

Browse Source 
Currently, if you run `do -i { sudo apt upgrade }`, stdin gets swallowed
and doesn't let you respond yes/no to the upgrade question. This PR
fixes that, but runs into https://github.com/nushell/nushell/issues/7205
so the tests fail.

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
This commit is contained in:
Alex Saveau 2023-01-24 21:24:38 -08:00 committed by GitHub
parent d64e381085
commit 5cbaabeeab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 72 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
crates/nu-command/src/core_commands/do_.rs
View File

@ -1,8 +1,11 @@
use std::thread;
use nu_engine::{eval_block_with_early_return, CallExt};
use nu_protocol::ast::Call;
use nu_protocol::engine::{Closure, Command, EngineState, Stack};
use nu_protocol::{
Category, Example, ListStream, PipelineData, ShellError, Signature, SyntaxShape, Type, Value,
Category, Example, ListStream, PipelineData, RawStream, ShellError, Signature, SyntaxShape,
Type, Value,
};
#[derive(Clone)]
@ -106,7 +109,7 @@ impl Command for Do {
block,
input,
call.redirect_stdout,
capture_errors || ignore_shell_errors || ignore_program_errors,
call.redirect_stdout,
);
match result {
@ -118,6 +121,52 @@ impl Command for Do {
metadata,
trim_end_newline,
}) if capture_errors => {
// Use a thread to receive stdout message.
// Or we may get a deadlock if child process sends out too much bytes to stderr.
//
// For example: in normal linux system, stderr pipe's limit is 65535 bytes.
// if child process sends out 65536 bytes, the process will be hanged because no consumer
// consumes the first 65535 bytes
// So we need a thread to receive stdout message, then the current thread can continue to consume
// stderr messages.
let stdout_handler = stdout.map(|stdout_stream| {
thread::spawn(move || {
let ctrlc = stdout_stream.ctrlc.clone();
let span = stdout_stream.span;
RawStream::new(
Box::new(vec![stdout_stream.into_bytes().map(|s| s.item)].into_iter()),
ctrlc,
span,
)
})
});
// Intercept stderr so we can return it in the error if the exit code is non-zero.
// The threading issues mentioned above dictate why we also need to intercept stdout.
let mut stderr_ctrlc = None;
let stderr_msg = match stderr {
None => "".to_string(),
Some(stderr_stream) => {
stderr_ctrlc = stderr_stream.ctrlc.clone();
stderr_stream.into_string().map(|s| s.item)?
}
};
let stdout = if let Some(handle) = stdout_handler {
match handle.join() {
Err(err) => {
return Err(ShellError::ExternalCommand(
"Fail to receive external commands stdout message".to_string(),
format!("{err:?}"),
span,
));
}
Ok(res) => Some(res),
}
} else {
None
};
let mut exit_code_ctrlc = None;
let exit_code: Vec<Value> = match exit_code {
None => vec![],
@ -128,11 +177,6 @@ impl Command for Do {
};
if let Some(Value::Int { val: code, .. }) = exit_code.last() {
if *code != 0 {
let stderr_msg = match stderr {
None => "".to_string(),
Some(stderr_stream) => stderr_stream.into_string().map(|s| s.item)?,
};
return Err(ShellError::ExternalCommand(
"External command failed".to_string(),
stderr_msg,
@ -143,7 +187,11 @@ impl Command for Do {
Ok(PipelineData::ExternalStream {
stdout,
stderr,
stderr: Some(RawStream::new(
Box::new(vec![Ok(stderr_msg.into_bytes())].into_iter()),
stderr_ctrlc,
span,
)),
exit_code: Some(ListStream::from_stream(
exit_code.into_iter(),
exit_code_ctrlc,
@ -168,10 +216,9 @@ impl Command for Do {
metadata,
trim_end_newline,
}),
Ok(PipelineData::Value(Value::Error { .. }, ..)) if ignore_shell_errors => {
Ok(PipelineData::Value(..)) | Err(_) if ignore_shell_errors => {
Ok(PipelineData::empty())
}
Err(_) if ignore_shell_errors => Ok(PipelineData::empty()),
r => r,
}
}

22
crates/nu-command/tests/commands/do_.rs
View File

@ -100,7 +100,7 @@ fn ignore_error_should_work_for_external_command() {
#[test]
#[cfg(not(windows))]
fn ignore_error_with_too_much_stderr_not_hang_nushell() {
fn capture_error_with_too_much_stderr_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::pipeline;
use nu_test_support::playground::Playground;
@ -115,8 +115,8 @@ fn ignore_error_with_too_much_stderr_not_hang_nushell() {
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
do -i {sh -c "cat a_large_file.txt 1>&2"} | complete | get stderr
"#
do -c {sh -c "cat a_large_file.txt 1>&2"} | complete | get stderr
"#,
));
assert_eq!(actual.out, large_file_body);
@ -125,7 +125,7 @@ fn ignore_error_with_too_much_stderr_not_hang_nushell() {
#[test]
#[cfg(not(windows))]
fn ignore_error_with_too_much_stdout_not_hang_nushell() {
fn capture_error_with_too_much_stdout_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::pipeline;
use nu_test_support::playground::Playground;
@ -140,8 +140,8 @@ fn ignore_error_with_too_much_stdout_not_hang_nushell() {
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
do -i {sh -c "cat a_large_file.txt"} | complete | get stdout
"#
do -c {sh -c "cat a_large_file.txt"} | complete | get stdout
"#,
));
assert_eq!(actual.out, large_file_body);
@ -150,7 +150,7 @@ fn ignore_error_with_too_much_stdout_not_hang_nushell() {
#[test]
#[cfg(not(windows))]
fn ignore_error_with_both_stdout_stderr_messages_not_hang_nushell() {
fn capture_error_with_both_stdout_stderr_messages_not_hang_nushell() {
use nu_test_support::fs::Stub::FileWithContent;
use nu_test_support::playground::Playground;
Playground::setup(
@ -172,16 +172,16 @@ fn ignore_error_with_both_stdout_stderr_messages_not_hang_nushell() {
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
do -i {bash test.sh} | complete | get stdout | str trim
"#
do -c {bash test.sh} | complete | get stdout | str trim
"#,
));
assert_eq!(actual.out, expect_body);
// check for stderr
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
do -i {bash test.sh} | complete | get stderr | str trim
"#
do -c {bash test.sh} | complete | get stderr | str trim
"#,
));
assert_eq!(actual.out, expect_body);
},

6
crates/nu-command/tests/commands/save.rs
View File

@ -95,7 +95,8 @@ fn save_stderr_and_stdout_to_same_file() {
r#"
let-env FOO = "bar";
let-env BAZ = "ZZZ";
do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_5/new-file.txt --stderr save_test_5/new-file.txt"#,
do -c {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_5/new-file.txt --stderr save_test_5/new-file.txt
"#,
);
let actual = file_contents(expected_file);
@ -118,7 +119,8 @@ fn save_stderr_and_stdout_to_diff_file() {
r#"
let-env FOO = "bar";
let-env BAZ = "ZZZ";
do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_6/log.txt --stderr save_test_6/err.txt"#,
do -c {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_6/log.txt --stderr save_test_6/err.txt
"#,
);
let actual = file_contents(expected_file);