Skip to content

Commit

Permalink
bootstrap: implement new feature bootstrap-self-test
Browse files Browse the repository at this point in the history
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 <work@onurozkan.dev>
  • Loading branch information
onur-ozkan committed May 19, 2024
1 parent 7c73595 commit 494cbd8
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 29 deletions.
1 change: 1 addition & 0 deletions src/bootstrap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
32 changes: 21 additions & 11 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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))
}
Expand All @@ -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;
}
Expand All @@ -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
Expand Down
11 changes: 1 addition & 10 deletions src/bootstrap/src/core/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
Expand Down Expand Up @@ -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(),
],
Expand All @@ -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,
);
Expand Down
22 changes: 20 additions & 2 deletions src/bootstrap/src/core/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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}");

Expand Down Expand Up @@ -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"));

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand Down
16 changes: 10 additions & 6 deletions src/bootstrap/src/core/sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
];
Expand Down Expand Up @@ -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()
Expand All @@ -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();
Expand All @@ -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;
}
Expand Down

0 comments on commit 494cbd8

Please sign in to comment.