Skip to content

Commit

Permalink
Fix using global options before an alias.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Jan 31, 2020
1 parent 2a0f0c8 commit 0279e8e
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 40 deletions.
4 changes: 2 additions & 2 deletions crates/resolver-tests/tests/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ proptest! {
config
.configure(
1,
None,
false,
None,
false,
false,
Expand Down Expand Up @@ -569,7 +569,7 @@ fn test_resolving_minimum_version_with_transitive_deps() {
config
.configure(
1,
None,
false,
None,
false,
false,
Expand Down
97 changes: 76 additions & 21 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand All @@ -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(())
Expand All @@ -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(&[
Expand Down
22 changes: 8 additions & 14 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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 {
Expand All @@ -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() {
Expand All @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions tests/testsuite/cargo_alias_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
5 changes: 2 additions & 3 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 0279e8e

Please sign in to comment.