refactor(cli): Lazy load config

Cargo.toml

@@ -61,7 +61,7 @@ toml_edit =  { version = "0.14.3", features = ["serde", "easy", "perf"] }
 unicode-xid = "0.2.0"
 url = "2.2.2"
 walkdir = "2.2"
-clap = "3.2.1"
+clap = "3.2.18"
 unicode-width = "0.1.5"
 openssl = { version = '0.10.11', optional = true }
 im-rc = "15.0.0"

src/bin/cargo/cli.rs

@@ -1,10 +1,8 @@
 use anyhow::anyhow;
+use cargo::core::shell::Shell;
 use cargo::core::{features, CliUnstable};
 use cargo::{self, drop_print, drop_println, CliResult, Config};
-use clap::{
-    error::{ContextKind, ContextValue},
-    AppSettings, Arg, ArgMatches,
-};
+use clap::{AppSettings, Arg, ArgMatches};
 use itertools::Itertools;
 use std::collections::HashMap;
 use std::fmt::Write;
@@ -24,36 +22,13 @@ lazy_static::lazy_static! {
     ]);
 }
 
-pub fn main(config: &mut Config) -> CliResult {
+pub fn main(config: &mut LazyConfig) -> CliResult {
+    let args = cli().try_get_matches()?;
+
     // 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).
-
-    if commands::help::handle_embedded_help(config) {
-        return Ok(());
-    }
-
-    let args = match cli().try_get_matches() {
-        Ok(args) => args,
-        Err(e) => {
-            if e.kind() == clap::ErrorKind::UnrecognizedSubcommand {
-                // An unrecognized subcommand might be an external subcommand.
-                let cmd = e
-                    .context()
-                    .find_map(|c| match c {
-                        (ContextKind::InvalidSubcommand, &ContextValue::String(ref cmd)) => {
-                            Some(cmd)
-                        }
-                        _ => None,
-                    })
-                    .expect("UnrecognizedSubcommand implies the presence of InvalidSubcommand");
-                return super::execute_external_subcommand(config, cmd, &[cmd, "--help"])
-                    .map_err(|_| e.into());
-            } else {
-                return Err(e.into());
-            }
-        }
-    };
+    let config = config.get_mut();
 
     // Global args need to be extracted before expanding aliases because the
     // clap code for extracting a subcommand discards global options
@@ -412,7 +387,7 @@ impl GlobalArgs {
     }
 }
 
-fn cli() -> App {
+pub fn cli() -> App {
     let is_rustup = std::env::var_os("RUSTUP_HOME").is_some();
     let usage = if is_rustup {
         "cargo [+toolchain] [OPTIONS] [SUBCOMMAND]"
@@ -425,6 +400,8 @@ fn cli() -> App {
         // Doesn't mix well with our list of common cargo commands.  See clap-rs/clap#3108 for
         // opening clap up to allow us to style our help template
         .disable_colored_help(true)
+        // Provide a custom help subcommand for calling into man pages
+        .disable_help_subcommand(true)
         .override_usage(usage)
         .help_template(
             "\
@@ -488,6 +465,49 @@ See 'cargo help <command>' for more information on a specific command.\n",
         .subcommands(commands::builtin())
 }
 
+/// Delay loading [`Config`] until access.
+///
+/// In the common path, the [`Config`] 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 LazyConfig {
+    config: Option<Config>,
+}
+
+impl LazyConfig {
+    pub fn new() -> Self {
+        Self { config: 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.config.is_some()
+    }
+
+    /// Get the config, loading it if needed
+    ///
+    /// On error, the process is terminated
+    pub fn get(&mut self) -> &Config {
+        self.get_mut()
+    }
+
+    /// Get the config, loading it if needed
+    ///
+    /// On error, the process is terminated
+    pub fn get_mut(&mut self) -> &mut Config {
+        self.config.get_or_insert_with(|| match Config::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();

src/bin/cargo/commands/help.rs

@@ -1,4 +1,5 @@
 use crate::aliased_command;
+use crate::command_prelude::*;
 use cargo::util::errors::CargoResult;
 use cargo::{drop_println, Config};
 use cargo_util::paths::resolve_executable;
@@ -10,43 +11,26 @@ use std::path::Path;
 
 const COMPRESSED_MAN: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/man.tgz"));
 
-/// Checks if the `help` command is being issued.
-///
-/// This runs before clap processing, because it needs to intercept the `help`
-/// command if a man page is available.
-///
-/// Returns `true` if help information was successfully displayed to the user.
-/// In this case, Cargo should exit.
-pub fn handle_embedded_help(config: &Config) -> bool {
-    match try_help(config) {
-        Ok(true) => true,
-        Ok(false) => false,
-        Err(e) => {
-            log::warn!("help failed: {:?}", e);
-            false
-        }
-    }
+pub fn cli() -> App {
+    subcommand("help")
+        .about("Displays help for a cargo subcommand")
+        .arg(Arg::new("SUBCOMMAND"))
 }
 
-fn try_help(config: &Config) -> CargoResult<bool> {
-    let mut args = std::env::args_os()
-        .skip(1)
-        .skip_while(|arg| arg.to_str().map_or(false, |s| s.starts_with('-')));
-    if !args
-        .next()
-        .map_or(false, |arg| arg.to_str() == Some("help"))
-    {
-        return Ok(false);
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
+    let subcommand = args.get_one::<String>("SUBCOMMAND");
+    if let Some(subcommand) = subcommand {
+        if !try_help(config, subcommand)? {
+            crate::execute_external_subcommand(config, subcommand, &[subcommand, "--help"])?;
+        }
+    } else {
+        let mut cmd = crate::cli::cli();
+        let _ = cmd.print_help();
     }
-    let subcommand = match args.next() {
-        Some(arg) => arg,
-        None => return Ok(false),
-    };
-    let subcommand = match subcommand.to_str() {
-        Some(s) => s,
-        None => return Ok(false),
-    };
+    Ok(())
+}
 
+fn try_help(config: &Config, subcommand: &str) -> CargoResult<bool> {
     let subcommand = match check_alias(config, subcommand) {
         // If this alias is more than a simple subcommand pass-through, show the alias.
         Some(argv) if argv.len() > 1 => {

src/bin/cargo/commands/mod.rs

@@ -13,6 +13,7 @@ pub fn builtin() -> Vec<App> {
         fix::cli(),
         generate_lockfile::cli(),
         git_checkout::cli(),
+        help::cli(),
         init::cli(),
         install::cli(),
         locate_project::cli(),
@@ -54,6 +55,7 @@ pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches) -> CliResu
         "fix" => fix::exec,
         "generate-lockfile" => generate_lockfile::exec,
         "git-checkout" => git_checkout::exec,
+        "help" => help::exec,
         "init" => init::exec,
         "install" => install::exec,
         "locate-project" => locate_project::exec,

src/bin/cargo/main.rs

@@ -1,7 +1,6 @@
 #![warn(rust_2018_idioms)] // while we're getting used to 2018
 #![allow(clippy::all)]
 
-use cargo::core::shell::Shell;
 use cargo::util::toml::StringOrVec;
 use cargo::util::CliError;
 use cargo::util::{self, closest_msg, command_prelude, CargoResult, CliResult, Config};
@@ -22,25 +21,17 @@ fn main() {
     #[cfg(not(feature = "pretty-env-logger"))]
     env_logger::init_from_env("CARGO_LOG");
 
-    let mut config = match Config::default() {
-        Ok(cfg) => cfg,
-        Err(e) => {
-            let mut shell = Shell::new();
-            cargo::exit_with_error(e.into(), &mut shell)
-        }
-    };
+    let mut config = cli::LazyConfig::new();
 
-    let result = match cargo::ops::fix_maybe_exec_rustc(&config) {
-        Ok(true) => Ok(()),
-        Ok(false) => {
-            let _token = cargo::util::job::setup();
-            cli::main(&mut config)
-        }
-        Err(e) => Err(CliError::from(e)),
+    let result = if let Some(lock_addr) = cargo::ops::fix_get_proxy_lock_addr() {
+        cargo::ops::fix_exec_rustc(config.get(), &lock_addr).map_err(|e| CliError::from(e))
+    } else {
+        let _token = cargo::util::job::setup();
+        cli::main(&mut config)
     };
 
     match result {
-        Err(e) => cargo::exit_with_error(e, &mut *config.shell()),
+        Err(e) => cargo::exit_with_error(e, &mut config.get_mut().shell()),
         Ok(()) => {}
     }
 }

src/cargo/ops/fix.rs

@@ -333,20 +333,23 @@ to prevent this issue from happening.
     Ok(())
 }
 
+/// Provide the lock address when running in proxy mode
+///
+/// Returns `None` if `fix` is not being run (not in proxy mode). Returns
+/// `Some(...)` if in `fix` proxy mode
+pub fn fix_get_proxy_lock_addr() -> Option<String> {
+    env::var(FIX_ENV).ok()
+}
+
 /// Entry point for `cargo` running as a proxy for `rustc`.
 ///
 /// This is called every time `cargo` is run to check if it is in proxy mode.
 ///
-/// Returns `false` if `fix` is not being run (not in proxy mode). Returns
-/// `true` if in `fix` proxy mode, and the fix was complete without any
-/// warnings or errors. If there are warnings or errors, this does not return,
+/// If there are warnings or errors, this does not return,
 /// and the process exits with the corresponding `rustc` exit code.
-pub fn fix_maybe_exec_rustc(config: &Config) -> CargoResult<bool> {
-    let lock_addr = match env::var(FIX_ENV) {
-        Ok(s) => s,
-        Err(_) => return Ok(false),
-    };
-
+///
+/// See [`fix_proxy_lock_addr`]
+pub fn fix_exec_rustc(config: &Config, lock_addr: &str) -> CargoResult<()> {
     let args = FixArgs::get()?;
     trace!("cargo-fix as rustc got file {:?}", args.file);
 
@@ -392,7 +395,7 @@ pub fn fix_maybe_exec_rustc(config: &Config) -> CargoResult<bool> {
         // any. If stderr is empty then there's no need for the final exec at
         // the end, we just bail out here.
         if output.status.success() && output.stderr.is_empty() {
-            return Ok(true);
+            return Ok(());
         }
 
         // Otherwise, if our rustc just failed, then that means that we broke the

src/cargo/ops/mod.rs

@@ -19,7 +19,7 @@ pub use self::cargo_read_manifest::{read_package, read_packages};
 pub use self::cargo_run::run;
 pub use self::cargo_test::{run_benches, run_tests, TestOptions};
 pub use self::cargo_uninstall::uninstall;
-pub use self::fix::{fix, fix_maybe_exec_rustc, FixOptions};
+pub use self::fix::{fix, fix_exec_rustc, fix_get_proxy_lock_addr, FixOptions};
 pub use self::lockfile::{load_pkg_lockfile, resolve_to_string, write_pkg_lockfile};
 pub use self::registry::HttpTimeout;
 pub use self::registry::{configure_http_handle, http_handle, http_handle_and_timeout};