Fix using global options before an alias.

Author: ehuss

crates/resolver-tests/tests/resolve.rs

@@ -61,7 +61,7 @@ proptest! {
         config
             .configure(
                 1,
-                None,
+                false,
                 None,
                 false,
                 false,
@@ -569,7 +569,7 @@ fn test_resolving_minimum_version_with_transitive_deps() {
     config
         .configure(
             1,
-            None,
+            false,
             None,
             false,
             false,

src/bin/cargo/cli.rs

@@ -92,16 +92,19 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"
         return Ok(());
     }
 
-    let args = expand_aliases(config, args)?;
-    let (cmd, subcommand_args) = match args.subcommand() {
+    // Global args need to be extracted before expanding aliases because the
+    // clap code for extracting a subcommand discards global options
+    // (appearing before the subcommand).
+    let (expanded_args, global_args) = expand_aliases(config, args)?;
+    let (cmd, subcommand_args) = match expanded_args.subcommand() {
         (cmd, Some(args)) => (cmd, args),
         _ => {
             // No subcommand provided.
             cli().print_help()?;
             return Ok(());
         }
     };
-    config_configure(config, &args, subcommand_args)?;
+    config_configure(config, &expanded_args, subcommand_args, global_args)?;
     super::init_git_transports(config);
 
     execute_subcommand(config, cmd, subcommand_args)
@@ -129,7 +132,7 @@ pub fn get_version_string(is_verbose: bool) -> String {
 fn expand_aliases(
     config: &mut Config,
     args: ArgMatches<'static>,
-) -> Result<ArgMatches<'static>, CliError> {
+) -> Result<(ArgMatches<'static>, GlobalArgs), CliError> {
     if let (cmd, Some(args)) = args.subcommand() {
         match (
             commands::builtin_exec(cmd),
@@ -148,41 +151,60 @@ fn expand_aliases(
                         .unwrap_or_default()
                         .map(|s| s.to_string()),
                 );
-                let args = cli()
+                // new_args strips out everything before the subcommand, so
+                // capture those global options now.
+                // Note that an alias to an external command will not receive
+                // these arguments. That may be confusing, but such is life.
+                let global_args = GlobalArgs::new(&args);
+                let new_args = cli()
                     .setting(AppSettings::NoBinaryName)
                     .get_matches_from_safe(alias)?;
-                return expand_aliases(config, args);
+                let (expanded_args, _) = expand_aliases(config, new_args)?;
+                return Ok((expanded_args, global_args));
             }
             (_, None) => {}
         }
     };
 
-    Ok(args)
+    Ok((args, GlobalArgs::default()))
 }
 
 fn config_configure(
     config: &mut Config,
     args: &ArgMatches<'_>,
     subcommand_args: &ArgMatches<'_>,
+    global_args: GlobalArgs,
 ) -> CliResult {
     let arg_target_dir = &subcommand_args.value_of_path("target-dir", config);
-    let config_args: Vec<&str> = args.values_of("config").unwrap_or_default().collect();
-    let quiet = if args.is_present("quiet") || subcommand_args.is_present("quiet") {
-        Some(true)
-    } else {
-        None
-    };
+    let verbose = global_args.verbose + args.occurrences_of("verbose") as u32;
+    // quiet is unusual because it is redefined in some subcommands in order
+    // to provide custom help text.
+    let quiet =
+        args.is_present("quiet") || subcommand_args.is_present("quiet") || global_args.quiet;
+    let global_color = global_args.color; // Extract so it can take reference.
+    let color = args
+        .value_of("color")
+        .or_else(|| global_color.as_ref().map(|s| s.as_ref()));
+    let frozen = args.is_present("frozen") || global_args.frozen;
+    let locked = args.is_present("locked") || global_args.locked;
+    let offline = args.is_present("offline") || global_args.offline;
+    let mut unstable_flags = global_args.unstable_flags;
+    if let Some(values) = args.values_of("unstable-features") {
+        unstable_flags.extend(values.map(|s| s.to_string()));
+    }
+    let mut config_args = global_args.config_args;
+    if let Some(values) = args.values_of("config") {
+        config_args.extend(values.map(|s| s.to_string()));
+    }
     config.configure(
-        args.occurrences_of("verbose") as u32,
+        verbose,
         quiet,
-        args.value_of("color"),
-        args.is_present("frozen"),
-        args.is_present("locked"),
-        args.is_present("offline"),
+        color,
+        frozen,
+        locked,
+        offline,
         arg_target_dir,
-        &args
-            .values_of_lossy("unstable-features")
-            .unwrap_or_default(),
+        &unstable_flags,
         &config_args,
     )?;
     Ok(())
@@ -202,6 +224,39 @@ fn execute_subcommand(
     super::execute_external_subcommand(config, cmd, &ext_args)
 }
 
+#[derive(Default)]
+struct GlobalArgs {
+    verbose: u32,
+    quiet: bool,
+    color: Option<String>,
+    frozen: bool,
+    locked: bool,
+    offline: bool,
+    unstable_flags: Vec<String>,
+    config_args: Vec<String>,
+}
+
+impl GlobalArgs {
+    fn new(args: &ArgMatches<'_>) -> GlobalArgs {
+        GlobalArgs {
+            verbose: args.occurrences_of("verbose") as u32,
+            quiet: args.is_present("quiet"),
+            color: args.value_of("color").map(|s| s.to_string()),
+            frozen: args.is_present("frozen"),
+            locked: args.is_present("locked"),
+            offline: args.is_present("offline"),
+            unstable_flags: args
+                .values_of_lossy("unstable-features")
+                .unwrap_or_default(),
+            config_args: args
+                .values_of("config")
+                .unwrap_or_default()
+                .map(|s| s.to_string())
+                .collect(),
+        }
+    }
+}
+
 fn cli() -> App {
     App::new("cargo")
         .settings(&[

src/cargo/util/config/mod.rs

@@ -602,14 +602,14 @@ impl Config {
     pub fn configure(
         &mut self,
         verbose: u32,
-        quiet: Option<bool>,
+        quiet: bool,
         color: Option<&str>,
         frozen: bool,
         locked: bool,
         offline: bool,
         target_dir: &Option<PathBuf>,
         unstable_flags: &[String],
-        cli_config: &[&str],
+        cli_config: &[String],
     ) -> CargoResult<()> {
         self.unstable_flags.parse(unstable_flags)?;
         if !cli_config.is_empty() {
@@ -618,7 +618,7 @@ impl Config {
             self.merge_cli_args()?;
         }
         let extra_verbose = verbose >= 2;
-        let verbose = if verbose == 0 { None } else { Some(true) };
+        let verbose = verbose != 0;
 
         #[derive(Deserialize, Default)]
         struct TermConfig {
@@ -632,25 +632,19 @@ impl Config {
         let color = color.or_else(|| term.color.as_ref().map(|s| s.as_ref()));
 
         let verbosity = match (verbose, term.verbose, quiet) {
-            (Some(true), _, None) | (None, Some(true), None) => Verbosity::Verbose,
+            (true, _, false) | (_, Some(true), false) => Verbosity::Verbose,
 
             // Command line takes precedence over configuration, so ignore the
             // configuration..
-            (None, _, Some(true)) => Verbosity::Quiet,
+            (false, _, true) => Verbosity::Quiet,
 
             // Can't pass both at the same time on the command line regardless
             // of configuration.
-            (Some(true), _, Some(true)) => {
+            (true, _, true) => {
                 bail!("cannot set both --verbose and --quiet");
             }
 
-            // Can't actually get `Some(false)` as a value from the command
-            // line, so just ignore them here to appease exhaustiveness checking
-            // in match statements.
-            (Some(false), _, _)
-            | (_, _, Some(false))
-            | (None, Some(false), None)
-            | (None, None, None) => Verbosity::Normal,
+            (false, _, false) => Verbosity::Normal,
         };
 
         let cli_target_dir = match target_dir.as_ref() {
@@ -659,7 +653,7 @@ impl Config {
         };
 
         self.shell().set_verbosity(verbosity);
-        self.shell().set_color_choice(color.map(|s| &s[..]))?;
+        self.shell().set_color_choice(color)?;
         self.extra_verbose = extra_verbose;
         self.frozen = frozen;
         self.locked = locked;

tests/testsuite/cargo_alias_config.rs

@@ -176,3 +176,19 @@ fn builtin_alias_takes_options() {
 
     p.cargo("r --example ex1 -- asdf").with_stdout("asdf").run();
 }
+
+#[cargo_test]
+fn global_options_with_alias() {
+    // Check that global options are passed through.
+    let p = project().file("src/lib.rs", "").build();
+
+    p.cargo("-v c")
+        .with_stderr(
+            "\
+[CHECKING] foo [..]
+[RUNNING] `rustc [..]
+[FINISHED] dev [..]
+",
+        )
+        .run();
+}

tests/testsuite/config.rs

@@ -76,17 +76,16 @@ impl ConfigBuilder {
         let homedir = paths::home();
         let mut config = Config::new(shell, cwd, homedir);
         config.set_env(self.env.clone());
-        let config_args: Vec<&str> = self.config_args.iter().map(AsRef::as_ref).collect();
         config.configure(
             0,
-            None,
+            false,
             None,
             false,
             false,
             false,
             &None,
             &self.unstable,
-            &config_args,
+            &self.config_args,
         )?;
         Ok(config)
     }