Print stderr streams to stderr in pipeline_data::print_if_stream() (#11929)

<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

Related to #11928 - `tee --stderr` doesn't really work as expected
without it

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

Print stderr streams to stderr in `pipeline_data::print_if_stream()`

This corrects unexpected behavior if a stream from an external program
is transformed while still preserving its stderr output. Before this
change, that output is just drained and discarded. Worse, it's drained
to a buffer, which could be really slow and memory hungry if there's a
lot of output on stderr.

This is needed to make `tee --stderr` function in a non-surprising way.
See #11928

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

A script that was erroneously not producing stderr output before might
now, but I can't think of a lot of examples of an external stream being
transformed without being converted.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`


# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
This commit is contained in:
Devyn Cairns 2024-02-24 07:32:39 -08:00 committed by GitHub
parent 995989dad4
commit 098527b263
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -4,9 +4,9 @@ use crate::{
format_error, Config, ListStream, RawStream, ShellError, Span, Value, format_error, Config, ListStream, RawStream, ShellError, Span, Value,
}; };
use nu_utils::{stderr_write_all_and_flush, stdout_write_all_and_flush}; use nu_utils::{stderr_write_all_and_flush, stdout_write_all_and_flush};
use std::path::PathBuf;
use std::sync::{atomic::AtomicBool, Arc}; use std::sync::{atomic::AtomicBool, Arc};
use std::thread; use std::thread;
use std::{io::Write, path::PathBuf};
const LINE_ENDING_PATTERN: &[char] = &['\r', '\n']; const LINE_ENDING_PATTERN: &[char] = &['\r', '\n'];
@ -846,13 +846,29 @@ pub fn print_if_stream(
to_stderr: bool, to_stderr: bool,
exit_code: Option<ListStream>, exit_code: Option<ListStream>,
) -> Result<i64, ShellError> { ) -> Result<i64, ShellError> {
// NOTE: currently we don't need anything from stderr
// so we just consume and throw away `stderr_stream` to make sure the pipe doesn't fill up
if let Some(stderr_stream) = stderr_stream { if let Some(stderr_stream) = stderr_stream {
// Write stderr to our stderr, if it's present
thread::Builder::new() thread::Builder::new()
.name("stderr consumer".to_string()) .name("stderr consumer".to_string())
.spawn(move || stderr_stream.into_bytes()) .spawn(move || {
let RawStream {
stream,
leftover,
ctrlc,
..
} = stderr_stream;
let mut stderr = std::io::stderr();
let _ = stderr.write_all(&leftover);
drop(leftover);
for bytes in stream {
if nu_utils::ctrl_c::was_pressed(&ctrlc) {
break;
}
if let Ok(bytes) = bytes {
let _ = stderr.write_all(&bytes);
}
}
})
.expect("could not create thread"); .expect("could not create thread");
} }