Skip to content

Commit 43972b6

Browse files
onur-ozkanrezwanahmedsami
authored andcommitted
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 <[email protected]>
1 parent c18434a commit 43972b6

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]
@@ -1593,24 +1593,6 @@ impl Config {
15931593
let mut is_user_configured_rust_channel = false;
15941594

15951595
if let Some(rust) = toml.rust {
1596-
if let Some(commit) = config.download_ci_rustc_commit(rust.download_rustc.clone()) {
1597-
// Primarily used by CI runners to avoid handling download-rustc incompatible
1598-
// options one by one on shell scripts.
1599-
let disable_ci_rustc_if_incompatible =
1600-
env::var_os("DISABLE_CI_RUSTC_IF_INCOMPATIBLE")
1601-
.is_some_and(|s| s == "1" || s == "true");
1602-
1603-
if let Err(e) = check_incompatible_options_for_ci_rustc(&rust) {
1604-
if disable_ci_rustc_if_incompatible {
1605-
config.download_rustc_commit = None;
1606-
} else {
1607-
panic!("{}", e);
1608-
}
1609-
} else {
1610-
config.download_rustc_commit = Some(commit);
1611-
}
1612-
}
1613-
16141596
let Rust {
16151597
optimize: optimize_toml,
16161598
debug: debug_toml,
@@ -1658,7 +1640,7 @@ impl Config {
16581640
new_symbol_mangling,
16591641
profile_generate,
16601642
profile_use,
1661-
download_rustc: _,
1643+
download_rustc,
16621644
lto,
16631645
validate_mir_opts,
16641646
frame_pointers,
@@ -1670,6 +1652,8 @@ impl Config {
16701652
is_user_configured_rust_channel = channel.is_some();
16711653
set(&mut config.channel, channel.clone());
16721654

1655+
config.download_rustc_commit = config.download_ci_rustc_commit(download_rustc);
1656+
16731657
debug = debug_toml;
16741658
debug_assertions = debug_assertions_toml;
16751659
debug_assertions_std = debug_assertions_std_toml;
@@ -2347,6 +2331,29 @@ impl Config {
23472331
None => None,
23482332
Some(commit) => {
23492333
self.download_ci_rustc(commit);
2334+
2335+
let builder_config_path =
2336+
self.out.join(self.build.triple).join("ci-rustc/builder-config");
2337+
let ci_config_toml = Self::get_toml(&builder_config_path);
2338+
let current_config_toml = Self::get_toml(self.config.as_ref().unwrap());
2339+
2340+
// Check the config compatibility
2341+
let res = check_incompatible_options_for_ci_rustc(
2342+
current_config_toml,
2343+
ci_config_toml,
2344+
);
2345+
2346+
let disable_ci_rustc_if_incompatible =
2347+
env::var_os("DISABLE_CI_RUSTC_IF_INCOMPATIBLE")
2348+
.is_some_and(|s| s == "1" || s == "true");
2349+
2350+
if disable_ci_rustc_if_incompatible && res.is_err() {
2351+
println!("WARNING: download-rustc is disabled with `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` env.");
2352+
return None;
2353+
}
2354+
2355+
res.unwrap();
2356+
23502357
Some(commit.clone())
23512358
}
23522359
})
@@ -2664,31 +2671,44 @@ impl Config {
26642671
}
26652672
}
26662673

2667-
/// Checks the CI rustc incompatible options by destructuring the `Rust` instance
2668-
/// and makes sure that no rust options from config.toml are missed.
2669-
fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> {
2674+
/// Compares the current Rust options against those in the CI rustc builder and detects any incompatible options.
2675+
/// It does this by destructuring the `Rust` instance to make sure every `Rust` field is covered and not missing.
2676+
fn check_incompatible_options_for_ci_rustc(
2677+
current_config_toml: TomlConfig,
2678+
ci_config_toml: TomlConfig,
2679+
) -> Result<(), String> {
26702680
macro_rules! err {
2671-
($name:expr) => {
2672-
if $name.is_some() {
2681+
($current:expr, $expected:expr) => {
2682+
if let Some(current) = $current {
2683+
if Some(current) != $expected {
26732684
return Err(format!(
26742685
"ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`.",
2675-
stringify!($name).replace("_", "-")
2686+
stringify!($expected).replace("_", "-")
26762687
));
2677-
}
2688+
};
2689+
};
26782690
};
26792691
}
26802692

26812693
macro_rules! warn {
2682-
($name:expr) => {
2683-
if $name.is_some() {
2694+
($current:expr, $expected:expr) => {
2695+
if let Some(current) = $current {
2696+
if Some(current) != $expected {
26842697
println!(
26852698
"WARNING: `rust.{}` has no effect with `rust.download-rustc`.",
2686-
stringify!($name).replace("_", "-")
2699+
stringify!($expected).replace("_", "-")
26872700
);
2688-
}
2701+
};
2702+
};
26892703
};
26902704
}
26912705

2706+
let (Some(current_rust_config), Some(ci_rust_config)) =
2707+
(current_config_toml.rust, ci_config_toml.rust)
2708+
else {
2709+
return Ok(());
2710+
};
2711+
26922712
let Rust {
26932713
// Following options are the CI rustc incompatible ones.
26942714
optimize,
@@ -2746,30 +2766,31 @@ fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> {
27462766
download_rustc: _,
27472767
validate_mir_opts: _,
27482768
frame_pointers: _,
2749-
} = rust;
2769+
} = ci_rust_config;
27502770

27512771
// There are two kinds of checks for CI rustc incompatible options:
27522772
// 1. Checking an option that may change the compiler behaviour/output.
27532773
// 2. Checking an option that have no effect on the compiler behaviour/output.
27542774
//
27552775
// If the option belongs to the first category, we call `err` macro for a hard error;
27562776
// otherwise, we just print a warning with `warn` macro.
2757-
err!(optimize);
2758-
err!(debug_logging);
2759-
err!(debuginfo_level_rustc);
2760-
err!(default_linker);
2761-
err!(rpath);
2762-
err!(strip);
2763-
err!(stack_protector);
2764-
err!(lld_mode);
2765-
err!(llvm_tools);
2766-
err!(llvm_bitcode_linker);
2767-
err!(jemalloc);
2768-
err!(lto);
2769-
2770-
warn!(channel);
2771-
warn!(description);
2772-
warn!(incremental);
2777+
2778+
err!(current_rust_config.optimize, optimize);
2779+
err!(current_rust_config.debug_logging, debug_logging);
2780+
err!(current_rust_config.debuginfo_level_rustc, debuginfo_level_rustc);
2781+
err!(current_rust_config.rpath, rpath);
2782+
err!(current_rust_config.strip, strip);
2783+
err!(current_rust_config.lld_mode, lld_mode);
2784+
err!(current_rust_config.llvm_tools, llvm_tools);
2785+
err!(current_rust_config.llvm_bitcode_linker, llvm_bitcode_linker);
2786+
err!(current_rust_config.jemalloc, jemalloc);
2787+
err!(current_rust_config.default_linker, default_linker);
2788+
err!(current_rust_config.stack_protector, stack_protector);
2789+
err!(current_rust_config.lto, lto);
2790+
2791+
warn!(current_rust_config.channel, channel);
2792+
warn!(current_rust_config.description, description);
2793+
warn!(current_rust_config.incremental, incremental);
27732794

27742795
Ok(())
27752796
}

0 commit comments

Comments
 (0)