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.
Dylan-DPC-zz · Sep 15, 2020 · 328c61c · 328c61c
1 parent 12c10e3
commit 328c61c
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 42 deletions.
diff --git 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
@@ -526,23 +526,9 @@ impl<'a> Builder<'a> {
     }
 
     fn new_internal(build: &Build, kind: Kind, paths: Vec<PathBuf>) -> 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
@@ -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
@@ -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<String>,
-    pub stage: Option<u32>,
+    pub stage: u32,
     pub keep_stage: Vec<u32>,
     pub src: PathBuf,
     pub jobs: Option<u32>,
@@ -300,6 +299,12 @@ struct Build {
     configure_args: Option<Vec<String>>,
     local_rebuild: Option<bool>,
     print_step_timings: Option<bool>,
+    doc_stage: Option<u32>,
+    build_stage: Option<u32>,
+    test_stage: Option<u32>,
+    install_stage: Option<u32>,
+    dist_stage: Option<u32>,
+    bench_stage: Option<u32>,
 }
 
 /// 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 {