Skip to content

Commit

Permalink
Auto merge of #4230 - Mark-Simulacrum:double-print, r=alexcrichton
Browse files Browse the repository at this point in the history
Prevent rustc stderr/stdout from being duplicated.

Please review carefully. I've not submitted patches to Cargo before, I think, so this may be flawed in some way I haven't detected yet. Tests are green locally, though.

Fixes #4223
  • Loading branch information
bors committed Jun 27, 2017
2 parents a3253d6 + f377ae2 commit 854bc16
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_rustc/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
let output = cmd.exec_with_streaming(
&mut |out_line| { state.stdout(out_line); Ok(()) },
&mut |err_line| { state.stderr(err_line); Ok(()) },
true,
).map_err(|e| {
CargoError::from(
format!("failed to run custom build command for `{}`\n{}",
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub trait Executor: Send + Sync + 'static {
handle_stdout: &mut FnMut(&str) -> CargoResult<()>,
handle_stderr: &mut FnMut(&str) -> CargoResult<()>)
-> CargoResult<()> {
cmd.exec_with_streaming(handle_stdout, handle_stderr)?;
cmd.exec_with_streaming(handle_stdout, handle_stderr, false)?;
Ok(())
}
}
Expand Down
33 changes: 21 additions & 12 deletions src/cargo/util/process_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ impl ProcessBuilder {

pub fn exec_with_streaming(&self,
on_stdout_line: &mut FnMut(&str) -> CargoResult<()>,
on_stderr_line: &mut FnMut(&str) -> CargoResult<()>)
on_stderr_line: &mut FnMut(&str) -> CargoResult<()>,
print_output: bool)
-> CargoResult<Output> {
let mut stdout = Vec::new();
let mut stderr = Vec::new();
Expand Down Expand Up @@ -187,18 +188,26 @@ impl ProcessBuilder {
stderr: stderr,
status: status,
};
if !output.status.success() {
Err(CargoErrorKind::ProcessErrorKind(process_error(
&format!("process didn't exit successfully: `{}`", self.debug_string()),
Some(&output.status), Some(&output))).into())
} else if let Some(e) = callback_error {
Err(CargoError::with_chain(e,
CargoErrorKind::ProcessErrorKind(process_error(
&format!("failed to parse process output: `{}`", self.debug_string()),
Some(&output.status), Some(&output)))))
} else {
Ok(output)

{
let to_print = if print_output {
Some(&output)
} else {
None
};
if !output.status.success() {
return Err(CargoErrorKind::ProcessErrorKind(process_error(
&format!("process didn't exit successfully: `{}`", self.debug_string()),
Some(&output.status), to_print)).into())
} else if let Some(e) = callback_error {
return Err(CargoError::with_chain(e,
CargoErrorKind::ProcessErrorKind(process_error(
&format!("failed to parse process output: `{}`", self.debug_string()),
Some(&output.status), to_print))))
}
}

Ok(output)
}

pub fn build_command(&self) -> Command {
Expand Down
10 changes: 10 additions & 0 deletions tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ fn cargo_compile_simple() {
execs().with_status(0).with_stdout("i am foo\n"));
}

#[test]
fn cargo_fail_with_no_stderr() {
let p = project("foo")
.file("Cargo.toml", &basic_bin_manifest("foo"))
.file("src/foo.rs", &String::from("refusal"));
let p = p.build();
assert_that(p.cargo("build").arg("--message-format=json"), execs().with_status(101)
.with_stderr_does_not_contain("--- stderr"));
}

/// Check that the `CARGO_INCREMENTAL` environment variable results in
/// `rustc` getting `-Zincremental` passed to it.
#[test]
Expand Down

0 comments on commit 854bc16

Please sign in to comment.