From 328c61c15711ae8a741b2aa56af1c450f54d1796 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 11 Sep 2020 22:17:16 -0400 Subject: [PATCH] Make the default stage for x.py configurable This allows configuring the default stage for each sub-command individually. - Normalize the stage as early as possible, so there's no confusion about which to use. - Don't add an explicit `stage` option in config.toml This offers no more flexibility than `*_stage` and makes it confusing which takes precedence. - Always give `--stage N` precedence over config.toml - Fix bootstrap tests This changes the tests to go through `Config::parse` so that they test the actual defaults, not the dummy ones provided by `default_opts`. To make this workable (and independent of the environment), it does not read `config.toml` for tests. --- config.toml.example | 17 ++++++++++ src/bootstrap/builder.rs | 31 ++---------------- src/bootstrap/builder/tests.rs | 14 ++++---- src/bootstrap/config.rs | 59 ++++++++++++++++++++++++++++++---- 4 files changed, 79 insertions(+), 42 deletions(-) diff --git a/config.toml.example b/config.toml.example index 2d5b3136450..f10d1a7b44e 100644 --- a/config.toml.example +++ b/config.toml.example @@ -111,6 +111,23 @@ # General build configuration options # ============================================================================= [build] +# The default stage to use for the `doc` subcommand +#doc-stage = 0 + +# The default stage to use for the `build` subcommand +#build-stage = 1 + +# The default stage to use for the `test` subcommand +#test-stage = 1 + +# The default stage to use for the `dist` subcommand +#dist-stage = 2 + +# The default stage to use for the `install` subcommand +#install-stage = 2 + +# The default stage to use for the `bench` subcommand +#bench-stage = 2 # Build triple for the original snapshot compiler. This must be a compiler that # nightlies are already produced for. The current platform must be able to run diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 31d4f1f28a8..034b01a502c 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -526,23 +526,9 @@ impl<'a> Builder<'a> { } fn new_internal(build: &Build, kind: Kind, paths: Vec) -> Builder<'_> { - let top_stage = if let Some(explicit_stage) = build.config.stage { - explicit_stage - } else { - // See https://github.com/rust-lang/compiler-team/issues/326 - match kind { - Kind::Doc => 0, - Kind::Build | Kind::Test => 1, - Kind::Bench | Kind::Dist | Kind::Install => 2, - // These are all bootstrap tools, which don't depend on the compiler. - // The stage we pass shouldn't matter, but use 0 just in case. - Kind::Check | Kind::Clippy | Kind::Fix | Kind::Run | Kind::Format => 0, - } - }; - Builder { build, - top_stage, + top_stage: build.config.stage, kind, cache: Cache::new(), stack: RefCell::new(Vec::new()), @@ -566,20 +552,7 @@ impl<'a> Builder<'a> { Subcommand::Format { .. } | Subcommand::Clean { .. } => panic!(), }; - let this = Self::new_internal(build, kind, paths.to_owned()); - - // CI should always run stage 2 builds, unless it specifically states otherwise - #[cfg(not(test))] - if build.config.stage.is_none() && build.ci_env != crate::CiEnv::None { - match kind { - Kind::Test | Kind::Doc | Kind::Build | Kind::Bench | Kind::Dist | Kind::Install => { - assert_eq!(this.top_stage, 2) - } - Kind::Check | Kind::Clippy | Kind::Fix | Kind::Run | Kind::Format => {} - } - } - - this + Self::new_internal(build, kind, paths.to_owned()) } pub fn execute_cli(&self) { diff --git a/src/bootstrap/builder/tests.rs b/src/bootstrap/builder/tests.rs index c6eac95c345..f96925f9270 100644 --- a/src/bootstrap/builder/tests.rs +++ b/src/bootstrap/builder/tests.rs @@ -2,8 +2,8 @@ use super::*; use crate::config::{Config, TargetSelection}; use std::thread; -fn configure(host: &[&str], target: &[&str]) -> Config { - let mut config = Config::default_opts(); +fn configure(cmd: &str, host: &[&str], target: &[&str]) -> Config { + let mut config = Config::parse(&[cmd.to_owned()]); // don't save toolstates config.save_toolstates = None; config.skip_only_host_steps = false; @@ -42,7 +42,7 @@ mod defaults { #[test] fn build_default() { - let build = Build::new(configure(&[], &[])); + let build = Build::new(configure("build", &[], &[])); let mut builder = Builder::new(&build); builder.run_step_descriptions(&Builder::get_step_descriptions(Kind::Build), &[]); @@ -70,7 +70,7 @@ mod defaults { #[test] fn build_stage_0() { - let config = Config { stage: Some(0), ..configure(&[], &[]) }; + let config = Config { stage: 0, ..configure("build", &[], &[]) }; let build = Build::new(config); let mut builder = Builder::new(&build); builder.run_step_descriptions(&Builder::get_step_descriptions(Kind::Build), &[]); @@ -92,7 +92,7 @@ mod defaults { #[test] fn doc_default() { - let mut config = configure(&[], &[]); + let mut config = configure("doc", &[], &[]); config.compiler_docs = true; config.cmd = Subcommand::Doc { paths: Vec::new(), open: false }; let build = Build::new(config); @@ -126,7 +126,7 @@ mod dist { use pretty_assertions::assert_eq; fn configure(host: &[&str], target: &[&str]) -> Config { - Config { stage: Some(2), ..super::configure(host, target) } + Config { stage: 2, ..super::configure("dist", host, target) } } #[test] @@ -455,7 +455,7 @@ mod dist { #[test] fn test_with_no_doc_stage0() { let mut config = configure(&[], &[]); - config.stage = Some(0); + config.stage = 0; config.cmd = Subcommand::Test { paths: vec!["library/std".into()], test_args: vec![], diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 5a79d3db5c9..cc0a534314a 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -10,7 +10,6 @@ use std::ffi::OsString; use std::fmt; use std::fs; use std::path::{Path, PathBuf}; -use std::process; use crate::cache::{Interned, INTERNER}; use crate::flags::Flags; @@ -57,7 +56,7 @@ pub struct Config { pub skip_only_host_steps: bool, pub on_fail: Option, - pub stage: Option, + pub stage: u32, pub keep_stage: Vec, pub src: PathBuf, pub jobs: Option, @@ -300,6 +299,12 @@ struct Build { configure_args: Option>, local_rebuild: Option, print_step_timings: Option, + doc_stage: Option, + build_stage: Option, + test_stage: Option, + install_stage: Option, + dist_stage: Option, + bench_stage: Option, } /// TOML representation of various global install decisions. @@ -480,13 +485,11 @@ impl Config { pub fn parse(args: &[String]) -> Config { let flags = Flags::parse(&args); - let file = flags.config.clone(); let mut config = Config::default_opts(); config.exclude = flags.exclude; config.rustc_error_format = flags.rustc_error_format; config.json_output = flags.json_output; config.on_fail = flags.on_fail; - config.stage = flags.stage; config.jobs = flags.jobs.map(threads_from_config); config.cmd = flags.cmd; config.incremental = flags.incremental; @@ -503,8 +506,14 @@ impl Config { config.out = dir; } - let toml = file + #[cfg(test)] + let toml = TomlConfig::default(); + #[cfg(not(test))] + let toml = flags + .config .map(|file| { + use std::process; + let contents = t!(fs::read_to_string(&file)); match toml::from_str(&contents) { Ok(table) => table, @@ -520,7 +529,7 @@ impl Config { }) .unwrap_or_else(TomlConfig::default); - let build = toml.build.clone().unwrap_or_default(); + let build = toml.build.unwrap_or_default(); // If --target was specified but --host wasn't specified, don't run any host-only tests. let has_hosts = build.host.is_some() || flags.host.is_some(); @@ -564,6 +573,44 @@ impl Config { set(&mut config.configure_args, build.configure_args); set(&mut config.local_rebuild, build.local_rebuild); set(&mut config.print_step_timings, build.print_step_timings); + + // See https://github.com/rust-lang/compiler-team/issues/326 + config.stage = match config.cmd { + Subcommand::Doc { .. } => flags.stage.or(build.doc_stage).unwrap_or(0), + Subcommand::Build { .. } => flags.stage.or(build.build_stage).unwrap_or(1), + Subcommand::Test { .. } => flags.stage.or(build.test_stage).unwrap_or(1), + Subcommand::Bench { .. } => flags.stage.or(build.bench_stage).unwrap_or(2), + Subcommand::Dist { .. } => flags.stage.or(build.dist_stage).unwrap_or(2), + Subcommand::Install { .. } => flags.stage.or(build.install_stage).unwrap_or(2), + // These are all bootstrap tools, which don't depend on the compiler. + // The stage we pass shouldn't matter, but use 0 just in case. + Subcommand::Clean { .. } + | Subcommand::Check { .. } + | Subcommand::Clippy { .. } + | Subcommand::Fix { .. } + | Subcommand::Run { .. } + | Subcommand::Format { .. } => flags.stage.unwrap_or(0), + }; + + // CI should always run stage 2 builds, unless it specifically states otherwise + #[cfg(not(test))] + if flags.stage.is_none() && crate::CiEnv::current() != crate::CiEnv::None { + match config.cmd { + Subcommand::Test { .. } + | Subcommand::Doc { .. } + | Subcommand::Build { .. } + | Subcommand::Bench { .. } + | Subcommand::Dist { .. } + | Subcommand::Install { .. } => assert_eq!(config.stage, 2), + Subcommand::Clean { .. } + | Subcommand::Check { .. } + | Subcommand::Clippy { .. } + | Subcommand::Fix { .. } + | Subcommand::Run { .. } + | Subcommand::Format { .. } => {} + } + } + config.verbose = cmp::max(config.verbose, flags.verbose); if let Some(ref install) = toml.install {