Auto merge of #5862 - kennytm:capture-rustc-output, r=alexcrichton

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

Fixes #5764 and #5695.

On Windows, we will parse the ANSI escape code into console commands via my `fwdansi` package, based on @ishitatsuyuki's idea in https://github.com/rust-lang/cargo/issues/5695#issuecomment-406300234. Outside of Windows the content is forwarded as-is.

Cargo.toml

@@ -67,6 +67,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"

src/cargo/core/compiler/custom_build.rs

@@ -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
@@ -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,

src/cargo/core/compiler/job_queue.rs

@@ -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;
@@ -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,
+        )
     }
 }
 
@@ -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();
         }
@@ -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) => {
@@ -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)?;

src/cargo/core/compiler/mod.rs

@@ -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,
@@ -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>,
@@ -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
@@ -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))?;
         }
@@ -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 {
@@ -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() {
@@ -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(())
     }))
 }
@@ -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]);
+    }
+}
+
 fn build_base_args<'a, 'cfg>(
     cx: &mut Context<'a, 'cfg>,
     cmd: &mut ProcessBuilder,
@@ -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");

src/cargo/core/shell.rs

@@ -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 {

src/cargo/lib.rs

@@ -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;