Skip to content

Migrate bootstrap to Clap-based argument parsing #110693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 7, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 70 additions & 22 deletions src/bootstrap/Cargo.lock
Original file line number Diff line number Diff line change
@@ -11,6 +11,12 @@ dependencies = [
"memchr",
]

[[package]]
name = "anstyle"
version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "41ed9a86bf92ae6580e0a31281f65a1b1d867c0cc68d5346e2ae128dddfa6a7d"

[[package]]
name = "autocfg"
version = "1.1.0"
@@ -38,10 +44,10 @@ version = "0.0.0"
dependencies = [
"build_helper",
"cc",
"clap",
"cmake",
"fd-lock",
"filetime",
"getopts",
"hex",
"ignore",
"is-terminal",
@@ -91,6 +97,46 @@ version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"

[[package]]
name = "clap"
version = "4.2.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "956ac1f6381d8d82ab4684768f89c0ea3afe66925ceadb4eeb3fc452ffc55d62"
dependencies = [
"clap_builder",
"clap_derive",
"once_cell",
]

[[package]]
name = "clap_builder"
version = "4.2.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "84080e799e54cff944f4b4a4b0e71630b0e0443b25b985175c7dddc1a859b749"
dependencies = [
"anstyle",
"bitflags",
"clap_lex",
]

[[package]]
name = "clap_derive"
version = "4.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3f9644cd56d6b87dbe899ef8b053e331c0637664e9e21a33dfcdc36093f5c5c4"
dependencies = [
"heck",
"proc-macro2",
"quote",
"syn 2.0.15",
]

[[package]]
name = "clap_lex"
version = "0.4.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8a2dd5a6fe8c6e3502f568a6353e5273bbb15193ad9a89e457b9970798efbea1"

[[package]]
name = "cmake"
version = "0.1.48"
@@ -175,7 +221,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f877be4f7c9f246b183111634f75baa039715e3f46ce860677d3b19a69fb229c"
dependencies = [
"quote",
"syn",
"syn 1.0.102",
]

[[package]]
@@ -260,15 +306,6 @@ dependencies = [
"version_check",
]

[[package]]
name = "getopts"
version = "0.2.21"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "14dbbfd5c71d70241ecf9e6f13737f7b5ce823821063188d7e46c41d371eebd5"
dependencies = [
"unicode-width",
]

[[package]]
name = "globset"
version = "0.4.8"
@@ -282,6 +319,12 @@ dependencies = [
"regex",
]

[[package]]
name = "heck"
version = "0.4.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8"

[[package]]
name = "hermit-abi"
version = "0.1.19"
@@ -486,18 +529,18 @@ dependencies = [

[[package]]
name = "proc-macro2"
version = "1.0.46"
version = "1.0.56"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "94e2ef8dbfc347b10c094890f778ee2e36ca9bb4262e86dc99cd217e35f3470b"
checksum = "2b63bdb0cd06f1f4dedf69b254734f9b45af66e4a031e42a7480257d9898b435"
dependencies = [
"unicode-ident",
]

[[package]]
name = "quote"
version = "1.0.18"
version = "1.0.26"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a1feb54ed693b93a84e14094943b84b7c4eae204c512b7ccb95ab0c66d278ad1"
checksum = "4424af4bf778aae2051a77b60283332f386554255d722233d09fbfc7e30da2fc"
dependencies = [
"proc-macro2",
]
@@ -606,7 +649,7 @@ checksum = "1f26faba0c3959972377d3b2d306ee9f71faee9714294e41bb777f83f88578be"
dependencies = [
"proc-macro2",
"quote",
"syn",
"syn 1.0.102",
]

[[package]]
@@ -642,6 +685,17 @@ dependencies = [
"unicode-ident",
]

[[package]]
name = "syn"
version = "2.0.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a34fcf3e8b60f57e6a14301a2e916d323af98b0ea63c599441eec8558660c822"
dependencies = [
"proc-macro2",
"quote",
"unicode-ident",
]

[[package]]
name = "sysinfo"
version = "0.26.7"
@@ -707,12 +761,6 @@ version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d22af068fba1eb5edcb4aea19d382b2a3deb4c8f9d475c589b6ada9e0fd493ee"

[[package]]
name = "unicode-width"
version = "0.1.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3ed742d4ea2bd1176e236172c8429aaf54486e7ac098db29ffe6529e0ce50973"

[[package]]
name = "version_check"
version = "0.9.4"
3 changes: 2 additions & 1 deletion src/bootstrap/Cargo.toml
Original file line number Diff line number Diff line change
@@ -34,7 +34,6 @@ is-terminal = "0.4"
build_helper = { path = "../tools/build_helper" }
cmake = "0.1.38"
filetime = "0.2"
getopts = "0.2.19"
cc = "1.0.69"
libc = "0.2"
hex = "0.4"
@@ -56,6 +55,7 @@ walkdir = "2"

# Dependencies needed by the build-metrics feature
sysinfo = { version = "0.26.0", optional = true }
clap = { version = "4.2.4", default-features = false, features = ["std", "usage", "help", "derive", "error-context"] }

# Solaris doesn't support flock() and thus fd-lock is not option now
[target.'cfg(not(target_os = "solaris"))'.dependencies]
@@ -86,6 +86,7 @@ build-metrics = ["sysinfo"]
# dependencies, only bootstrap itself.
[profile.dev]
debug = 0

[profile.dev.package]
# Only use debuginfo=1 to further reduce compile times.
bootstrap.debug = 1
31 changes: 19 additions & 12 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ pub use crate::Compiler;
// - use std::lazy for `Lazy`
// - use std::cell for `OnceCell`
// Once they get stabilized and reach beta.
use clap::ValueEnum;
use once_cell::sync::{Lazy, OnceCell};

pub struct Builder<'a> {
@@ -576,19 +577,24 @@ impl<'a> ShouldRun<'a> {
}
}

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, ValueEnum)]
pub enum Kind {
#[clap(alias = "b")]
Build,
#[clap(alias = "c")]
Check,
Clippy,
Fix,
Format,
#[clap(alias = "t")]
Test,
Bench,
#[clap(alias = "d")]
Doc,
Clean,
Dist,
Install,
#[clap(alias = "r")]
Run,
Setup,
Suggest,
@@ -887,18 +893,19 @@ impl<'a> Builder<'a> {
}

pub fn new(build: &Build) -> Builder<'_> {
let paths = &build.config.paths;
let (kind, paths) = match build.config.cmd {
Subcommand::Build { ref paths } => (Kind::Build, &paths[..]),
Subcommand::Check { ref paths } => (Kind::Check, &paths[..]),
Subcommand::Clippy { ref paths, .. } => (Kind::Clippy, &paths[..]),
Subcommand::Fix { ref paths } => (Kind::Fix, &paths[..]),
Subcommand::Doc { ref paths, .. } => (Kind::Doc, &paths[..]),
Subcommand::Test { ref paths, .. } => (Kind::Test, &paths[..]),
Subcommand::Bench { ref paths, .. } => (Kind::Bench, &paths[..]),
Subcommand::Dist { ref paths } => (Kind::Dist, &paths[..]),
Subcommand::Install { ref paths } => (Kind::Install, &paths[..]),
Subcommand::Run { ref paths, .. } => (Kind::Run, &paths[..]),
Subcommand::Clean { ref paths, .. } => (Kind::Clean, &paths[..]),
Subcommand::Build => (Kind::Build, &paths[..]),
Subcommand::Check { .. } => (Kind::Check, &paths[..]),
Subcommand::Clippy { .. } => (Kind::Clippy, &paths[..]),
Subcommand::Fix => (Kind::Fix, &paths[..]),
Subcommand::Doc { .. } => (Kind::Doc, &paths[..]),
Subcommand::Test { .. } => (Kind::Test, &paths[..]),
Subcommand::Bench { .. } => (Kind::Bench, &paths[..]),
Subcommand::Dist => (Kind::Dist, &paths[..]),
Subcommand::Install => (Kind::Install, &paths[..]),
Subcommand::Run { .. } => (Kind::Run, &paths[..]),
Subcommand::Clean { .. } => (Kind::Clean, &paths[..]),
Subcommand::Format { .. } => (Kind::Format, &[][..]),
Subcommand::Suggest { .. } => (Kind::Suggest, &[][..]),
Subcommand::Setup { profile: ref path } => (
19 changes: 11 additions & 8 deletions src/bootstrap/builder/tests.rs
Original file line number Diff line number Diff line change
@@ -236,7 +236,7 @@ mod defaults {
fn doc_default() {
let mut config = configure("doc", &["A"], &["A"]);
config.compiler_docs = true;
config.cmd = Subcommand::Doc { paths: Vec::new(), open: false, json: false };
config.cmd = Subcommand::Doc { open: false, json: false };
let mut cache = run_build(&[], config);
let a = TargetSelection::from_user("A");

@@ -545,19 +545,21 @@ mod dist {
fn test_with_no_doc_stage0() {
let mut config = configure(&["A"], &["A"]);
config.stage = 0;
config.paths = vec!["library/std".into()];
config.cmd = Subcommand::Test {
paths: vec!["library/std".into()],
test_args: vec![],
rustc_args: vec![],
fail_fast: true,
doc_tests: DocTests::No,
no_fail_fast: false,
no_doc: true,
doc: false,
bless: false,
force_rerun: false,
compare_mode: None,
rustfix_coverage: false,
pass: None,
run: None,
only_modified: false,
skip: vec![],
};

let build = Build::new(config);
@@ -587,7 +589,7 @@ mod dist {
fn doc_ci() {
let mut config = configure(&["A"], &["A"]);
config.compiler_docs = true;
config.cmd = Subcommand::Doc { paths: Vec::new(), open: false, json: false };
config.cmd = Subcommand::Doc { open: false, json: false };
let build = Build::new(config);
let mut builder = Builder::new(&build);
builder.run_step_descriptions(&Builder::get_step_descriptions(Kind::Doc), &[]);
@@ -616,11 +618,12 @@ mod dist {
// Behavior of `x.py test` doing various documentation tests.
let mut config = configure(&["A"], &["A"]);
config.cmd = Subcommand::Test {
paths: vec![],
test_args: vec![],
rustc_args: vec![],
fail_fast: true,
doc_tests: DocTests::Yes,
no_fail_fast: false,
doc: true,
no_doc: false,
skip: vec![],
bless: false,
force_rerun: false,
compare_mode: None,
18 changes: 5 additions & 13 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
@@ -20,15 +20,7 @@ fn args(builder: &Builder<'_>) -> Vec<String> {
arr.iter().copied().map(String::from)
}

if let Subcommand::Clippy {
fix,
clippy_lint_allow,
clippy_lint_deny,
clippy_lint_warn,
clippy_lint_forbid,
..
} = &builder.config.cmd
{
if let Subcommand::Clippy { fix, allow, deny, warn, forbid, .. } = &builder.config.cmd {
// disable the most spammy clippy lints
let ignored_lints = vec![
"many_single_char_names", // there are a lot in stdarch
@@ -53,10 +45,10 @@ fn args(builder: &Builder<'_>) -> Vec<String> {
args.extend(strings(&["--", "--cap-lints", "warn"]));
args.extend(ignored_lints.iter().map(|lint| format!("-Aclippy::{}", lint)));
let mut clippy_lint_levels: Vec<String> = Vec::new();
clippy_lint_allow.iter().for_each(|v| clippy_lint_levels.push(format!("-A{}", v)));
clippy_lint_deny.iter().for_each(|v| clippy_lint_levels.push(format!("-D{}", v)));
clippy_lint_warn.iter().for_each(|v| clippy_lint_levels.push(format!("-W{}", v)));
clippy_lint_forbid.iter().for_each(|v| clippy_lint_levels.push(format!("-F{}", v)));
allow.iter().for_each(|v| clippy_lint_levels.push(format!("-A{}", v)));
deny.iter().for_each(|v| clippy_lint_levels.push(format!("-D{}", v)));
warn.iter().for_each(|v| clippy_lint_levels.push(format!("-W{}", v)));
forbid.iter().for_each(|v| clippy_lint_levels.push(format!("-F{}", v)));
args.extend(clippy_lint_levels);
args.extend(builder.config.free_args.clone());
args
39 changes: 28 additions & 11 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ use crate::cache::{Interned, INTERNER};
use crate::cc_detect::{ndk_compiler, Language};
use crate::channel::{self, GitInfo};
pub use crate::flags::Subcommand;
use crate::flags::{Color, Flags};
use crate::flags::{Color, Flags, Warnings};
use crate::util::{exe, output, t};
use once_cell::sync::OnceCell;
use serde::{Deserialize, Deserializer};
@@ -237,6 +237,8 @@ pub struct Config {
initial_rustfmt: RefCell<RustfmtState>,
#[cfg(test)]
pub initial_rustfmt: RefCell<RustfmtState>,

pub paths: Vec<PathBuf>,
}

#[derive(Default, Deserialize, Clone)]
@@ -376,6 +378,16 @@ pub struct TargetSelection {
file: Option<Interned<String>>,
}

/// Newtype over `Vec<TargetSelection>` so we can implement custom parsing logic
#[derive(Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub struct TargetSelectionList(Vec<TargetSelection>);

pub fn target_selection_list(s: &str) -> Result<TargetSelectionList, String> {
Ok(TargetSelectionList(
s.split(",").filter(|s| !s.is_empty()).map(TargetSelection::from_user).collect(),
))
}

impl TargetSelection {
pub fn from_user(selection: &str) -> Self {
let path = Path::new(selection);
@@ -871,26 +883,24 @@ impl Config {
}

fn parse_inner<'a>(args: &[String], get_toml: impl 'a + Fn(&Path) -> TomlConfig) -> Config {
let flags = Flags::parse(&args);
let mut flags = Flags::parse(&args);
let mut config = Config::default_opts();

// Set flags.
config.paths = std::mem::take(&mut flags.paths);
config.exclude = flags.exclude.into_iter().map(|path| TaskPath::parse(path)).collect();
config.include_default_paths = flags.include_default_paths;
config.rustc_error_format = flags.rustc_error_format;
config.json_output = flags.json_output;
config.on_fail = flags.on_fail;
config.jobs = flags.jobs.map(threads_from_config);
config.jobs = Some(threads_from_config(flags.jobs as u32));
config.cmd = flags.cmd;
config.incremental = flags.incremental;
config.dry_run = if flags.dry_run { DryRun::UserSelected } else { DryRun::Disabled };
config.keep_stage = flags.keep_stage;
config.keep_stage_std = flags.keep_stage_std;
config.color = flags.color;
config.free_args = flags.free_args.clone().unwrap_or_default();
if let Some(value) = flags.deny_warnings {
config.deny_warnings = value;
}
config.free_args = std::mem::take(&mut flags.free_args);
config.llvm_profile_use = flags.llvm_profile_use;
config.llvm_profile_generate = flags.llvm_profile_generate;
config.llvm_bolt_profile_generate = flags.llvm_bolt_profile_generate;
@@ -1021,14 +1031,14 @@ impl Config {
config.out = dir;
}

config.hosts = if let Some(arg_host) = flags.host {
config.hosts = if let Some(TargetSelectionList(arg_host)) = flags.host {
arg_host
} else if let Some(file_host) = build.host {
file_host.iter().map(|h| TargetSelection::from_user(h)).collect()
} else {
vec![config.build]
};
config.targets = if let Some(arg_target) = flags.target {
config.targets = if let Some(TargetSelectionList(arg_target)) = flags.target {
arg_target
} else if let Some(file_target) = build.target {
file_target.iter().map(|h| TargetSelection::from_user(h)).collect()
@@ -1064,7 +1074,7 @@ impl Config {
set(&mut config.print_step_rusage, build.print_step_rusage);
set(&mut config.patch_binaries_for_nix, build.patch_binaries_for_nix);

config.verbose = cmp::max(config.verbose, flags.verbose);
config.verbose = cmp::max(config.verbose, flags.verbose as usize);

if let Some(install) = toml.install {
config.prefix = install.prefix.map(PathBuf::from);
@@ -1137,7 +1147,14 @@ impl Config {
config.rustc_default_linker = rust.default_linker;
config.musl_root = rust.musl_root.map(PathBuf::from);
config.save_toolstates = rust.save_toolstates.map(PathBuf::from);
set(&mut config.deny_warnings, flags.deny_warnings.or(rust.deny_warnings));
set(
&mut config.deny_warnings,
match flags.warnings {
Warnings::Deny => Some(true),
Warnings::Warn => Some(false),
Warnings::Default => rust.deny_warnings,
},
);
set(&mut config.backtrace_on_ice, rust.backtrace_on_ice);
set(&mut config.rust_verify_llvm_ir, rust.verify_llvm_ir);
config.rust_thin_lto_import_instr_limit = rust.thin_lto_import_instr_limit;
8 changes: 7 additions & 1 deletion src/bootstrap/config/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::{Config, TomlConfig};
use super::{Config, Flags, TomlConfig};
use clap::CommandFactory;
use std::{env, path::Path};

fn toml(config: &str) -> impl '_ + Fn(&Path) -> TomlConfig {
@@ -88,3 +89,8 @@ fn detect_src_and_out() {
test(parse("build.build-dir = \"/tmp\""), build_dir);
}
}

#[test]
fn clap_verify() {
Flags::command().debug_assert();
}
995 changes: 336 additions & 659 deletions src/bootstrap/flags.rs

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
@@ -660,8 +660,8 @@ impl Build {

// hardcoded subcommands
match &self.config.cmd {
Subcommand::Format { check, paths } => {
return format::format(&builder::Builder::new(&self), *check, &paths);
Subcommand::Format { check } => {
return format::format(&builder::Builder::new(&self), *check, &self.config.paths);
}
Subcommand::Suggest { run } => {
return suggest::suggest(&builder::Builder::new(&self), *run);
2 changes: 1 addition & 1 deletion src/bootstrap/test.rs
Original file line number Diff line number Diff line change
@@ -1545,7 +1545,7 @@ note: if you're sure you want to do this, please open an issue as to why. In the

// Get paths from cmd args
let paths = match &builder.config.cmd {
Subcommand::Test { ref paths, .. } => &paths[..],
Subcommand::Test { .. } => &builder.config.paths[..],
_ => &[],
};