From f816d3a75479ba073570a4e998735be28770a307 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 2 Mar 2023 12:23:14 +0100 Subject: [PATCH] add a splash of color --- src/bootstrap/Cargo.lock | 22 ++++++++++++ src/bootstrap/Cargo.toml | 2 ++ src/bootstrap/config.rs | 6 ++++ src/bootstrap/lib.rs | 18 ++++++++++ src/bootstrap/render_tests.rs | 63 +++++++++++++++++++---------------- 5 files changed, 82 insertions(+), 29 deletions(-) diff --git a/src/bootstrap/Cargo.lock b/src/bootstrap/Cargo.lock index e861d520c534..8195823efaff 100644 --- a/src/bootstrap/Cargo.lock +++ b/src/bootstrap/Cargo.lock @@ -11,6 +11,17 @@ dependencies = [ "memchr", ] +[[package]] +name = "atty" +version = "0.2.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" +dependencies = [ + "hermit-abi", + "libc", + "winapi", +] + [[package]] name = "autocfg" version = "1.1.0" @@ -36,6 +47,7 @@ dependencies = [ name = "bootstrap" version = "0.0.0" dependencies = [ + "atty", "build_helper", "cc", "cmake", @@ -59,6 +71,7 @@ dependencies = [ "walkdir", "winapi", "xz2", + "yansi-term", ] [[package]] @@ -800,3 +813,12 @@ name = "yansi" version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" + +[[package]] +name = "yansi-term" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe5c30ade05e61656247b2e334a031dfd0cc466fadef865bdcdea8d537951bf1" +dependencies = [ + "winapi", +] diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index 663987f113c8..e704799867b3 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -30,6 +30,7 @@ path = "bin/sccache-plus-cl.rs" test = false [dependencies] +atty = "0.2.14" build_helper = { path = "../tools/build_helper" } cmake = "0.1.38" fd-lock = "3.0.8" @@ -52,6 +53,7 @@ opener = "0.5" once_cell = "1.7.2" xz2 = "0.1" walkdir = "2" +yansi-term = "0.1.2" # Dependencies needed by the build-metrics feature sysinfo = { version = "0.26.0", optional = true } diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 05e742549499..bfa237aa14d8 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -87,6 +87,9 @@ pub struct Config { pub patch_binaries_for_nix: bool, pub stage0_metadata: Stage0Metadata, + pub stdout_is_tty: bool, + pub stderr_is_tty: bool, + pub on_fail: Option, pub stage: u32, pub keep_stage: Vec, @@ -822,6 +825,9 @@ impl Config { config.deny_warnings = true; config.bindir = "bin".into(); + config.stdout_is_tty = atty::is(atty::Stream::Stdout); + config.stderr_is_tty = atty::is(atty::Stream::Stderr); + // set by build.rs config.build = TargetSelection::from_user(&env!("BUILD_TRIPLE")); diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index f10d640a2090..0eff8769b609 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -89,6 +89,7 @@ pub use crate::builder::PathSet; use crate::cache::{Interned, INTERNER}; pub use crate::config::Config; pub use crate::flags::Subcommand; +use yansi_term::Color; const LLVM_TOOLS: &[&str] = &[ "llvm-cov", // used to generate coverage report @@ -1575,6 +1576,23 @@ to download LLVM rather than building it. self.config.ninja_in_file } + + pub fn color_for_stdout(&self, color: Color, message: &str) -> String { + self.color_for_inner(color, message, self.config.stdout_is_tty) + } + + pub fn color_for_stderr(&self, color: Color, message: &str) -> String { + self.color_for_inner(color, message, self.config.stderr_is_tty) + } + + fn color_for_inner(&self, color: Color, message: &str, is_tty: bool) -> String { + let use_color = match self.config.color { + flags::Color::Always => true, + flags::Color::Never => false, + flags::Color::Auto => is_tty, + }; + if use_color { color.paint(message).to_string() } else { message.to_string() } + } } #[cfg(unix)] diff --git a/src/bootstrap/render_tests.rs b/src/bootstrap/render_tests.rs index ae2147f7550f..b5ab486f3ac6 100644 --- a/src/bootstrap/render_tests.rs +++ b/src/bootstrap/render_tests.rs @@ -10,6 +10,7 @@ use crate::builder::Builder; use std::io::{BufRead, BufReader, Write}; use std::process::{ChildStdout, Command, Stdio}; use std::time::Duration; +use yansi_term::Color; const TERSE_TESTS_PER_LINE: usize = 88; @@ -37,13 +38,12 @@ fn run_tests(builder: &Builder<'_>, cmd: &mut Command) -> bool { builder.verbose(&format!("running: {cmd:?}")); let mut process = cmd.spawn().unwrap(); - let stdout = process.stdout.take().unwrap(); - let verbose = builder.config.verbose_tests; - let handle = std::thread::spawn(move || Renderer::new(stdout, verbose).render_all()); + + // This runs until the stdout of the child is closed, which means the child exited. We don't + // run this on another thread since the builder is not Sync. + Renderer::new(process.stdout.take().unwrap(), builder).render_all(); let result = process.wait().unwrap(); - handle.join().expect("test formatter thread failed"); - if !result.success() && builder.is_verbose() { println!( "\n\ncommand did not execute successfully: {cmd:?}\n\ @@ -54,21 +54,21 @@ fn run_tests(builder: &Builder<'_>, cmd: &mut Command) -> bool { result.success() } -struct Renderer { +struct Renderer<'a> { stdout: BufReader, failures: Vec, - verbose: bool, + builder: &'a Builder<'a>, tests_count: Option, executed_tests: usize, terse_tests_in_line: usize, } -impl Renderer { - fn new(stdout: ChildStdout, verbose: bool) -> Self { +impl<'a> Renderer<'a> { + fn new(stdout: ChildStdout, builder: &'a Builder<'a>) -> Self { Self { stdout: BufReader::new(stdout), failures: Vec::new(), - verbose, + builder, tests_count: None, executed_tests: 0, terse_tests_in_line: 0, @@ -99,7 +99,7 @@ impl Renderer { fn render_test_outcome(&mut self, outcome: Outcome<'_>, test: &TestOutcome) { self.executed_tests += 1; - if self.verbose { + if self.builder.config.verbose_tests { self.render_test_outcome_verbose(outcome, test); } else { self.render_test_outcome_terse(outcome, test); @@ -109,12 +109,13 @@ impl Renderer { fn render_test_outcome_verbose(&self, outcome: Outcome<'_>, test: &TestOutcome) { if let Some(exec_time) = test.exec_time { println!( - "test {} ... {outcome} (in {:.2?})", + "test {} ... {} (in {:.2?})", test.name, + outcome.long(self.builder), Duration::from_secs_f64(exec_time) ); } else { - println!("test {} ... {outcome}", test.name); + println!("test {} ... {}", test.name, outcome.long(self.builder)); } } @@ -130,20 +131,13 @@ impl Renderer { } self.terse_tests_in_line += 1; - print!( - "{}", - match outcome { - Outcome::Ok => ".", - Outcome::Failed => "F", - Outcome::Ignored { .. } => "i", - } - ); + print!("{}", outcome.short(self.builder)); let _ = std::io::stdout().flush(); } fn render_suite_outcome(&self, outcome: Outcome<'_>, suite: &SuiteOutcome) { // The terse output doesn't end with a newline, so we need to add it ourselves. - if !self.verbose { + if !self.builder.config.verbose_tests { println!(); } @@ -163,8 +157,9 @@ impl Renderer { } println!( - "\ntest result: {outcome}. {} passed; {} failed; {} ignored; {} measured; \ + "\ntest result: {}. {} passed; {} failed; {} ignored; {} measured; \ {} filtered out; finished in {:.2?}\n", + outcome.long(self.builder), suite.passed, suite.failed, suite.ignored, @@ -213,13 +208,23 @@ enum Outcome<'a> { Ignored { reason: Option<&'a str> }, } -impl std::fmt::Display for Outcome<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl Outcome<'_> { + fn short(&self, builder: &Builder<'_>) -> String { match self { - Outcome::Ok => f.write_str("ok"), - Outcome::Failed => f.write_str("FAILED"), - Outcome::Ignored { reason: None } => f.write_str("ignored"), - Outcome::Ignored { reason: Some(reason) } => write!(f, "ignored, {reason}"), + Outcome::Ok => builder.color_for_stdout(Color::Green, "."), + Outcome::Failed => builder.color_for_stdout(Color::Red, "F"), + Outcome::Ignored { .. } => builder.color_for_stdout(Color::Yellow, "i"), + } + } + + fn long(&self, builder: &Builder<'_>) -> String { + match self { + Outcome::Ok => builder.color_for_stdout(Color::Green, "ok"), + Outcome::Failed => builder.color_for_stdout(Color::Red, "FAILED"), + Outcome::Ignored { reason: None } => builder.color_for_stdout(Color::Yellow, "ignored"), + Outcome::Ignored { reason: Some(reason) } => { + builder.color_for_stdout(Color::Yellow, &format!("ignored, {reason}")) + } } } }