From eb69c211ce07e1f6ef900675115e8c58f18c740c Mon Sep 17 00:00:00 2001 From: Richard Schneeman Date: Mon, 6 Nov 2023 15:27:28 -0600 Subject: [PATCH] Fix a bug where stdout was copying stderr In the migration of the Ruby buildpack to use this crate I noticed a regression: ``` - Running `bash -c "ps aux | grep cargo"` rschneeman 26398 0.0 0.0 408417520 9600 s004 S+ 11:24AM 0:00.13 /Users/rschneeman/.cargo/bin/cargo-watch watch -c -x ltest rschneeman 17561 0.0 0.0 408494848 848 s000 R+ 2:39PM 0:00.00 grep cargo rschneeman 17559 0.0 0.0 408627920 3088 s000 S+ 2:39PM 0:00.00 bash -c ps aux | grep cargo rschneeman 15128 0.0 0.0 408433904 10176 s006 S+ 2:35PM 0:00.40 /Users/rschneeman/.cargo/bin/cargo-watch watch -c -x ltest - Done (< 0.1s) ``` I isolated it to the `fun_run` PR https://github.com/heroku/buildpacks-ruby/pull/232 and after some debugging found it was due to accidentally copying `child.stdout` for `child_stderr`. I added a test that verifies that stderr is redirected correctly. --- Cargo.toml | 3 +++ src/command.rs | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6b3e315..749b6dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,3 +15,6 @@ regex = "1" [features] which_problem = ["dep:which_problem"] + +[dev-dependencies] +pretty_assertions = "1" diff --git a/src/command.rs b/src/command.rs index 7946949..4e1dd70 100644 --- a/src/command.rs +++ b/src/command.rs @@ -23,7 +23,7 @@ pub(crate) fn output_and_write_streams( let stdout_thread = mem::take(&mut child.stdout).map(|mut child_stdout| { scope.spawn(move || std::io::copy(&mut child_stdout, &mut stdout)) }); - let stderr_thread = mem::take(&mut child.stdout).map(|mut child_stderr| { + let stderr_thread = mem::take(&mut child.stderr).map(|mut child_stderr| { scope.spawn(move || std::io::copy(&mut child_stderr, &mut stderr)) }); @@ -56,11 +56,12 @@ pub(crate) fn output_and_write_streams( #[cfg(test)] mod test { use super::*; + use pretty_assertions::assert_str_eq; use std::process::Command; #[test] #[cfg(unix)] - fn test_output_and_write_streams() { + fn test_output_and_write_streams_stdout() { let mut stdout_buf = Vec::new(); let mut stderr_buf = Vec::new(); @@ -76,6 +77,20 @@ mod test { assert_eq!(output.stdout, "Hello World!".as_bytes()); assert_eq!(output.stderr, Vec::::new()); } + + #[test] + #[cfg(unix)] + fn test_output_and_write_streams_stderr() { + let mut stdout_buf = Vec::new(); + let mut stderr_buf = Vec::new(); + + let mut cmd = Command::new("bash"); + cmd.args(["-c", "echo -n Hello World! >&2"]); + + let _ = output_and_write_streams(&mut cmd, &mut stdout_buf, &mut stderr_buf).unwrap(); + + assert_str_eq!(&String::from_utf8_lossy(&stderr_buf), "Hello World!"); + } } /// Constructs a writer that writes to two other writers. Similar to the UNIX `tee` command.