Skip to content

Commit

Permalink
Rollup merge of #76800 - jyn514:usage, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Don't generate bootstrap usage unless it's needed

Previously, `x.py` would unconditionally run `x.py build` to get the
help message. After #76165,
when checking the CI stage was moved into `Config`, that would cause an
assertion failure (but only only in CI!):

```
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `2`', src/bootstrap/config.rs:619:49
```

This changes bootstrap to only generate a help message when it needs
to (when someone passes `--help`).

r? @Mark-Simulacrum
This should fix the CI failures in #76797 and #75991.
  • Loading branch information
RalfJung authored Sep 20, 2020
2 parents 2911b8c + c35ce3f commit b603143
Showing 1 changed file with 32 additions and 30 deletions.
62 changes: 32 additions & 30 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ impl Default for Subcommand {

impl Flags {
pub fn parse(args: &[String]) -> Flags {
let mut extra_help = String::new();
let mut subcommand_help = String::from(
"\
Usage: x.py <subcommand> [options] [<paths>...]
Expand Down Expand Up @@ -170,16 +169,6 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`",
"VALUE",
);

// fn usage()
let usage =
|exit_code: i32, opts: &Options, subcommand_help: &str, extra_help: &str| -> ! {
println!("{}", opts.usage(subcommand_help));
if !extra_help.is_empty() {
println!("{}", extra_help);
}
process::exit(exit_code);
};

// We can't use getopt to parse the options until we have completed specifying which
// options are valid, but under the current implementation, some options are conditional on
// the subcommand. Therefore we must manually identify the subcommand first, so that we can
Expand Down Expand Up @@ -263,12 +252,38 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`",
_ => {}
};

// fn usage()
let usage = |exit_code: i32, opts: &Options, verbose: bool, subcommand_help: &str| -> ! {
let mut extra_help = String::new();

// All subcommands except `clean` can have an optional "Available paths" section
if verbose {
let config = Config::parse(&["build".to_string()]);
let build = Build::new(config);

let maybe_rules_help = Builder::get_help(&build, subcommand.as_str());
extra_help.push_str(maybe_rules_help.unwrap_or_default().as_str());
} else if !(subcommand.as_str() == "clean" || subcommand.as_str() == "fmt") {
extra_help.push_str(
format!("Run `./x.py {} -h -v` to see a list of available paths.", subcommand)
.as_str(),
);
}

println!("{}", opts.usage(subcommand_help));
if !extra_help.is_empty() {
println!("{}", extra_help);
}
process::exit(exit_code);
};

// Done specifying what options are possible, so do the getopts parsing
let matches = opts.parse(&args[..]).unwrap_or_else(|e| {
// Invalid argument/option format
println!("\n{}\n", e);
usage(1, &opts, &subcommand_help, &extra_help);
usage(1, &opts, false, &subcommand_help);
});

// Extra sanity check to make sure we didn't hit this crazy corner case:
//
// ./x.py --frobulate clean build
Expand Down Expand Up @@ -436,24 +451,11 @@ Arguments:
let paths = matches.free[1..].iter().map(|p| p.into()).collect::<Vec<PathBuf>>();

let cfg_file = env::var_os("BOOTSTRAP_CONFIG").map(PathBuf::from);

// All subcommands except `clean` can have an optional "Available paths" section
if matches.opt_present("verbose") {
let config = Config::parse(&["build".to_string()]);
let build = Build::new(config);

let maybe_rules_help = Builder::get_help(&build, subcommand.as_str());
extra_help.push_str(maybe_rules_help.unwrap_or_default().as_str());
} else if !(subcommand.as_str() == "clean" || subcommand.as_str() == "fmt") {
extra_help.push_str(
format!("Run `./x.py {} -h -v` to see a list of available paths.", subcommand)
.as_str(),
);
}
let verbose = matches.opt_present("verbose");

// User passed in -h/--help?
if matches.opt_present("help") {
usage(0, &opts, &subcommand_help, &extra_help);
usage(0, &opts, verbose, &subcommand_help);
}

let cmd = match subcommand.as_str() {
Expand Down Expand Up @@ -483,7 +485,7 @@ Arguments:
"clean" => {
if !paths.is_empty() {
println!("\nclean does not take a path argument\n");
usage(1, &opts, &subcommand_help, &extra_help);
usage(1, &opts, verbose, &subcommand_help);
}

Subcommand::Clean { all: matches.opt_present("all") }
Expand All @@ -494,12 +496,12 @@ Arguments:
"run" | "r" => {
if paths.is_empty() {
println!("\nrun requires at least a path!\n");
usage(1, &opts, &subcommand_help, &extra_help);
usage(1, &opts, verbose, &subcommand_help);
}
Subcommand::Run { paths }
}
_ => {
usage(1, &opts, &subcommand_help, &extra_help);
usage(1, &opts, verbose, &subcommand_help);
}
};

Expand Down

0 comments on commit b603143

Please sign in to comment.