bootstrap: implement new feature bootstrap-self-test

onur-ozkan · onur-ozkan · commit 494cbd8ed2c0 · 2024-05-19T15:13:07.000+03:00
Some of the bootstrap logics should be ignored during unit tests because they either
make the tests take longer or cause them to fail. Therefore we need to be able to exclude
them from the bootstrap when it's called by unit tests. This change introduces a new feature
called `bootstrap-self-test`, which is enabled on bootstrap unit tests by default. This allows
us to keep the logic separate between compiler builds and bootstrap tests without needing messy
workarounds (like checking if target names match those in the unit tests).

Signed-off-by: onur-ozkan &lt;work@onurozkan.dev&gt;
diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml
@@ -7,6 +7,7 @@ default-run = "bootstrap"
 
 [features]
 build-metrics = ["sysinfo"]
+bootstrap-self-test = [] # enabled in the bootstrap unit tests
 
 [lib]
 path = "src/lib.rs"
diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs
@@ -3061,6 +3061,7 @@ impl Step for Bootstrap {
 
         let mut cmd = Command::new(&builder.initial_cargo);
         cmd.arg("test")
+            .args(["--features", "bootstrap-self-test"])
             .current_dir(builder.src.join("src/bootstrap"))
             .env("RUSTFLAGS", "-Cdebuginfo=2")
             .env("CARGO_TARGET_DIR", builder.out.join("bootstrap"))
diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs
@@ -22,8 +22,6 @@ use crate::utils::cache::{Interned, INTERNER};
 use crate::utils::channel::{self, GitInfo};
 use crate::utils::helpers::{exe, output, t};
 use build_helper::exit;
-use build_helper::util::fail;
-use semver::Version;
 use serde::{Deserialize, Deserializer};
 use serde_derive::Deserialize;
 
@@ -1249,11 +1247,13 @@ impl Config {
         // has already been (kinda-cross-)compiled to Windows land, we require a normal Windows path.
         cmd.arg("rev-parse").arg("--show-cdup");
         // Discard stderr because we expect this to fail when building from a tarball.
-        let output = cmd
-            .stderr(std::process::Stdio::null())
-            .output()
-            .ok()
-            .and_then(|output| if output.status.success() { Some(output) } else { None });
+        let output = cmd.stderr(std::process::Stdio::null()).output().ok().and_then(|output| {
+            if output.status.success() {
+                Some(output)
+            } else {
+                None
+            }
+        });
         if let Some(output) = output {
             let git_root_relative = String::from_utf8(output.stdout).unwrap();
             // We need to canonicalize this path to make sure it uses backslashes instead of forward slashes,
@@ -2346,7 +2346,11 @@ impl Config {
             .get(&target)
             .and_then(|t| t.split_debuginfo)
             .or_else(|| {
-                if self.build == target { self.rust_split_debuginfo_for_build_triple } else { None }
+                if self.build == target {
+                    self.rust_split_debuginfo_for_build_triple
+                } else {
+                    None
+                }
             })
             .unwrap_or_else(|| SplitDebuginfo::default_for_platform(target))
     }
@@ -2373,8 +2377,14 @@ impl Config {
         }
     }
 
-    // check rustc/cargo version is same or lower with 1 apart from the building one
+    #[cfg(feature = "bootstrap-self-test")]
+    pub fn check_stage0_version(&self, _program_path: &Path, _component_name: &'static str) {}
+
+    /// check rustc/cargo version is same or lower with 1 apart from the building one
+    #[cfg(not(feature = "bootstrap-self-test"))]
     pub fn check_stage0_version(&self, program_path: &Path, component_name: &'static str) {
+        use build_helper::util::fail;
+
         if self.dry_run() {
             return;
         }
@@ -2391,10 +2401,10 @@ impl Config {
         }
 
         let stage0_version =
-            Version::parse(stage0_output.next().unwrap().split('-').next().unwrap().trim())
+            semver::Version::parse(stage0_output.next().unwrap().split('-').next().unwrap().trim())
                 .unwrap();
         let source_version =
-            Version::parse(fs::read_to_string(self.src.join("src/version")).unwrap().trim())
+            semver::Version::parse(fs::read_to_string(self.src.join("src/version")).unwrap().trim())
                 .unwrap();
         if !(source_version == stage0_version
             || (source_version.major == stage0_version.major
diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs
@@ -15,10 +15,7 @@ fn parse(config: &str) -> Config {
     Config::parse_inner(
         &[
             "check".to_string(),
-            "--set=build.rustc=/does/not/exist".to_string(),
-            "--set=build.cargo=/does/not/exist".to_string(),
             "--config=/does/not/exist".to_string(),
-            "--skip-stage0-validation".to_string(),
         ],
         |&_| toml::from_str(&config).unwrap(),
     )
@@ -171,10 +168,7 @@ fn override_toml_duplicate() {
     Config::parse_inner(
         &[
             "check".to_owned(),
-            "--set=build.rustc=/does/not/exist".to_string(),
-            "--set=build.cargo=/does/not/exist".to_string(),
-            "--config=/does/not/exist".to_owned(),
-            "--skip-stage0-validation".to_owned(),
+            "--config=/does/not/exist".to_string(),
             "--set=change-id=1".to_owned(),
             "--set=change-id=2".to_owned(),
         ],
@@ -200,9 +194,6 @@ fn profile_user_dist() {
     Config::parse_inner(
         &[
             "check".to_owned(),
-            "--set=build.rustc=/does/not/exist".to_string(),
-            "--set=build.cargo=/does/not/exist".to_string(),
-            "--skip-stage0-validation".to_string(),
         ],
         get_toml,
     );
diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs
@@ -9,11 +9,10 @@ use std::{
 };
 
 use build_helper::ci::CiEnv;
-use build_helper::stage0_parser::VersionMetadata;
 use xz2::bufread::XzDecoder;
 
+use crate::utils::helpers::hex_encode;
 use crate::utils::helpers::{check_run, exe, move_file, program_out_of_date};
-use crate::{core::build_steps::llvm::detect_llvm_sha, utils::helpers::hex_encode};
 use crate::{t, Config};
 
 static SHOULD_FIX_BINS_AND_DYLIBS: OnceLock<bool> = OnceLock::new();
@@ -405,9 +404,17 @@ impl Config {
         cargo_clippy
     }
 
+    #[cfg(feature = "bootstrap-self-test")]
+    pub(crate) fn maybe_download_rustfmt(&self) -> Option<PathBuf> {
+        None
+    }
+
     /// NOTE: rustfmt is a completely different toolchain than the bootstrap compiler, so it can't
     /// reuse target directories or artifacts
+    #[cfg(not(feature = "bootstrap-self-test"))]
     pub(crate) fn maybe_download_rustfmt(&self) -> Option<PathBuf> {
+        use build_helper::stage0_parser::VersionMetadata;
+
         let VersionMetadata { date, version } = self.stage0_metadata.rustfmt.as_ref()?;
         let channel = format!("{version}-{date}");
 
@@ -487,6 +494,10 @@ impl Config {
         );
     }
 
+    #[cfg(feature = "bootstrap-self-test")]
+    pub(crate) fn download_beta_toolchain(&self) {}
+
+    #[cfg(not(feature = "bootstrap-self-test"))]
     pub(crate) fn download_beta_toolchain(&self) {
         self.verbose(|| println!("downloading stage0 beta artifacts"));
 
@@ -665,7 +676,13 @@ download-rustc = false
         self.unpack(&tarball, &bin_root, prefix);
     }
 
+    #[cfg(feature = "bootstrap-self-test")]
+    pub(crate) fn maybe_download_ci_llvm(&self) {}
+
+    #[cfg(not(feature = "bootstrap-self-test"))]
     pub(crate) fn maybe_download_ci_llvm(&self) {
+        use crate::core::build_steps::llvm::detect_llvm_sha;
+
         if !self.llvm_from_ci {
             return;
         }
@@ -707,6 +724,7 @@ download-rustc = false
         }
     }
 
+    #[cfg(not(feature = "bootstrap-self-test"))]
     fn download_ci_llvm(&self, llvm_sha: &str) {
         let llvm_assertions = self.llvm_assertions;
 
diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs
@@ -8,13 +8,15 @@
 //! In theory if we get past this phase it's a bug if a build fails, but in
 //! practice that's likely not true!
 
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fs;
 use std::path::PathBuf;
 use std::process::Command;
-use walkdir::WalkDir;
+
+#[cfg(not(feature = "bootstrap-self-test"))]
+use std::collections::HashSet;
 
 use crate::builder::Kind;
 use crate::core::config::Target;
@@ -31,6 +33,7 @@ pub struct Finder {
 // it might not yet be included in stage0. In such cases, we handle the targets missing from stage0 in this list.
 //
 // Targets can be removed from this list once they are present in the stage0 compiler (usually by updating the beta compiler of the bootstrap).
+#[cfg(not(feature = "bootstrap-self-test"))]
 const STAGE0_MISSING_TARGETS: &[&str] = &[
     // just a dummy comment so the list doesn't get onelined
 ];
@@ -167,6 +170,7 @@ than building it.
         .map(|p| cmd_finder.must_have(p))
         .or_else(|| cmd_finder.maybe_have("reuse"));
 
+    #[cfg(not(feature = "bootstrap-self-test"))]
     let stage0_supported_target_list: HashSet<String> =
         output(Command::new(&build.config.initial_rustc).args(["--print", "target-list"]))
             .lines()
@@ -193,11 +197,11 @@ than building it.
             continue;
         }
 
-        let target_str = target.to_string();
-
         // Ignore fake targets that are only used for unit tests in bootstrap.
-        if !["A-A", "B-B", "C-C"].contains(&target_str.as_str()) {
+        #[cfg(not(feature = "bootstrap-self-test"))]
+        {
             let mut has_target = false;
+            let target_str = target.to_string();
 
             let missing_targets_hashset: HashSet<_> = STAGE0_MISSING_TARGETS.iter().map(|t| t.to_string()).collect();
             let duplicated_targets: Vec<_> = stage0_supported_target_list.intersection(&missing_targets_hashset).collect();
@@ -222,7 +226,7 @@ than building it.
                     target_filename.push(".json");
 
                     // Recursively traverse through nested directories.
-                    let walker = WalkDir::new(custom_target_path).into_iter();
+                    let walker = walkdir::WalkDir::new(custom_target_path).into_iter();
                     for entry in walker.filter_map(|e| e.ok()) {
                         has_target |= entry.file_name() == target_filename;
                     }
