Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fully capture rustc and rustdoc output when -Zcompile-progress is passed #5862

Merged
merged 3 commits into from
Aug 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ core-foundation = { version = "0.6.0", features = ["mac_os_10_7_support"] }

[target.'cfg(windows)'.dependencies]
miow = "0.3.1"
fwdansi = "1"

[target.'cfg(windows)'.dependencies.winapi]
version = "0.3"
Expand Down
18 changes: 7 additions & 11 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
let build_scripts = super::load_build_deps(cx, unit);
let kind = unit.kind;
let json_messages = bcx.build_config.json_messages();
let extra_verbose = bcx.config.extra_verbose();

// Check to see if the build script has already run, and if it has keep
// track of whether it has told us about some explicit dependencies
Expand Down Expand Up @@ -320,17 +321,12 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
state.build_plan(invocation_name, cmd.clone(), Arc::new(Vec::new()));
} else {
state.running(&cmd);
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| {
let output = if extra_verbose {
state.capture_output(cmd, true)
} else {
cmd.exec_with_output()
};
let output = output.map_err(|e| {
format_err!(
"failed to run custom build command for `{}`\n{}",
pkg_name,
Expand Down
55 changes: 27 additions & 28 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::io;
use std::mem;
use std::sync::mpsc::{channel, Receiver, Sender};
use std::sync::Arc;
use std::process::Output;

use crossbeam_utils;
use crossbeam_utils::thread::Scope;
Expand Down Expand Up @@ -107,12 +108,22 @@ impl<'a> JobState<'a> {
.send(Message::BuildPlanMsg(module_name, cmd, filenames));
}

pub fn stdout(&self, out: &str) {
let _ = self.tx.send(Message::Stdout(out.to_string()));
}

pub fn stderr(&self, err: &str) {
let _ = self.tx.send(Message::Stderr(err.to_string()));
pub fn capture_output(
&self,
cmd: ProcessBuilder,
print_output: bool,
) -> CargoResult<Output> {
cmd.exec_with_streaming(
&mut |out| {
let _ = self.tx.send(Message::Stdout(out.to_string()));
Ok(())
},
&mut |err| {
let _ = self.tx.send(Message::Stderr(err.to_string()));
Ok(())
},
print_output,
)
}
}

Expand Down Expand Up @@ -226,7 +237,6 @@ impl<'a> JobQueue<'a> {
// currently a pretty big task. This is issue #5695.
let mut error = None;
let mut progress = Progress::with_style("Building", ProgressStyle::Ratio, cx.bcx.config);
let mut progress_maybe_changed = true; // avoid flickering due to build script
if !cx.bcx.config.cli_unstable().compile_progress {
progress.disable();
}
Expand Down Expand Up @@ -274,22 +284,13 @@ impl<'a> JobQueue<'a> {
// to the jobserver itself.
tokens.truncate(self.active.len() - 1);

if progress_maybe_changed {
let count = total - self.queue.len();
let active_names = self.active.iter()
.map(Key::name_for_progress)
.collect::<Vec<_>>();
drop(progress.tick_now(count, total, &format!(": {}", active_names.join(", "))));
}
let count = total - self.queue.len();
let active_names = self.active.iter()
.map(Key::name_for_progress)
.collect::<Vec<_>>();
drop(progress.tick_now(count, total, &format!(": {}", active_names.join(", "))));
let event = self.rx.recv().unwrap();

progress_maybe_changed = match event {
Message::Stdout(_) | Message::Stderr(_) => cx.bcx.config.extra_verbose(),
_ => true,
};
if progress_maybe_changed {
progress.clear();
}
progress.clear();

match event {
Message::Run(cmd) => {
Expand All @@ -302,14 +303,12 @@ impl<'a> JobQueue<'a> {
plan.update(&module_name, &cmd, &filenames)?;
}
Message::Stdout(out) => {
if cx.bcx.config.extra_verbose() {
println!("{}", out);
}
println!("{}", out);
}
Message::Stderr(err) => {
if cx.bcx.config.extra_verbose() {
writeln!(cx.bcx.config.shell().err(), "{}", err)?;
}
let mut shell = cx.bcx.config.shell();
shell.print_ansi(err.as_bytes())?;
shell.err().write(b"\n")?;
}
Message::FixDiagnostic(msg) => {
print.print(&msg)?;
Expand Down
69 changes: 53 additions & 16 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ pub trait Executor: Send + Sync + 'static {
Ok(())
}

fn exec_and_capture_output(
&self,
cmd: ProcessBuilder,
id: &PackageId,
target: &Target,
mode: CompileMode,
_state: &job_queue::JobState<'_>,
) -> CargoResult<()> {
// we forward to exec() to keep RLS working.
self.exec(cmd, id, target, mode)
}

fn exec_json(
&self,
cmd: ProcessBuilder,
Expand All @@ -97,7 +109,18 @@ pub trait Executor: Send + Sync + 'static {
#[derive(Copy, Clone)]
pub struct DefaultExecutor;

impl Executor for DefaultExecutor {}
impl Executor for DefaultExecutor {
fn exec_and_capture_output(
&self,
cmd: ProcessBuilder,
_id: &PackageId,
_target: &Target,
_mode: CompileMode,
state: &job_queue::JobState<'_>,
) -> CargoResult<()> {
state.capture_output(cmd, false).map(drop)
}
}

fn compile<'a, 'cfg: 'a>(
cx: &mut Context<'a, 'cfg>,
Expand Down Expand Up @@ -216,6 +239,8 @@ fn rustc<'a, 'cfg>(
.unwrap_or_else(|| cx.bcx.config.cwd())
.to_path_buf();

let should_capture_output = cx.bcx.config.cli_unstable().compile_progress;

return Ok(Work::new(move |state| {
// Only at runtime have we discovered what the extra -L and -l
// arguments are for native libraries, so we process those here. We
Expand Down Expand Up @@ -291,7 +316,12 @@ fn rustc<'a, 'cfg>(
} else if build_plan {
state.build_plan(buildkey, rustc.clone(), outputs.clone());
} else {
exec.exec(rustc, &package_id, &target, mode)
let exec_result = if should_capture_output {
exec.exec_and_capture_output(rustc, &package_id, &target, mode, state)
} else {
exec.exec(rustc, &package_id, &target, mode)
};
exec_result
.map_err(Internal::new)
.chain_err(|| format!("Could not compile `{}`.", name))?;
}
Expand Down Expand Up @@ -580,6 +610,7 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
rustdoc.arg("--crate-name").arg(&unit.target.crate_name());
add_path_args(bcx, unit, &mut rustdoc);
add_cap_lints(bcx, unit, &mut rustdoc);
add_color(bcx, &mut rustdoc);

if unit.kind != Kind::Host {
if let Some(ref target) = bcx.build_config.requested_target {
Expand Down Expand Up @@ -612,6 +643,8 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
let build_state = cx.build_state.clone();
let key = (unit.pkg.package_id().clone(), unit.kind);

let should_capture_output = cx.bcx.config.cli_unstable().compile_progress;

Ok(Work::new(move |state| {
if let Some(output) = build_state.outputs.lock().unwrap().get(&key) {
for cfg in output.cfgs.iter() {
Expand All @@ -622,9 +655,13 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
}
}
state.running(&rustdoc);
rustdoc
.exec()
.chain_err(|| format!("Could not document `{}`.", name))?;

let exec_result = if should_capture_output {
state.capture_output(rustdoc, false).map(drop)
} else {
rustdoc.exec()
};
exec_result.chain_err(|| format!("Could not document `{}`.", name))?;
Ok(())
}))
}
Expand Down Expand Up @@ -672,6 +709,15 @@ fn add_cap_lints(bcx: &BuildContext, unit: &Unit, cmd: &mut ProcessBuilder) {
}
}

fn add_color(bcx: &BuildContext, cmd: &mut ProcessBuilder) {
let capture_output = bcx.config.cli_unstable().compile_progress;
let shell = bcx.config.shell();
if capture_output || shell.color_choice() != ColorChoice::CargoAuto {
let color = if shell.supports_color() { "always" } else { "never" };
cmd.args(&["--color", color]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly different than the previous invocation that was moved up from below, but I think it's equivalent, right?

Previously if I pass --color never to Cargo it'll forward that to rustc unconditionally. I think that'll happen here because stream.supports_color() is already set to return false unconditionally inside of termcolor, I think?

(and I believe the reverse is true for --color always)

Just wanted to make sure!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously if I pass --color never to Cargo it'll forward that to rustc unconditionally. I think that'll happen here because stream.supports_color() is already set to return false unconditionally inside of termcolor, I think?

Yes. supports_color() will return false if the stream is "NoColor", and true otherwise.

When a stream is created, it will create the "NoColor" variant if ColorChoice::should_attempt_color() returns false, which is when --color never or --color auto with $TERM ≠ "dumb".

Thus --color never will imply supports_color() returns false unconditionally.

}
}

fn build_base_args<'a, 'cfg>(
cx: &mut Context<'a, 'cfg>,
cmd: &mut ProcessBuilder,
Expand All @@ -696,17 +742,8 @@ fn build_base_args<'a, 'cfg>(

cmd.arg("--crate-name").arg(&unit.target.crate_name());

add_path_args(&cx.bcx, unit, cmd);

match bcx.config.shell().color_choice() {
ColorChoice::Always => {
cmd.arg("--color").arg("always");
}
ColorChoice::Never => {
cmd.arg("--color").arg("never");
}
ColorChoice::CargoAuto => {}
}
add_path_args(bcx, unit, cmd);
add_color(bcx, cmd);

if bcx.build_config.json_messages() {
cmd.arg("--error-format").arg("json");
Expand Down
21 changes: 21 additions & 0 deletions src/cargo/core/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,27 @@ impl Shell {
ShellOut::Write(_) => ColorChoice::Never,
}
}

/// Whether the shell supports color.
pub fn supports_color(&self) -> bool {
match &self.err {
ShellOut::Write(_) => false,
ShellOut::Stream { stream, .. } => stream.supports_color(),
}
}

/// Prints a message and translates ANSI escape code into console colors.
pub fn print_ansi(&mut self, message: &[u8]) -> CargoResult<()> {
#[cfg(windows)]
{
if let ShellOut::Stream { stream, .. } = &mut self.err {
::fwdansi::write_ansi(stream, message)?;
return Ok(());
}
}
self.err().write_all(message)?;
Ok(())
}
}

impl Default for Shell {
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ extern crate failure;
extern crate filetime;
extern crate flate2;
extern crate fs2;
#[cfg(windows)]
extern crate fwdansi;
extern crate git2;
extern crate glob;
extern crate hex;
Expand Down