Skip to content

Commit b980705

Browse files
committed
detect incompatible CI LLVM options more precisely
Previously, the logic here was simply checking whether the option was set in `config.toml`. This approach was not manageable in our CI runners as we set so many options in config.toml. In reality, those values are not incompatible since they are usually the same value used to generate the CI llvm. Now, the new logic compares the configuration values with the values used to generate the CI llvm, so we get more precise results and make the process more manageable. Signed-off-by: onur-ozkan <[email protected]>
1 parent 0d63418 commit b980705

File tree

2 files changed

+138
-3
lines changed

2 files changed

+138
-3
lines changed

src/bootstrap/src/core/config/config.rs

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,13 +1211,19 @@ impl Config {
12111211
}
12121212
}
12131213

1214+
pub(crate) fn get_builder_toml(&self, build_name: &str) -> Result<TomlConfig, toml::de::Error> {
1215+
let builder_config_path =
1216+
self.out.join(self.build.triple).join(build_name).join(BUILDER_CONFIG_FILENAME);
1217+
Self::get_toml(&builder_config_path)
1218+
}
1219+
12141220
#[cfg(test)]
1215-
fn get_toml(_: &Path) -> Result<TomlConfig, toml::de::Error> {
1221+
pub(crate) fn get_toml(_: &Path) -> Result<TomlConfig, toml::de::Error> {
12161222
Ok(TomlConfig::default())
12171223
}
12181224

12191225
#[cfg(not(test))]
1220-
fn get_toml(file: &Path) -> Result<TomlConfig, toml::de::Error> {
1226+
pub(crate) fn get_toml(file: &Path) -> Result<TomlConfig, toml::de::Error> {
12211227
let contents =
12221228
t!(fs::read_to_string(file), format!("config file {} not found", file.display()));
12231229
// Deserialize to Value and then TomlConfig to prevent the Deserialize impl of
@@ -2840,6 +2846,106 @@ impl Config {
28402846
}
28412847
}
28422848

2849+
/// Compares the current `Llvm` options against those in the CI LLVM builder and detects any incompatible options.
2850+
/// It does this by destructuring the `Llvm` instance to make sure every `Llvm` field is covered and not missing.
2851+
pub(crate) fn check_incompatible_options_for_ci_llvm(
2852+
current_config_toml: TomlConfig,
2853+
ci_config_toml: TomlConfig,
2854+
) -> Result<(), String> {
2855+
macro_rules! err {
2856+
($current:expr, $expected:expr) => {
2857+
if let Some(current) = &$current {
2858+
if Some(current) != $expected.as_ref() {
2859+
return Err(format!(
2860+
"ERROR: Setting `llvm.{}` is incompatible with `llvm.download-ci-llvm`. \
2861+
Current value: {:?}, Expected value(s): {}{:?}",
2862+
stringify!($expected).replace("_", "-"),
2863+
$current,
2864+
if $expected.is_some() { "None/" } else { "" },
2865+
$expected,
2866+
));
2867+
};
2868+
};
2869+
};
2870+
}
2871+
2872+
macro_rules! warn {
2873+
($current:expr, $expected:expr) => {
2874+
if let Some(current) = &$current {
2875+
if Some(current) != $expected.as_ref() {
2876+
println!(
2877+
"WARNING: `llvm.{}` has no effect with `llvm.download-ci-llvm`. \
2878+
Current value: {:?}, Expected value(s): {}{:?}",
2879+
stringify!($expected).replace("_", "-"),
2880+
$current,
2881+
if $expected.is_some() { "None/" } else { "" },
2882+
$expected,
2883+
);
2884+
};
2885+
};
2886+
};
2887+
}
2888+
2889+
let (Some(current_llvm_config), Some(ci_llvm_config)) =
2890+
(current_config_toml.llvm, ci_config_toml.llvm)
2891+
else {
2892+
return Ok(());
2893+
};
2894+
2895+
let Llvm {
2896+
optimize,
2897+
thin_lto,
2898+
release_debuginfo,
2899+
assertions: _,
2900+
tests: _,
2901+
plugins,
2902+
ccache: _,
2903+
static_libstdcpp: _,
2904+
libzstd,
2905+
ninja: _,
2906+
targets,
2907+
experimental_targets,
2908+
link_jobs: _,
2909+
link_shared: _,
2910+
version_suffix,
2911+
clang_cl,
2912+
cflags,
2913+
cxxflags,
2914+
ldflags,
2915+
use_libcxx,
2916+
use_linker,
2917+
allow_old_toolchain,
2918+
polly,
2919+
clang,
2920+
enable_warnings,
2921+
download_ci_llvm: _,
2922+
build_config,
2923+
} = ci_llvm_config;
2924+
2925+
err!(current_llvm_config.optimize, optimize);
2926+
err!(current_llvm_config.thin_lto, thin_lto);
2927+
err!(current_llvm_config.release_debuginfo, release_debuginfo);
2928+
err!(current_llvm_config.libzstd, libzstd);
2929+
err!(current_llvm_config.targets, targets);
2930+
err!(current_llvm_config.experimental_targets, experimental_targets);
2931+
err!(current_llvm_config.clang_cl, clang_cl);
2932+
err!(current_llvm_config.version_suffix, version_suffix);
2933+
err!(current_llvm_config.cflags, cflags);
2934+
err!(current_llvm_config.cxxflags, cxxflags);
2935+
err!(current_llvm_config.ldflags, ldflags);
2936+
err!(current_llvm_config.use_libcxx, use_libcxx);
2937+
err!(current_llvm_config.use_linker, use_linker);
2938+
err!(current_llvm_config.allow_old_toolchain, allow_old_toolchain);
2939+
err!(current_llvm_config.polly, polly);
2940+
err!(current_llvm_config.clang, clang);
2941+
err!(current_llvm_config.build_config, build_config);
2942+
err!(current_llvm_config.plugins, plugins);
2943+
2944+
warn!(current_llvm_config.enable_warnings, enable_warnings);
2945+
2946+
Ok(())
2947+
}
2948+
28432949
/// Compares the current Rust options against those in the CI rustc builder and detects any incompatible options.
28442950
/// It does this by destructuring the `Rust` instance to make sure every `Rust` field is covered and not missing.
28452951
fn check_incompatible_options_for_ci_rustc(

src/bootstrap/src/core/download.rs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ impl Config {
299299
let mut tar = tar::Archive::new(decompressor);
300300

301301
let is_ci_rustc = dst.ends_with("ci-rustc");
302+
let is_ci_llvm = dst.ends_with("ci-llvm");
302303

303304
// `compile::Sysroot` needs to know the contents of the `rustc-dev` tarball to avoid adding
304305
// it to the sysroot unless it was explicitly requested. But parsing the 100 MB tarball is slow.
@@ -315,7 +316,9 @@ impl Config {
315316
let mut short_path = t!(original_path.strip_prefix(directory_prefix));
316317
let is_builder_config = short_path.to_str() == Some(BUILDER_CONFIG_FILENAME);
317318

318-
if !(short_path.starts_with(pattern) || (is_ci_rustc && is_builder_config)) {
319+
if !(short_path.starts_with(pattern)
320+
|| ((is_ci_rustc || is_ci_llvm) && is_builder_config))
321+
{
319322
continue;
320323
}
321324
short_path = short_path.strip_prefix(pattern).unwrap_or(short_path);
@@ -700,17 +703,43 @@ download-rustc = false
700703

701704
#[cfg(not(feature = "bootstrap-self-test"))]
702705
pub(crate) fn maybe_download_ci_llvm(&self) {
706+
use build_helper::exit;
707+
703708
use crate::core::build_steps::llvm::detect_llvm_sha;
709+
use crate::core::config::check_incompatible_options_for_ci_llvm;
704710

705711
if !self.llvm_from_ci {
706712
return;
707713
}
714+
708715
let llvm_root = self.ci_llvm_root();
709716
let llvm_stamp = llvm_root.join(".llvm-stamp");
710717
let llvm_sha = detect_llvm_sha(self, self.rust_info.is_managed_git_subrepository());
711718
let key = format!("{}{}", llvm_sha, self.llvm_assertions);
712719
if program_out_of_date(&llvm_stamp, &key) && !self.dry_run() {
713720
self.download_ci_llvm(&llvm_sha);
721+
722+
if let Some(config_path) = &self.config {
723+
let current_config_toml = Self::get_toml(config_path).unwrap();
724+
725+
match self.get_builder_toml("ci-llvm") {
726+
Ok(ci_config_toml) => {
727+
check_incompatible_options_for_ci_llvm(current_config_toml, ci_config_toml)
728+
.unwrap();
729+
}
730+
Err(e) if e.to_string().contains("unknown field") => {
731+
println!(
732+
"WARNING: CI rustc has some fields that are no longer supported in bootstrap; download-rustc will be disabled."
733+
);
734+
println!("HELP: Consider rebasing to a newer commit if available.");
735+
}
736+
Err(e) => {
737+
eprintln!("ERROR: Failed to parse CI LLVM config.toml: {e}");
738+
exit!(2);
739+
}
740+
};
741+
};
742+
714743
if self.should_fix_bins_and_dylibs() {
715744
for entry in t!(fs::read_dir(llvm_root.join("bin"))) {
716745
self.fix_bin_or_dylib(&t!(entry).path());

0 commit comments

Comments
 (0)