From 70d0659b1f41401ca6745a925f133c88da3664ae Mon Sep 17 00:00:00 2001 From: Rain Date: Mon, 9 Sep 2024 02:23:15 -0700 Subject: [PATCH] [cargo-nextest] use stylesheets everywhere, update supports-color (#1699) This picks up an update to supports-color v3, and also uses that to determine color support everywhere. We stop using owo-colors' `if_supports_color` because that hasn't been updated to v3 yet. --- Cargo.lock | 33 +----- Cargo.toml | 4 +- cargo-nextest/Cargo.toml | 2 +- cargo-nextest/src/dispatch.rs | 108 ++++++++++++------ cargo-nextest/src/double_spawn.rs | 7 +- cargo-nextest/src/errors.rs | 99 ++++++---------- cargo-nextest/src/helpers.rs | 10 +- cargo-nextest/src/main.rs | 6 +- cargo-nextest/src/output.rs | 103 +++++++++++------ cargo-nextest/src/update.rs | 16 +-- .../test-helpers/cargo-nextest-dup.rs | 5 +- workspace-hack/Cargo.toml | 1 - 12 files changed, 201 insertions(+), 193 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c9008c9f83b..25da890036a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -388,7 +388,7 @@ dependencies = [ "semver", "serde_json", "shell-words", - "supports-color 2.1.0", + "supports-color", "supports-unicode", "swrite", "thiserror", @@ -1448,17 +1448,6 @@ version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3" -[[package]] -name = "is-terminal" -version = "0.4.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f23ff5ef2b80d608d61efee834934d862cd92461afc0560dedf493e4c033738b" -dependencies = [ - "hermit-abi", - "libc", - "windows-sys 0.52.0", -] - [[package]] name = "is_ci" version = "1.2.0" @@ -1578,7 +1567,7 @@ dependencies = [ "cfg-if", "miette-derive", "owo-colors 4.0.0", - "supports-color 3.0.0", + "supports-color", "supports-hyperlinks", "supports-unicode", "terminal_size", @@ -1813,7 +1802,6 @@ dependencies = [ "miette", "miniz_oxide", "num-traits", - "owo-colors 4.0.0", "proc-macro2", "quote", "rand", @@ -1946,9 +1934,6 @@ name = "owo-colors" version = "4.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "caff54706df99d2a78a5a4e3455ff45448d81ef1bb63c22cd14052ca0e993a3f" -dependencies = [ - "supports-color 2.1.0", -] [[package]] name = "pathdiff" @@ -2791,19 +2776,9 @@ checksum = "81cdd64d312baedb58e21336b31bc043b77e01cc99033ce76ef539f78e965ebc" [[package]] name = "supports-color" -version = "2.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6398cde53adc3c4557306a96ce67b302968513830a77a95b2b17305d9719a89" -dependencies = [ - "is-terminal", - "is_ci", -] - -[[package]] -name = "supports-color" -version = "3.0.0" +version = "3.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9829b314621dfc575df4e409e79f9d6a66a3bd707ab73f23cb4aa3a854ac854f" +checksum = "8775305acf21c96926c900ad056abeef436701108518cf890020387236ac5a77" dependencies = [ "is_ci", ] diff --git a/Cargo.toml b/Cargo.toml index 1a80c426977..a798dd5a361 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,9 @@ guppy = "0.17.7" maplit = "1.0.2" miette = "7.2.0" once_cell = "1.19.0" -owo-colors = { version = "4.0.0", features = ["supports-colors"] } +# note: we don't use owo-colors' if_supports_color support for now, instead preferring to use our +# own supports-color + stylesheets everywhere. +owo-colors = "4.0.0" newtype-uuid = { version = "1.1.0", features = ["v4"] } nextest-metadata = { version = "0.12.1", path = "nextest-metadata" } nextest-workspace-hack = "0.1.0" diff --git a/cargo-nextest/Cargo.toml b/cargo-nextest/Cargo.toml index 460d8211e86..fe30ea5f92f 100644 --- a/cargo-nextest/Cargo.toml +++ b/cargo-nextest/Cargo.toml @@ -35,7 +35,7 @@ pathdiff = { version = "0.2.1", features = ["camino"] } quick-junit.workspace = true semver = "1.0.23" shell-words = "1.1.0" -supports-color = "2.1.0" +supports-color = "3.0.1" supports-unicode = "3.0.0" serde_json = "1.0.128" swrite.workspace = true diff --git a/cargo-nextest/src/dispatch.rs b/cargo-nextest/src/dispatch.rs index 50578157801..7ee5e4ab622 100644 --- a/cargo-nextest/src/dispatch.rs +++ b/cargo-nextest/src/dispatch.rs @@ -3,7 +3,7 @@ use crate::{ cargo_cli::{CargoCli, CargoOptions}, - output::{should_redact, OutputContext, OutputOpts, OutputWriter}, + output::{should_redact, OutputContext, OutputOpts, OutputWriter, StderrStyles}, reuse_build::{make_path_mapper, ArchiveFormatOpt, ReuseBuildOpts}, ExpectedError, Result, ReuseBuildKind, }; @@ -47,7 +47,7 @@ use nextest_runner::{ RustcCli, }; use once_cell::sync::OnceCell; -use owo_colors::{OwoColorize, Stream, Style}; +use owo_colors::OwoColorize; use quick_junit::XmlString; use semver::Version; use std::{ @@ -71,16 +71,32 @@ pub struct CargoNextestApp { } impl CargoNextestApp { + /// Initializes the output context. + pub fn init_output(&self) -> OutputContext { + match &self.subcommand { + NextestSubcommand::Nextest(args) => args.common.output.init(), + NextestSubcommand::Ntr(args) => args.common.output.init(), + #[cfg(unix)] + // Double-spawned processes should never use coloring. + NextestSubcommand::DoubleSpawn(_) => OutputContext::color_never_init(), + } + } + /// Executes the app. - pub fn exec(self, cli_args: Vec, output_writer: &mut OutputWriter) -> Result { + pub fn exec( + self, + cli_args: Vec, + output: OutputContext, + output_writer: &mut OutputWriter, + ) -> Result { #[cfg(feature = "experimental-tokio-console")] nextest_runner::console::init(); match self.subcommand { - NextestSubcommand::Nextest(app) => app.exec(cli_args, output_writer), - NextestSubcommand::Ntr(opts) => opts.exec(cli_args, output_writer), + NextestSubcommand::Nextest(app) => app.exec(cli_args, output, output_writer), + NextestSubcommand::Ntr(opts) => opts.exec(cli_args, output, output_writer), #[cfg(unix)] - NextestSubcommand::DoubleSpawn(opts) => opts.exec(), + NextestSubcommand::DoubleSpawn(opts) => opts.exec(output), } } } @@ -111,9 +127,12 @@ impl AppOpts { /// Execute the command. /// /// Returns the exit code. - fn exec(self, cli_args: Vec, output_writer: &mut OutputWriter) -> Result { - let output = self.common.output.init(); - + fn exec( + self, + cli_args: Vec, + output: OutputContext, + output_writer: &mut OutputWriter, + ) -> Result { match self.command { Command::List { cargo_options, @@ -173,8 +192,8 @@ impl AppOpts { } Command::ShowConfig { command } => command.exec( self.common.manifest_path, - self.common.output, self.common.config_opts, + output, output_writer, ), Command::Self_ { command } => command.exec(self.common.output), @@ -408,9 +427,12 @@ struct NtrOpts { } impl NtrOpts { - fn exec(self, cli_args: Vec, output_writer: &mut OutputWriter) -> Result { - let output = self.common.output.init(); - + fn exec( + self, + cli_args: Vec, + output: OutputContext, + output_writer: &mut OutputWriter, + ) -> Result { let base = BaseApp::new( output, self.run_opts.reuse_build, @@ -1079,9 +1101,11 @@ impl BaseApp { let host = HostPlatform::current(PlatformLibdir::from_rustc_stdout( RustcCli::print_host_libdir().read(), ))?; - let target = if let Some(triple) = - discover_target_triple(&cargo_configs, cargo_opts.target.as_deref()) - { + let target = if let Some(triple) = discover_target_triple( + &cargo_configs, + cargo_opts.target.as_deref(), + &output.stderr_styles(), + ) { let libdir = PlatformLibdir::from_rustc_stdout( RustcCli::print_target_libdir(&triple).read(), ); @@ -1189,6 +1213,8 @@ impl BaseApp { } fn check_version_config_initial(&self, version_cfg: &NextestVersionConfig) -> Result<()> { + let styles = self.output.stderr_styles(); + match version_cfg.eval( &self.current_version, self.config_opts.override_version_check, @@ -1210,8 +1236,8 @@ impl BaseApp { } => { log::warn!( "this repository recommends nextest version {}, but the current version is {}", - required.if_supports_color(Stream::Stderr, |x| x.bold()), - current.if_supports_color(Stream::Stderr, |x| x.bold()), + required.style(styles.bold), + current.style(styles.bold), ); if let Some(tool) = tool { log::info!( @@ -1267,6 +1293,8 @@ impl BaseApp { } fn check_version_config_final(&self, version_cfg: &NextestVersionConfig) -> Result<()> { + let styles = self.output.stderr_styles(); + match version_cfg.eval( &self.current_version, self.config_opts.override_version_check, @@ -1288,8 +1316,8 @@ impl BaseApp { } => { log::warn!( "this repository recommends nextest version {}, but the current version is {}", - required.if_supports_color(Stream::Stderr, |x| x.bold()), - current.if_supports_color(Stream::Stderr, |x| x.bold()), + required.style(styles.bold), + current.style(styles.bold), ); if let Some(tool) = tool { log::info!( @@ -1303,6 +1331,7 @@ impl BaseApp { crate::helpers::log_needs_update( log::Level::Info, crate::helpers::BYPASS_VERSION_TEXT, + &styles, ); Ok(()) @@ -1333,8 +1362,13 @@ impl BaseApp { } fn load_runner(&self, build_platforms: &BuildPlatforms) -> &TargetRunner { - self.target_runner - .get_or_init(|| runner_for_target(&self.cargo_configs, build_platforms)) + self.target_runner.get_or_init(|| { + runner_for_target( + &self.cargo_configs, + build_platforms, + &self.output.stderr_styles(), + ) + }) } fn exec_archive( @@ -1798,11 +1832,10 @@ impl ShowConfigCommand { fn exec( self, manifest_path: Option, - output: OutputOpts, config_opts: ConfigOpts, + output: OutputContext, output_writer: &mut OutputWriter, ) -> Result { - let output = output.init(); match self { Self::Version {} => { let mut cargo_cli = @@ -1856,6 +1889,7 @@ impl ShowConfigCommand { crate::helpers::log_needs_update( log::Level::Error, crate::helpers::BYPASS_VERSION_TEXT, + &output.stderr_styles(), ); Ok(nextest_metadata::NextestExitCode::REQUIRED_VERSION_NOT_MET) } @@ -1863,6 +1897,7 @@ impl ShowConfigCommand { crate::helpers::log_needs_update( log::Level::Warn, crate::helpers::BYPASS_VERSION_TEXT, + &output.stderr_styles(), ); Ok(nextest_metadata::NextestExitCode::RECOMMENDED_VERSION_NOT_MET) } @@ -2199,6 +2234,7 @@ fn acquire_graph_data( fn discover_target_triple( cargo_configs: &CargoConfigs, target_cli_option: Option<&str>, + styles: &StderrStyles, ) -> Option { match TargetTriple::find(cargo_configs, target_cli_option) { Ok(Some(triple)) => { @@ -2216,7 +2252,7 @@ fn discover_target_triple( None } Err(err) => { - warn_on_err("target triple", &err); + warn_on_err("target triple", &err, styles); None } } @@ -2225,52 +2261,48 @@ fn discover_target_triple( fn runner_for_target( cargo_configs: &CargoConfigs, build_platforms: &BuildPlatforms, + styles: &StderrStyles, ) -> TargetRunner { match TargetRunner::new(cargo_configs, build_platforms) { Ok(runner) => { if build_platforms.target.is_some() { if let Some(runner) = runner.target() { - log_platform_runner("for the target platform, ", runner); + log_platform_runner("for the target platform, ", runner, styles); } if let Some(runner) = runner.host() { - log_platform_runner("for the host platform, ", runner); + log_platform_runner("for the host platform, ", runner, styles); } } else { // If triple is None, then the host and target platforms use the same runner if // any. if let Some(runner) = runner.target() { - log_platform_runner("", runner); + log_platform_runner("", runner, styles); } } runner } Err(err) => { - warn_on_err("target runner", &err); + warn_on_err("target runner", &err, styles); TargetRunner::empty() } } } -fn log_platform_runner(prefix: &str, runner: &PlatformRunner) { +fn log_platform_runner(prefix: &str, runner: &PlatformRunner, styles: &StderrStyles) { let runner_command = shell_words::join(std::iter::once(runner.binary()).chain(runner.args())); log::info!( "{prefix}using target runner `{}` defined by {}", - runner_command.if_supports_color(Stream::Stderr, |s| s.bold()), + runner_command.style(styles.bold), runner.source() ) } -fn warn_on_err(thing: &str, err: &(dyn std::error::Error)) { +fn warn_on_err(thing: &str, err: &(dyn std::error::Error), styles: &StderrStyles) { let mut s = String::with_capacity(256); swrite!(s, "could not determine {thing}: {err}"); let mut next_error = err.source(); while let Some(err) = next_error { - swrite!( - s, - "\n {} {}", - "caused by:".if_supports_color(Stream::Stderr, |s| s.style(Style::new().yellow())), - err - ); + swrite!(s, "\n {} {}", "caused by:".style(styles.warning_text), err); next_error = err.source(); } diff --git a/cargo-nextest/src/double_spawn.rs b/cargo-nextest/src/double_spawn.rs index 8ff77c2446f..fea2e586641 100644 --- a/cargo-nextest/src/double_spawn.rs +++ b/cargo-nextest/src/double_spawn.rs @@ -1,7 +1,7 @@ // Copyright (c) The nextest Contributors // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::{output::Color, ExpectedError, Result}; +use crate::{output::OutputContext, ExpectedError, Result}; use camino::Utf8PathBuf; use clap::Args; use nextest_runner::double_spawn::double_spawn_child_init; @@ -17,9 +17,8 @@ pub(crate) struct DoubleSpawnOpts { } impl DoubleSpawnOpts { - pub(crate) fn exec(self) -> Result { - // This double-spawned process should never use coloring. - Color::Never.init(); + // output is passed in to ensure that the context is initialized. + pub(crate) fn exec(self, _output: OutputContext) -> Result { double_spawn_child_init(); let args = shell_words::split(&self.args).map_err(|err| { ExpectedError::DoubleSpawnParseArgsError { diff --git a/cargo-nextest/src/errors.rs b/cargo-nextest/src/errors.rs index 1e52f680a1d..cce4794c47c 100644 --- a/cargo-nextest/src/errors.rs +++ b/cargo-nextest/src/errors.rs @@ -1,13 +1,13 @@ // Copyright (c) The nextest Contributors // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::ExtractOutputFormat; +use crate::{output::StderrStyles, ExtractOutputFormat}; use camino::Utf8PathBuf; use itertools::Itertools; use nextest_filtering::errors::FiltersetParseErrors; use nextest_metadata::NextestExitCode; use nextest_runner::{errors::*, redact::Redactor}; -use owo_colors::{OwoColorize, Stream}; +use owo_colors::OwoColorize; use semver::Version; use std::{error::Error, string::FromUtf8Error}; use thiserror::Error; @@ -440,17 +440,14 @@ impl ExpectedError { } /// Displays this error to stderr. - pub fn display_to_stderr(&self) { + pub fn display_to_stderr(&self, styles: &StderrStyles) { let mut next_error = match &self { Self::SetCurrentDirFailed { error } => { log::error!("could not change to requested directory"); Some(error as &dyn Error) } Self::CargoMetadataExecFailed { command, err } => { - log::error!( - "failed to execute `{}`", - command.if_supports_color(Stream::Stderr, |x| x.bold()) - ); + log::error!("failed to execute `{}`", command.style(styles.bold)); Some(err as &dyn Error) } Self::CargoMetadataFailed { .. } => { @@ -458,10 +455,7 @@ impl ExpectedError { None } Self::CargoLocateProjectExecFailed { command, err } => { - log::error!( - "failed to execute `{}`", - command.if_supports_color(Stream::Stderr, |x| x.bold()) - ); + log::error!("failed to execute `{}`", command.style(styles.bold)); Some(err as &dyn Error) } Self::CargoLocateProjectFailed { .. } => { @@ -475,7 +469,7 @@ impl ExpectedError { Self::WorkspaceRootInvalid { workspace_root } => { log::error!( "workspace root `{}` is invalid", - workspace_root.if_supports_color(Stream::Stderr, |x| x.bold()) + workspace_root.style(styles.bold) ); None } @@ -491,7 +485,7 @@ impl ExpectedError { ReuseBuildKind::ReuseWithWorkspaceRemap { workspace_root } => { format!( "\n(hint: ensure that project source is available at {})", - workspace_root.if_supports_color(Stream::Stderr, |x| x.bold()) + workspace_root.style(styles.bold) ) } ReuseBuildKind::Reuse => { @@ -503,14 +497,14 @@ impl ExpectedError { }; log::error!( "workspace root manifest at {} does not exist{hint_str}", - path.if_supports_color(Stream::Stderr, |x| x.bold()) + path.style(styles.bold) ); None } Self::StoreDirCreateError { store_dir, err } => { log::error!( "failed to create store dir at `{}`", - store_dir.if_supports_color(Stream::Stderr, |x| x.bold()) + store_dir.style(styles.bold) ); Some(err as &dyn Error) } @@ -527,9 +521,7 @@ impl ExpectedError { "for config file `{}`{}, failed to parse overrides for profile: {}", err.config_file(), provided_by_tool(err.tool()), - override_error - .profile_name - .if_supports_color(Stream::Stderr, |p| p.bold()), + override_error.profile_name.style(styles.bold) ); for report in override_error.reports() { log::error!(target: "cargo_nextest::no_heading", "{report:?}"); @@ -543,18 +535,14 @@ impl ExpectedError { } => { let known_groups_str = known_groups .iter() - .map(|group_name| { - group_name.if_supports_color(Stream::Stderr, |x| x.bold()) - }) + .map(|group_name| group_name.style(styles.bold)) .join(", "); let mut errors_str = String::new(); for error in errors { errors_str.push_str(&format!( " - group `{}` in overrides for profile `{}`\n", - error.name.if_supports_color(Stream::Stderr, |x| x.bold()), - error - .profile_name - .if_supports_color(Stream::Stderr, |x| x.bold()) + error.name.style(styles.bold), + error.profile_name.style(styles.bold) )); } @@ -572,18 +560,14 @@ impl ExpectedError { } => { let known_scripts_str = known_scripts .iter() - .map(|group_name| { - group_name.if_supports_color(Stream::Stderr, |x| x.bold()) - }) + .map(|group_name| group_name.style(styles.bold)) .join(", "); let mut errors_str = String::new(); for error in errors { errors_str.push_str(&format!( " - script `{}` specified within profile `{}`\n", - error.name.if_supports_color(Stream::Stderr, |x| x.bold()), - error - .profile_name - .if_supports_color(Stream::Stderr, |x| x.bold()) + error.name.style(styles.bold), + error.profile_name.style(styles.bold) )); } @@ -598,15 +582,11 @@ impl ExpectedError { ConfigParseErrorKind::UnknownExperimentalFeatures { unknown, known } => { let unknown_str = unknown .iter() - .map(|feature_name| { - feature_name.if_supports_color(Stream::Stderr, |x| x.bold()) - }) + .map(|feature_name| feature_name.style(styles.bold)) .join(", "); let known_str = known .iter() - .map(|feature_name| { - feature_name.if_supports_color(Stream::Stderr, |x| x.bold()) - }) + .map(|feature_name| feature_name.style(styles.bold)) .join(", "); log::error!( @@ -639,14 +619,14 @@ impl ExpectedError { Self::MetadataMaterializeError { arg_name, err } => { log::error!( "error reading metadata from argument {}", - format!("--{arg_name}").if_supports_color(Stream::Stderr, |x| x.bold()) + format!("--{arg_name}").style(styles.bold) ); Some(err as &dyn Error) } Self::UnknownArchiveFormat { archive_file, err } => { log::error!( "failed to autodetect archive format for {}", - archive_file.if_supports_color(Stream::Stderr, |x| x.bold()) + archive_file.style(styles.bold) ); Some(err as &dyn Error) } @@ -657,16 +637,14 @@ impl ExpectedError { } => { log::error!( "error creating archive `{}`", - redactor - .redact_path(archive_file) - .if_supports_color(Stream::Stderr, |x| x.bold()) + redactor.redact_path(archive_file).style(styles.bold) ); Some(err as &dyn Error) } Self::ArchiveExtractError { archive_file, err } => { log::error!( "error extracting archive `{}`", - archive_file.if_supports_color(Stream::Stderr, |x| x.bold()) + archive_file.style(styles.bold) ); Some(err as &dyn Error) } @@ -677,17 +655,14 @@ impl ExpectedError { Self::PathMapperConstructError { arg_name, err } => { log::error!( "argument {} specified `{}` that couldn't be read", - format!("--{arg_name}").if_supports_color(Stream::Stderr, |x| x.bold()), - err.input().if_supports_color(Stream::Stderr, |x| x.bold()) + format!("--{arg_name}").style(styles.bold), + err.input().style(styles.bold) ); Some(err as &dyn Error) } Self::CargoMetadataParseError { file_name, err } => { let metadata_source = match file_name { - Some(path) => format!( - " from file `{}`", - path.if_supports_color(Stream::Stderr, |x| x.bold()) - ), + Some(path) => format!(" from file `{}`", path.style(styles.bold)), None => "".to_owned(), }; log::error!("error parsing Cargo metadata{}", metadata_source); @@ -702,26 +677,20 @@ impl ExpectedError { Some(err as &dyn Error) } Self::BuildExecFailed { command, err } => { - log::error!( - "failed to execute `{}`", - command.if_supports_color(Stream::Stderr, |x| x.bold()) - ); + log::error!("failed to execute `{}`", command.style(styles.bold)); Some(err as &dyn Error) } Self::BuildFailed { command, exit_code } => { let with_code_str = match exit_code { Some(code) => { - format!( - " with code {}", - code.if_supports_color(Stream::Stderr, |x| x.bold()) - ) + format!(" with code {}", code.style(styles.bold)) } None => "".to_owned(), }; log::error!( "command `{}` exited{}", - command.if_supports_color(Stream::Stderr, |x| x.bold()), + command.style(styles.bold), with_code_str, ); @@ -771,8 +740,8 @@ impl ExpectedError { } => { log::error!( "this repository requires nextest version {}, but the current version is {}", - required.if_supports_color(Stream::Stderr, |x| x.bold()), - current.if_supports_color(Stream::Stderr, |x| x.bold()), + required.style(styles.bold), + current.style(styles.bold), ); if let Some(tool) = tool { log::info!( @@ -785,6 +754,7 @@ impl ExpectedError { crate::helpers::log_needs_update( log::Level::Info, crate::helpers::BYPASS_VERSION_TEXT, + styles, ); None } @@ -797,7 +767,7 @@ impl ExpectedError { Self::UpdateError { err } => { log::error!( "failed to update nextest (please update manually by visiting <{}>)", - "https://get.nexte.st".if_supports_color(Stream::Stderr, |x| x.bold()) + "https://get.nexte.st".style(styles.bold) ); Some(err as &dyn Error) } @@ -849,10 +819,7 @@ impl ExpectedError { Some(err as &dyn Error) } Self::DebugExtractReadError { kind, path, err } => { - log::error!( - "error reading {kind} file `{}`", - path.if_supports_color(Stream::Stderr, |x| x.bold()), - ); + log::error!("error reading {kind} file `{}`", path.style(styles.bold),); Some(err as &dyn Error) } Self::DebugExtractWriteError { format, err } => { diff --git a/cargo-nextest/src/helpers.rs b/cargo-nextest/src/helpers.rs index aedf6bd947f..92c5dfb88d5 100644 --- a/cargo-nextest/src/helpers.rs +++ b/cargo-nextest/src/helpers.rs @@ -1,20 +1,22 @@ // Copyright (c) The nextest Contributors // SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::output::StderrStyles; + #[cfg(feature = "self-update")] -pub(crate) fn log_needs_update(level: log::Level, extra: &str) { - use owo_colors::{OwoColorize, Stream}; +pub(crate) fn log_needs_update(level: log::Level, extra: &str, styles: &StderrStyles) { + use owo_colors::OwoColorize; log::log!( level, "update nextest with {}{}", - "cargo nextest self update".if_supports_color(Stream::Stderr, |x| x.bold()), + "cargo nextest self update".style(styles.bold), extra, ); } #[cfg(not(feature = "self-update"))] -pub(crate) fn log_needs_update(level: log::Level, extra: &str) { +pub(crate) fn log_needs_update(level: log::Level, extra: &str, _styles: &StderrStyles) { log::log!(level, "update nextest via your package manager{}", extra); } diff --git a/cargo-nextest/src/main.rs b/cargo-nextest/src/main.rs index 918d8e7734f..4652d10bf24 100644 --- a/cargo-nextest/src/main.rs +++ b/cargo-nextest/src/main.rs @@ -14,10 +14,12 @@ fn main() -> Result<()> { .collect(); let opts = CargoNextestApp::parse(); - match opts.exec(cli_args, &mut OutputWriter::default()) { + let output = opts.init_output(); + + match opts.exec(cli_args, output, &mut OutputWriter::default()) { Ok(code) => std::process::exit(code), Err(error) => { - error.display_to_stderr(); + error.display_to_stderr(&output.stderr_styles()); std::process::exit(error.process_exit_code()) } } diff --git a/cargo-nextest/src/output.rs b/cargo-nextest/src/output.rs index a4369e059e6..0b77d81a778 100644 --- a/cargo-nextest/src/output.rs +++ b/cargo-nextest/src/output.rs @@ -7,7 +7,7 @@ use env_logger::fmt::Formatter; use log::{Level, LevelFilter, Record}; use miette::{GraphicalTheme, MietteHandlerOpts, ThemeStyles}; use nextest_runner::{reporter::ReporterStderr, write_str::WriteStr}; -use owo_colors::{style, OwoColorize, Stream, Style}; +use owo_colors::{style, OwoColorize, Style}; use std::{ io::{self, BufWriter, Stderr, Stdout, Write}, marker::PhantomData, @@ -74,11 +74,34 @@ impl OutputOpts { #[derive(Copy, Clone, Debug)] #[must_use] -pub(crate) struct OutputContext { +pub struct OutputContext { pub(crate) verbose: bool, pub(crate) color: Color, } +impl OutputContext { + // color_never_init is only used for double-spawning, which only exists on Unix platforms. + #[cfg(unix)] + pub(crate) fn color_never_init() -> Self { + Color::Never.init(); + Self { + verbose: false, + color: Color::Never, + } + } + + /// Returns general stderr styles for the current output context. + pub fn stderr_styles(&self) -> StderrStyles { + let mut styles = StderrStyles::default(); + + if self.color.should_colorize(supports_color::Stream::Stderr) { + styles.colorize(); + } + + styles + } +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)] #[must_use] #[derive(Default)] @@ -93,23 +116,17 @@ static INIT_LOGGER: std::sync::Once = std::sync::Once::new(); impl Color { pub(crate) fn init(self) { - match self { - Color::Auto => { - owo_colors::unset_override(); - } - Color::Always => { - owo_colors::set_override(true); - } - Color::Never => { - owo_colors::set_override(false); - } + // Pass the styles in as a stylesheet to ensure we use the latest supports-color here. + let mut log_styles = LogStyles::default(); + if self.should_colorize(supports_color::Stream::Stderr) { + log_styles.colorize(); } INIT_LOGGER.call_once(|| { env_logger::Builder::new() .filter_level(LevelFilter::Info) .parse_env("NEXTEST_LOG") - .format(format_fn) + .format(move |f, record| format_fn(f, record, &log_styles)) .init(); miette::set_hook(Box::new(move |_| { @@ -159,41 +176,51 @@ impl Color { } } -fn format_fn(f: &mut Formatter, record: &Record<'_>) -> std::io::Result<()> { +fn format_fn(f: &mut Formatter, record: &Record<'_>, styles: &LogStyles) -> std::io::Result<()> { if record.target() == "cargo_nextest::no_heading" { writeln!(f, "{}", record.args())?; return Ok(()); } match record.level() { - Level::Error => writeln!( - f, - "{}: {}", - "error".if_supports_color(Stream::Stderr, |s| s.style(Style::new().red().bold())), - record.args() - ), - Level::Warn => writeln!( - f, - "{}: {}", - "warning".if_supports_color(Stream::Stderr, |s| s.style(Style::new().yellow().bold())), - record.args() - ), - Level::Info => writeln!( - f, - "{}: {}", - "info".if_supports_color(Stream::Stderr, |s| s.bold()), - record.args() - ), - Level::Debug => writeln!( - f, - "{}: {}", - "debug".if_supports_color(Stream::Stderr, |s| s.bold()), - record.args() - ), + Level::Error => writeln!(f, "{}: {}", "error".style(styles.error), record.args()), + Level::Warn => writeln!(f, "{}: {}", "warning".style(styles.warning), record.args()), + Level::Info => writeln!(f, "{}: {}", "info".style(styles.info), record.args()), + Level::Debug => writeln!(f, "{}: {}", "debug".style(styles.debug), record.args()), _other => Ok(()), } } +#[derive(Debug, Default)] +struct LogStyles { + error: Style, + warning: Style, + info: Style, + debug: Style, +} + +impl LogStyles { + fn colorize(&mut self) { + self.error = style().red().bold(); + self.warning = style().yellow().bold(); + self.info = style().bold(); + self.debug = style().bold(); + } +} + +#[derive(Debug, Default)] +pub struct StderrStyles { + pub(crate) bold: Style, + pub(crate) warning_text: Style, +} + +impl StderrStyles { + fn colorize(&mut self) { + self.bold = style().bold(); + self.warning_text = style().yellow(); + } +} + /// A helper for capturing output in tests /// /// The test pass is gated by `#[cfg(test)]` to allow a better diff --git a/cargo-nextest/src/update.rs b/cargo-nextest/src/update.rs index a7236ea2e2d..1294b63ebf1 100644 --- a/cargo-nextest/src/update.rs +++ b/cargo-nextest/src/update.rs @@ -5,7 +5,7 @@ use crate::{output::OutputContext, ExpectedError, Result}; use camino::Utf8PathBuf; use nextest_metadata::NextestExitCode; use nextest_runner::update::{CheckStatus, MuktiBackend, UpdateVersion}; -use owo_colors::{OwoColorize, Stream}; +use owo_colors::OwoColorize; use semver::Version; use std::cmp::Ordering; @@ -57,11 +57,13 @@ pub(crate) fn perform_update( v.cmp_precedence(&min_version_with_setup()).is_ge() })?; + let styles = output.stderr_styles(); + match status { CheckStatus::AlreadyOnRequested(version) => { log::info!( "cargo-nextest is already at the latest version: {}", - version.if_supports_color(Stream::Stderr, |s| s.bold()) + version.style(styles.bold), ); Ok(0) } @@ -72,8 +74,8 @@ pub(crate) fn perform_update( log::info!( "not performing downgrade from {} to {}\n\ (pass in --force to force downgrade)", - current_version.if_supports_color(Stream::Stderr, |s| s.bold()), - requested.if_supports_color(Stream::Stderr, |s| s.bold()), + current_version.style(styles.bold), + requested.style(styles.bold), ); Ok(NextestExitCode::UPDATE_DOWNGRADE_NOT_PERFORMED) } @@ -85,8 +87,8 @@ pub(crate) fn perform_update( Ordering::Equal => "reinstall", Ordering::Less => "downgrade", }, - current_version.if_supports_color(Stream::Stderr, |s| s.bold()), - ctx.version.if_supports_color(Stream::Stderr, |s| s.bold()) + current_version.style(styles.bold), + ctx.version.style(styles.bold) ); if check { // check + non-empty ops implies a non-zero exit status. @@ -115,7 +117,7 @@ pub(crate) fn perform_update( .map_err(|err| ExpectedError::UpdateError { err })?; log::info!( "cargo-nextest updated to {}", - ctx.version.if_supports_color(Stream::Stderr, |s| s.bold()) + ctx.version.style(styles.bold) ); Ok(0) } else { diff --git a/integration-tests/test-helpers/cargo-nextest-dup.rs b/integration-tests/test-helpers/cargo-nextest-dup.rs index cb48dfffaaf..355818431ee 100644 --- a/integration-tests/test-helpers/cargo-nextest-dup.rs +++ b/integration-tests/test-helpers/cargo-nextest-dup.rs @@ -17,10 +17,11 @@ fn main() -> Result<()> { .collect(); let opts = CargoNextestApp::parse(); - match opts.exec(cli_args, &mut OutputWriter::default()) { + let output = opts.init_output(); + match opts.exec(cli_args, output, &mut OutputWriter::default()) { Ok(code) => std::process::exit(code), Err(error) => { - error.display_to_stderr(); + error.display_to_stderr(&output.stderr_styles()); std::process::exit(error.process_exit_code()) } } diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 77247d86eb6..859faa17f0c 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -26,7 +26,6 @@ log = { version = "0.4.22", default-features = false, features = ["std"] } memchr = { version = "2.7.4" } miette = { version = "7.2.0", features = ["fancy"] } num-traits = { version = "0.2.19", default-features = false, features = ["libm", "std"] } -owo-colors = { version = "4.0.0", default-features = false, features = ["supports-colors"] } rand = { version = "0.8.5" } serde = { version = "1.0.210", features = ["alloc", "derive"] } serde_json = { version = "1.0.128", features = ["unbounded_depth"] }