From 84348548d97846be112e94af03d20eab198b2857 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 20 Feb 2024 10:48:59 -0600 Subject: [PATCH 1/5] refactor(cli): Switch '-C' to using 'reload_rooted_at' --- src/bin/cargo/cli.rs | 10 +--------- src/bin/cargo/main.rs | 1 + src/cargo/util/config/mod.rs | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 9bcfcb14281..18f6afc658e 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -19,8 +19,6 @@ pub fn main(lazy_gctx: &mut LazyContext) -> CliResult { let args = cli().try_get_matches()?; // Update the process-level notion of cwd - // This must be completed before config is initialized - assert_eq!(lazy_gctx.is_init(), false); if let Some(new_cwd) = args.get_one::("directory") { // This is a temporary hack. This cannot access `Config`, so this is a bit messy. // This does not properly parse `-Z` flags that appear after the subcommand. @@ -40,6 +38,7 @@ pub fn main(lazy_gctx: &mut LazyContext) -> CliResult { .into()); } std::env::set_current_dir(&new_cwd).context("could not change to requested directory")?; + lazy_gctx.get_mut().reload_cwd()?; } // CAUTION: Be careful with using `config` until it is configured below. @@ -661,13 +660,6 @@ impl LazyContext { Self { gctx: None } } - /// Check whether the config is loaded - /// - /// This is useful for asserts in case the environment needs to be setup before loading - pub fn is_init(&self) -> bool { - self.gctx.is_some() - } - /// Get the config, loading it if needed /// /// On error, the process is terminated diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index 9eacb58d631..e1bb48bc3bd 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -20,6 +20,7 @@ fn main() { setup_logger(); let mut lazy_gctx = cli::LazyContext::new(); + lazy_gctx.get(); let result = if let Some(lock_addr) = cargo::ops::fix_get_proxy_lock_addr() { cargo::ops::fix_exec_rustc(lazy_gctx.get(), &lock_addr).map_err(|e| CliError::from(e)) diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index ce55d73328b..e4eefc977db 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -565,6 +565,25 @@ impl GlobalContext { self.search_stop_path = Some(path); } + /// Switches the working directory to [`std::env::current_dir`] + /// + /// There is not a need to also call [`Self::reload_rooted_at`]. + pub fn reload_cwd(&mut self) -> CargoResult<()> { + let cwd = env::current_dir() + .with_context(|| "couldn't get the current directory of the process")?; + let homedir = homedir(&cwd).ok_or_else(|| { + anyhow!( + "Cargo couldn't find your home directory. \ + This probably means that $HOME was not set." + ) + })?; + + self.cwd = cwd; + self.home_path = Filesystem::new(homedir); + self.reload_rooted_at(self.cwd.clone())?; + Ok(()) + } + /// Reloads on-disk configuration values, starting at the given path and /// walking up its ancestors. pub fn reload_rooted_at>(&mut self, path: P) -> CargoResult<()> { From e5fd597015c592598a3f2a0207f14347aa935a4d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 20 Feb 2024 10:53:30 -0600 Subject: [PATCH 2/5] Revert "refactor(cli): Lazily do first-pass config loading" This reverts commit eda1652b401de91bc5373906725aaa5a9b3dd04a. --- src/bin/cargo/cli.rs | 51 +++++-------------------------------------- src/bin/cargo/main.rs | 16 +++++++++----- 2 files changed, 17 insertions(+), 50 deletions(-) diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 18f6afc658e..88c9a7f4a52 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -1,5 +1,4 @@ use anyhow::{anyhow, Context as _}; -use cargo::core::shell::Shell; use cargo::core::{features, CliUnstable}; use cargo::{drop_print, drop_println, CargoResult}; use clap::builder::UnknownArgumentValueParser; @@ -15,7 +14,11 @@ use crate::command_prelude::*; use crate::util::is_rustup; use cargo::util::style; -pub fn main(lazy_gctx: &mut LazyContext) -> CliResult { +pub fn main(gctx: &mut GlobalContext) -> CliResult { + // CAUTION: Be careful with using `config` until it is configured below. + // In general, try to avoid loading config values unless necessary (like + // the [alias] table). + let args = cli().try_get_matches()?; // Update the process-level notion of cwd @@ -38,14 +41,9 @@ pub fn main(lazy_gctx: &mut LazyContext) -> CliResult { .into()); } std::env::set_current_dir(&new_cwd).context("could not change to requested directory")?; - lazy_gctx.get_mut().reload_cwd()?; + gctx.reload_cwd()?; } - // CAUTION: Be careful with using `config` until it is configured below. - // In general, try to avoid loading config values unless necessary (like - // the [alias] table). - let gctx = lazy_gctx.get_mut(); - let (expanded_args, global_args) = expand_aliases(gctx, args, vec![])?; if expanded_args @@ -645,43 +643,6 @@ See 'cargo help <>' for more information on a sp .subcommands(commands::builtin()) } -/// Delay loading [`GlobalContext`] until access. -/// -/// In the common path, the [`GlobalContext`] is dependent on CLI parsing and shouldn't be loaded until -/// after that is done but some other paths (like fix or earlier errors) might need access to it, -/// so this provides a way to share the instance and the implementation across these different -/// accesses. -pub struct LazyContext { - gctx: Option, -} - -impl LazyContext { - pub fn new() -> Self { - Self { gctx: None } - } - - /// Get the config, loading it if needed - /// - /// On error, the process is terminated - pub fn get(&mut self) -> &GlobalContext { - self.get_mut() - } - - /// Get the config, loading it if needed - /// - /// On error, the process is terminated - pub fn get_mut(&mut self) -> &mut GlobalContext { - self.gctx - .get_or_insert_with(|| match GlobalContext::default() { - Ok(cfg) => cfg, - Err(e) => { - let mut shell = Shell::new(); - cargo::exit_with_error(e.into(), &mut shell) - } - }) - } -} - #[test] fn verify_cli() { cli().debug_assert(); diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index e1bb48bc3bd..a84d300c498 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -1,5 +1,6 @@ #![allow(clippy::self_named_module_files)] // false positive in `commands/build.rs` +use cargo::core::shell::Shell; use cargo::util::network::http::http_handle; use cargo::util::network::http::needs_custom_http_transport; use cargo::util::{self, closest_msg, command_prelude, CargoResult}; @@ -19,18 +20,23 @@ use crate::command_prelude::*; fn main() { setup_logger(); - let mut lazy_gctx = cli::LazyContext::new(); - lazy_gctx.get(); + let mut gctx = match GlobalContext::default() { + Ok(gctx) => gctx, + Err(e) => { + let mut shell = Shell::new(); + cargo::exit_with_error(e.into(), &mut shell) + } + }; let result = if let Some(lock_addr) = cargo::ops::fix_get_proxy_lock_addr() { - cargo::ops::fix_exec_rustc(lazy_gctx.get(), &lock_addr).map_err(|e| CliError::from(e)) + cargo::ops::fix_exec_rustc(&gctx, &lock_addr).map_err(|e| CliError::from(e)) } else { let _token = cargo::util::job::setup(); - cli::main(&mut lazy_gctx) + cli::main(&mut gctx) }; match result { - Err(e) => cargo::exit_with_error(e, &mut lazy_gctx.get_mut().shell()), + Err(e) => cargo::exit_with_error(e, &mut *gctx.shell()), Ok(()) => {} } } From 4c3b0ade3d4bb10e3e7ccaffe135e850d6f5394f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 20 Feb 2024 11:00:14 -0600 Subject: [PATCH 3/5] refactor(shell): Group by type --- src/cargo/core/shell.rs | 126 ++++++++++++++++++++-------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs index 5c47b1b1c8f..35e9cc63416 100644 --- a/src/cargo/core/shell.rs +++ b/src/cargo/core/shell.rs @@ -9,44 +9,6 @@ use crate::util::errors::CargoResult; use crate::util::hostname; use crate::util::style::*; -pub enum TtyWidth { - NoTty, - Known(usize), - Guess(usize), -} - -impl TtyWidth { - /// Returns the width of the terminal to use for diagnostics (which is - /// relayed to rustc via `--diagnostic-width`). - pub fn diagnostic_terminal_width(&self) -> Option { - // ALLOWED: For testing cargo itself only. - #[allow(clippy::disallowed_methods)] - if let Ok(width) = std::env::var("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS") { - return Some(width.parse().unwrap()); - } - match *self { - TtyWidth::NoTty | TtyWidth::Guess(_) => None, - TtyWidth::Known(width) => Some(width), - } - } - - /// Returns the width used by progress bars for the tty. - pub fn progress_max_width(&self) -> Option { - match *self { - TtyWidth::NoTty => None, - TtyWidth::Known(width) | TtyWidth::Guess(width) => Some(width), - } - } -} - -/// The requested verbosity of output. -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum Verbosity { - Verbose, - Normal, - Quiet, -} - /// An abstraction around console output that remembers preferences for output /// verbosity and color. pub struct Shell { @@ -77,31 +39,6 @@ impl fmt::Debug for Shell { } } -/// A `Write`able object, either with or without color support -enum ShellOut { - /// A plain write object without color support - Write(AutoStream>), - /// Color-enabled stdio, with information on whether color should be used - Stream { - stdout: AutoStream, - stderr: AutoStream, - stderr_tty: bool, - color_choice: ColorChoice, - hyperlinks: bool, - }, -} - -/// Whether messages should use color output -#[derive(Debug, PartialEq, Clone, Copy)] -pub enum ColorChoice { - /// Force color output - Always, - /// Force disable color output - Never, - /// Intelligently guess whether to use color output - CargoAuto, -} - impl Shell { /// Creates a new shell (color choice and verbosity), defaulting to 'auto' color and verbose /// output. @@ -444,6 +381,20 @@ impl Default for Shell { } } +/// A `Write`able object, either with or without color support +enum ShellOut { + /// A plain write object without color support + Write(AutoStream>), + /// Color-enabled stdio, with information on whether color should be used + Stream { + stdout: AutoStream, + stderr: AutoStream, + stderr_tty: bool, + color_choice: ColorChoice, + hyperlinks: bool, + }, +} + impl ShellOut { /// Prints out a message with a status. The status comes first, and is bold plus the given /// color. The status can be justified, in which case the max width that will right align is @@ -488,6 +439,55 @@ impl ShellOut { } } +pub enum TtyWidth { + NoTty, + Known(usize), + Guess(usize), +} + +impl TtyWidth { + /// Returns the width of the terminal to use for diagnostics (which is + /// relayed to rustc via `--diagnostic-width`). + pub fn diagnostic_terminal_width(&self) -> Option { + // ALLOWED: For testing cargo itself only. + #[allow(clippy::disallowed_methods)] + if let Ok(width) = std::env::var("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS") { + return Some(width.parse().unwrap()); + } + match *self { + TtyWidth::NoTty | TtyWidth::Guess(_) => None, + TtyWidth::Known(width) => Some(width), + } + } + + /// Returns the width used by progress bars for the tty. + pub fn progress_max_width(&self) -> Option { + match *self { + TtyWidth::NoTty => None, + TtyWidth::Known(width) | TtyWidth::Guess(width) => Some(width), + } + } +} + +/// The requested verbosity of output. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum Verbosity { + Verbose, + Normal, + Quiet, +} + +/// Whether messages should use color output +#[derive(Debug, PartialEq, Clone, Copy)] +pub enum ColorChoice { + /// Force color output + Always, + /// Force disable color output + Never, + /// Intelligently guess whether to use color output + CargoAuto, +} + impl ColorChoice { /// Converts our color choice to anstream's version. fn to_anstream_color_choice(self) -> anstream::ColorChoice { From cf2cca9d8cfe701a6ae61c0dd6491f7ff59d0773 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 20 Feb 2024 11:07:41 -0600 Subject: [PATCH 4/5] refactor(shell): Allow reusing ColorChoice parsing --- src/cargo/core/shell.rs | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs index 35e9cc63416..e8ad1874ea5 100644 --- a/src/cargo/core/shell.rs +++ b/src/cargo/core/shell.rs @@ -236,18 +236,10 @@ impl Shell { .. } = self.output { - let cfg = match color { - Some("always") => ColorChoice::Always, - Some("never") => ColorChoice::Never, - - Some("auto") | None => ColorChoice::CargoAuto, - - Some(arg) => anyhow::bail!( - "argument for --color must be auto, always, or \ - never, but found `{}`", - arg - ), - }; + let cfg = color + .map(|c| c.parse()) + .transpose()? + .unwrap_or(ColorChoice::CargoAuto); *color_choice = cfg; let stdout_choice = cfg.to_anstream_color_choice(); let stderr_choice = cfg.to_anstream_color_choice(); @@ -499,6 +491,25 @@ impl ColorChoice { } } +impl std::str::FromStr for ColorChoice { + type Err = anyhow::Error; + fn from_str(color: &str) -> Result { + let cfg = match color { + "always" => ColorChoice::Always, + "never" => ColorChoice::Never, + + "auto" => ColorChoice::CargoAuto, + + arg => anyhow::bail!( + "argument for --color must be auto, always, or \ + never, but found `{}`", + arg + ), + }; + Ok(cfg) + } +} + fn supports_color(choice: anstream::ColorChoice) -> bool { match choice { anstream::ColorChoice::Always From 67474259e9893fdfa3a9f98148dc871e3c970e31 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 20 Feb 2024 11:16:30 -0600 Subject: [PATCH 5/5] fix(cli): Control clap colors through term.color Fixes #9012 --- src/bin/cargo/cli.rs | 26 +++++++++++++++++++++----- src/bin/cargo/commands/help.rs | 2 +- src/cargo/util/config/mod.rs | 12 ++++++------ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 88c9a7f4a52..f6c3f5f3497 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -1,5 +1,6 @@ use anyhow::{anyhow, Context as _}; use cargo::core::{features, CliUnstable}; +use cargo::util::config::TermConfig; use cargo::{drop_print, drop_println, CargoResult}; use clap::builder::UnknownArgumentValueParser; use itertools::Itertools; @@ -12,6 +13,7 @@ use super::commands; use super::list_commands; use crate::command_prelude::*; use crate::util::is_rustup; +use cargo::core::shell::ColorChoice; use cargo::util::style; pub fn main(gctx: &mut GlobalContext) -> CliResult { @@ -19,7 +21,7 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult { // In general, try to avoid loading config values unless necessary (like // the [alias] table). - let args = cli().try_get_matches()?; + let args = cli(gctx).try_get_matches()?; // Update the process-level notion of cwd if let Some(new_cwd) = args.get_one::("directory") { @@ -172,7 +174,7 @@ Run with `{literal}cargo -Z{literal:#} {placeholder}[FLAG] [COMMAND]{placeholder Some((cmd, args)) => (cmd, args), _ => { // No subcommand provided. - cli().print_help()?; + cli(gctx).print_help()?; return Ok(()); } }; @@ -335,7 +337,7 @@ For more information, see issue #12207 Command { +pub fn cli(gctx: &GlobalContext) -> Command { + // Don't let config errors get in the way of parsing arguments + let term = gctx.get::("term").unwrap_or_default(); + let color = term + .color + .and_then(|c| c.parse().ok()) + .unwrap_or(ColorChoice::CargoAuto); + let color = match color { + ColorChoice::Always => clap::ColorChoice::Always, + ColorChoice::Never => clap::ColorChoice::Never, + ColorChoice::CargoAuto => clap::ColorChoice::Auto, + }; + let usage = if is_rustup() { color_print::cstr!("cargo [+toolchain] [OPTIONS] [COMMAND]\n cargo [+toolchain] [OPTIONS] -Zscript <> [ARGS]...") } else { @@ -536,6 +550,7 @@ pub fn cli() -> Command { // We also want these to come before auto-generated `--help` .next_display_order(800) .allow_external_subcommands(true) + .color(color) .styles(styles) // Provide a custom help subcommand for calling into man pages .disable_help_subcommand(true) @@ -645,7 +660,8 @@ See 'cargo help <>' for more information on a sp #[test] fn verify_cli() { - cli().debug_assert(); + let gctx = GlobalContext::default().unwrap(); + cli(&gctx).debug_assert(); } #[test] diff --git a/src/bin/cargo/commands/help.rs b/src/bin/cargo/commands/help.rs index 48d0df0ecf0..a92f5d140bc 100644 --- a/src/bin/cargo/commands/help.rs +++ b/src/bin/cargo/commands/help.rs @@ -39,7 +39,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { } } } else { - let mut cmd = crate::cli::cli(); + let mut cmd = crate::cli::cli(gctx); let _ = cmd.print_help(); } Ok(()) diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index e4eefc977db..cf9402c7bc4 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -2636,14 +2636,14 @@ impl BuildTargetConfig { } #[derive(Deserialize, Default)] -struct TermConfig { - verbose: Option, - quiet: Option, - color: Option, - hyperlinks: Option, +pub struct TermConfig { + pub verbose: Option, + pub quiet: Option, + pub color: Option, + pub hyperlinks: Option, #[serde(default)] #[serde(deserialize_with = "progress_or_string")] - progress: Option, + pub progress: Option, } #[derive(Debug, Default, Deserialize)]