Skip to content

Commit 0518e8c

Browse files
committed
detect incompatible CI rustc 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 rustc. Now, the new logic compares the configuration values with the values used to generate the CI rustc, so we get more precise results and make the process more manageable. Signed-off-by: onur-ozkan <work@onurozkan.dev>
1 parent b2f1fc1 commit 0518e8c

File tree

1 file changed

+69
-48
lines changed

1 file changed

+69
-48
lines changed

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

Lines changed: 69 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ impl Display for DebuginfoLevel {
117117
/// 2) MSVC
118118
/// - Self-contained: `-Clinker=<path to rust-lld>`
119119
/// - External: `-Clinker=lld`
120-
#[derive(Default, Copy, Clone)]
120+
#[derive(Copy, Clone, Default, PartialEq)]
121121
pub enum LldMode {
122122
/// Do not use LLD
123123
#[default]
@@ -1581,24 +1581,6 @@ impl Config {
15811581
let mut is_user_configured_rust_channel = false;
15821582

15831583
if let Some(rust) = toml.rust {
1584-
if let Some(commit) = config.download_ci_rustc_commit(rust.download_rustc.clone()) {
1585-
// Primarily used by CI runners to avoid handling download-rustc incompatible
1586-
// options one by one on shell scripts.
1587-
let disable_ci_rustc_if_incompatible =
1588-
env::var_os("DISABLE_CI_RUSTC_IF_INCOMPATIBLE")
1589-
.is_some_and(|s| s == "1" || s == "true");
1590-
1591-
if let Err(e) = check_incompatible_options_for_ci_rustc(&rust) {
1592-
if disable_ci_rustc_if_incompatible {
1593-
config.download_rustc_commit = None;
1594-
} else {
1595-
panic!("{}", e);
1596-
}
1597-
} else {
1598-
config.download_rustc_commit = Some(commit);
1599-
}
1600-
}
1601-
16021584
let Rust {
16031585
optimize: optimize_toml,
16041586
debug: debug_toml,
@@ -1646,7 +1628,7 @@ impl Config {
16461628
new_symbol_mangling,
16471629
profile_generate,
16481630
profile_use,
1649-
download_rustc: _,
1631+
download_rustc,
16501632
lto,
16511633
validate_mir_opts,
16521634
frame_pointers,
@@ -1658,6 +1640,8 @@ impl Config {
16581640
is_user_configured_rust_channel = channel.is_some();
16591641
set(&mut config.channel, channel.clone());
16601642

1643+
config.download_rustc_commit = config.download_ci_rustc_commit(download_rustc);
1644+
16611645
debug = debug_toml;
16621646
debug_assertions = debug_assertions_toml;
16631647
debug_assertions_std = debug_assertions_std_toml;
@@ -2335,6 +2319,29 @@ impl Config {
23352319
None => None,
23362320
Some(commit) => {
23372321
self.download_ci_rustc(commit);
2322+
2323+
let builder_config_path =
2324+
self.out.join(self.build.triple).join("ci-rustc/builder-config");
2325+
let ci_config_toml = Self::get_toml(&builder_config_path);
2326+
let current_config_toml = Self::get_toml(self.config.as_ref().unwrap());
2327+
2328+
// Check the config compatibility
2329+
let res = check_incompatible_options_for_ci_rustc(
2330+
current_config_toml,
2331+
ci_config_toml,
2332+
);
2333+
2334+
let disable_ci_rustc_if_incompatible =
2335+
env::var_os("DISABLE_CI_RUSTC_IF_INCOMPATIBLE")
2336+
.is_some_and(|s| s == "1" || s == "true");
2337+
2338+
if disable_ci_rustc_if_incompatible && res.is_err() {
2339+
println!("WARNING: download-rustc is disabled with `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` env.");
2340+
return None;
2341+
}
2342+
2343+
res.unwrap();
2344+
23382345
Some(commit.clone())
23392346
}
23402347
})
@@ -2652,31 +2659,44 @@ impl Config {
26522659
}
26532660
}
26542661

2655-
/// Checks the CI rustc incompatible options by destructuring the `Rust` instance
2656-
/// and makes sure that no rust options from config.toml are missed.
2657-
fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> {
2662+
/// Compares the current Rust options against those in the CI rustc builder and detects any incompatible options.
2663+
/// It does this by destructuring the `Rust` instance to make sure every `Rust` field is covered and not missing.
2664+
fn check_incompatible_options_for_ci_rustc(
2665+
current_config_toml: TomlConfig,
2666+
ci_config_toml: TomlConfig,
2667+
) -> Result<(), String> {
26582668
macro_rules! err {
2659-
($name:expr) => {
2660-
if $name.is_some() {
2669+
($current:expr, $expected:expr) => {
2670+
if let Some(current) = $current {
2671+
if Some(current) != $expected {
26612672
return Err(format!(
26622673
"ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`.",
2663-
stringify!($name).replace("_", "-")
2674+
stringify!($expected).replace("_", "-")
26642675
));
2665-
}
2676+
};
2677+
};
26662678
};
26672679
}
26682680

26692681
macro_rules! warn {
2670-
($name:expr) => {
2671-
if $name.is_some() {
2682+
($current:expr, $expected:expr) => {
2683+
if let Some(current) = $current {
2684+
if Some(current) != $expected {
26722685
println!(
26732686
"WARNING: `rust.{}` has no effect with `rust.download-rustc`.",
2674-
stringify!($name).replace("_", "-")
2687+
stringify!($expected).replace("_", "-")
26752688
);
2676-
}
2689+
};
2690+
};
26772691
};
26782692
}
26792693

2694+
let (Some(current_rust_config), Some(ci_rust_config)) =
2695+
(current_config_toml.rust, ci_config_toml.rust)
2696+
else {
2697+
return Ok(());
2698+
};
2699+
26802700
let Rust {
26812701
// Following options are the CI rustc incompatible ones.
26822702
optimize,
@@ -2734,30 +2754,31 @@ fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> {
27342754
download_rustc: _,
27352755
validate_mir_opts: _,
27362756
frame_pointers: _,
2737-
} = rust;
2757+
} = ci_rust_config;
27382758

27392759
// There are two kinds of checks for CI rustc incompatible options:
27402760
// 1. Checking an option that may change the compiler behaviour/output.
27412761
// 2. Checking an option that have no effect on the compiler behaviour/output.
27422762
//
27432763
// If the option belongs to the first category, we call `err` macro for a hard error;
27442764
// otherwise, we just print a warning with `warn` macro.
2745-
err!(optimize);
2746-
err!(debug_logging);
2747-
err!(debuginfo_level_rustc);
2748-
err!(default_linker);
2749-
err!(rpath);
2750-
err!(strip);
2751-
err!(stack_protector);
2752-
err!(lld_mode);
2753-
err!(llvm_tools);
2754-
err!(llvm_bitcode_linker);
2755-
err!(jemalloc);
2756-
err!(lto);
2757-
2758-
warn!(channel);
2759-
warn!(description);
2760-
warn!(incremental);
2765+
2766+
err!(current_rust_config.optimize, optimize);
2767+
err!(current_rust_config.debug_logging, debug_logging);
2768+
err!(current_rust_config.debuginfo_level_rustc, debuginfo_level_rustc);
2769+
err!(current_rust_config.rpath, rpath);
2770+
err!(current_rust_config.strip, strip);
2771+
err!(current_rust_config.lld_mode, lld_mode);
2772+
err!(current_rust_config.llvm_tools, llvm_tools);
2773+
err!(current_rust_config.llvm_bitcode_linker, llvm_bitcode_linker);
2774+
err!(current_rust_config.jemalloc, jemalloc);
2775+
err!(current_rust_config.default_linker, default_linker);
2776+
err!(current_rust_config.stack_protector, stack_protector);
2777+
err!(current_rust_config.lto, lto);
2778+
2779+
warn!(current_rust_config.channel, channel);
2780+
warn!(current_rust_config.description, description);
2781+
warn!(current_rust_config.incremental, incremental);
27612782

27622783
Ok(())
27632784
}

0 commit comments

Comments
 (0)