From 8f677e8fb20f0ad6d93b39d8b736ee7bc0939973 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sun, 19 May 2024 11:42:34 +0300 Subject: [PATCH] bootstrap: implement new feature `bootstrap-self-test` 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 --- src/bootstrap/Cargo.toml | 1 + src/bootstrap/src/core/build_steps/test.rs | 1 + src/bootstrap/src/core/config/config.rs | 19 +++++++++------ src/bootstrap/src/core/config/tests.rs | 28 ++++------------------ src/bootstrap/src/core/download.rs | 22 +++++++++++++++-- src/bootstrap/src/core/sanity.rs | 16 ++++++++----- 6 files changed, 49 insertions(+), 38 deletions(-) diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index ca0d1fa5bd0..32dd3efa7a6 100644 --- 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 index 29b3d1669b4..aaedee65ed7 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -3053,6 +3053,7 @@ fn run(self, builder: &Builder<'_>) { 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 index 70d32f2f6d9..17e37c1ecd2 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -22,8 +22,6 @@ 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; @@ -2382,8 +2380,14 @@ pub fn git_config(&self) -> GitConfig<'_> { } } - // 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; } @@ -2400,11 +2404,12 @@ pub fn check_stage0_version(&self, program_path: &Path, component_name: &'static } let stage0_version = - 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(stage0_output.next().unwrap().split('-').next().unwrap().trim()) .unwrap(); + let source_version = 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 && (source_version.minor == stage0_version.minor diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index c4073910623..bfb2c02860d 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/src/bootstrap/src/core/config/tests.rs @@ -14,16 +14,9 @@ }; 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(), - ) + Config::parse_inner(&["check".to_string(), "--config=/does/not/exist".to_string()], |&_| { + toml::from_str(&config).unwrap() + }) } #[test] @@ -212,10 +205,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(), ], @@ -238,15 +228,7 @@ fn get_toml(file: &Path) -> TomlConfig { .and_then(|table: toml::Value| TomlConfig::deserialize(table)) .unwrap() } - 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, - ); + Config::parse_inner(&["check".to_owned()], get_toml); } #[test] diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 60f48c5923e..2b11b8c3d4f 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -9,11 +9,10 @@ }; 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 = OnceLock::new(); @@ -405,9 +404,17 @@ pub(crate) fn download_clippy(&self) -> PathBuf { cargo_clippy } + #[cfg(feature = "bootstrap-self-test")] + pub(crate) fn maybe_download_rustfmt(&self) -> Option { + 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 { + 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 @@ pub(crate) fn download_ci_rustc(&self, commit: &str) { ); } + #[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 @@ fn download_component( 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 @@ pub(crate) fn maybe_download_ci_llvm(&self) { } } + #[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 index 8ffa97ab78b..ead38ebc6d5 100644 --- 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 @@ pub fn check(build: &mut Build) { .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 = output(Command::new(&build.config.initial_rustc).args(["--print", "target-list"])) .lines() @@ -193,11 +197,11 @@ pub fn check(build: &mut Build) { 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(); @@ -226,7 +230,7 @@ pub fn check(build: &mut Build) { 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; }