Skip to content

Commit

Permalink
Fix a bug where stdout was copying stderr
Browse files Browse the repository at this point in the history
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 heroku/buildpacks-ruby#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.
  • Loading branch information
Richard Schneeman committed Nov 6, 2023
1 parent f30c098 commit eb69c21
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ regex = "1"

[features]
which_problem = ["dep:which_problem"]

[dev-dependencies]
pretty_assertions = "1"
19 changes: 17 additions & 2 deletions src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) fn output_and_write_streams<OW: Write + Send, EW: Write + Send>(
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))
});

Expand Down Expand Up @@ -56,11 +56,12 @@ pub(crate) fn output_and_write_streams<OW: Write + Send, EW: Write + Send>(
#[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();

Expand All @@ -76,6 +77,20 @@ mod test {
assert_eq!(output.stdout, "Hello World!".as_bytes());
assert_eq!(output.stderr, Vec::<u8>::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.
Expand Down

0 comments on commit eb69c21

Please sign in to comment.