detect incompatible CI rustc options more precisely

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

@@ -36,6 +36,14 @@ macro_rules! check_ci_llvm {
     };
 }
 
+/// This file is embedded in the overlay directory of the tarball sources. It is
+/// useful in scenarios where developers want to see how the tarball sources were
+/// generated.
+///
+/// We also use this file to compare the host's config.toml against the CI rustc builder
+/// configuration to detect any incompatible options.
+pub(crate) const BUILDER_CONFIG_FILENAME: &str = "builder-config";
+
 #[derive(Clone, Default)]
 pub enum DryRun {
     /// This isn't a dry run.
@@ -47,7 +55,7 @@ pub enum DryRun {
     UserSelected,
 }
 
-#[derive(Copy, Clone, Default, PartialEq, Eq)]
+#[derive(Copy, Clone, Default, Debug, Eq, PartialEq)]
 pub enum DebuginfoLevel {
     #[default]
     None,
@@ -117,7 +125,7 @@ impl Display for DebuginfoLevel {
 /// 2) MSVC
 /// - Self-contained: `-Clinker=<path to rust-lld>`
 /// - External: `-Clinker=lld`
-#[derive(Default, Copy, Clone)]
+#[derive(Copy, Clone, Default, Debug, PartialEq)]
 pub enum LldMode {
     /// Do not use LLD
     #[default]
@@ -1191,40 +1199,42 @@ impl Config {
         }
     }
 
-    pub fn parse(flags: Flags) -> Config {
-        #[cfg(test)]
-        fn get_toml(_: &Path) -> TomlConfig {
-            TomlConfig::default()
-        }
+    #[cfg(test)]
+    fn get_toml(_: &Path) -> Result<TomlConfig, toml::de::Error> {
+        Ok(TomlConfig::default())
+    }
 
-        #[cfg(not(test))]
-        fn get_toml(file: &Path) -> TomlConfig {
-            let contents =
-                t!(fs::read_to_string(file), format!("config file {} not found", file.display()));
-            // Deserialize to Value and then TomlConfig to prevent the Deserialize impl of
-            // TomlConfig and sub types to be monomorphized 5x by toml.
-            toml::from_str(&contents)
-                .and_then(|table: toml::Value| TomlConfig::deserialize(table))
-                .unwrap_or_else(|err| {
-                    if let Ok(Some(changes)) = toml::from_str(&contents)
-                        .and_then(|table: toml::Value| ChangeIdWrapper::deserialize(table)).map(|change_id| change_id.inner.map(crate::find_recent_config_change_ids))
-                    {
-                        if !changes.is_empty() {
-                            println!(
-                                "WARNING: There have been changes to x.py since you last updated:\n{}",
-                                crate::human_readable_changes(&changes)
-                            );
-                        }
+    #[cfg(not(test))]
+    fn get_toml(file: &Path) -> Result<TomlConfig, toml::de::Error> {
+        let contents =
+            t!(fs::read_to_string(file), format!("config file {} not found", file.display()));
+        // Deserialize to Value and then TomlConfig to prevent the Deserialize impl of
+        // TomlConfig and sub types to be monomorphized 5x by toml.
+        toml::from_str(&contents)
+            .and_then(|table: toml::Value| TomlConfig::deserialize(table))
+            .inspect_err(|_| {
+                if let Ok(Some(changes)) = toml::from_str(&contents)
+                    .and_then(|table: toml::Value| ChangeIdWrapper::deserialize(table))
+                    .map(|change_id| change_id.inner.map(crate::find_recent_config_change_ids))
+                {
+                    if !changes.is_empty() {
+                        println!(
+                            "WARNING: There have been changes to x.py since you last updated:\n{}",
+                            crate::human_readable_changes(&changes)
+                        );
                     }
+                }
+            })
+    }
 
-                    eprintln!("failed to parse TOML configuration '{}': {err}", file.display());
-                    exit!(2);
-                })
-        }
-        Self::parse_inner(flags, get_toml)
+    pub fn parse(flags: Flags) -> Config {
+        Self::parse_inner(flags, Self::get_toml)
     }
 
-    pub(crate) fn parse_inner(mut flags: Flags, get_toml: impl Fn(&Path) -> TomlConfig) -> Config {
+    pub(crate) fn parse_inner(
+        mut flags: Flags,
+        get_toml: impl Fn(&Path) -> Result<TomlConfig, toml::de::Error>,
+    ) -> Config {
         let mut config = Config::default_opts();
 
         // Set flags.
@@ -1332,7 +1342,10 @@ impl Config {
             } else {
                 toml_path.clone()
             });
-            get_toml(&toml_path)
+            get_toml(&toml_path).unwrap_or_else(|e| {
+                eprintln!("ERROR: Failed to parse '{}': {e}", toml_path.display());
+                exit!(2);
+            })
         } else {
             config.config = None;
             TomlConfig::default()
@@ -1363,7 +1376,13 @@ impl Config {
             include_path.push("bootstrap");
             include_path.push("defaults");
             include_path.push(format!("config.{include}.toml"));
-            let included_toml = get_toml(&include_path);
+            let included_toml = get_toml(&include_path).unwrap_or_else(|e| {
+                eprintln!(
+                    "ERROR: Failed to parse default config profile at '{}': {e}",
+                    include_path.display()
+                );
+                exit!(2);
+            });
             toml.merge(included_toml, ReplaceOpt::IgnoreDuplicate);
         }
 
@@ -1579,24 +1598,6 @@ impl Config {
         let mut is_user_configured_rust_channel = false;
 
         if let Some(rust) = toml.rust {
-            if let Some(commit) = config.download_ci_rustc_commit(rust.download_rustc.clone()) {
-                // Primarily used by CI runners to avoid handling download-rustc incompatible
-                // options one by one on shell scripts.
-                let disable_ci_rustc_if_incompatible =
-                    env::var_os("DISABLE_CI_RUSTC_IF_INCOMPATIBLE")
-                        .is_some_and(|s| s == "1" || s == "true");
-
-                if let Err(e) = check_incompatible_options_for_ci_rustc(&rust) {
-                    if disable_ci_rustc_if_incompatible {
-                        config.download_rustc_commit = None;
-                    } else {
-                        panic!("{}", e);
-                    }
-                } else {
-                    config.download_rustc_commit = Some(commit);
-                }
-            }
-
             let Rust {
                 optimize: optimize_toml,
                 debug: debug_toml,
@@ -1644,7 +1645,7 @@ impl Config {
                 new_symbol_mangling,
                 profile_generate,
                 profile_use,
-                download_rustc: _,
+                download_rustc,
                 lto,
                 validate_mir_opts,
                 frame_pointers,
@@ -1656,6 +1657,8 @@ impl Config {
             is_user_configured_rust_channel = channel.is_some();
             set(&mut config.channel, channel.clone());
 
+            config.download_rustc_commit = config.download_ci_rustc_commit(download_rustc);
+
             debug = debug_toml;
             debug_assertions = debug_assertions_toml;
             debug_assertions_std = debug_assertions_std_toml;
@@ -2333,6 +2336,45 @@ impl Config {
                 None => None,
                 Some(commit) => {
                     self.download_ci_rustc(commit);
+
+                    if let Some(config_path) = &self.config {
+                        let builder_config_path =
+                            self.out.join(self.build.triple).join("ci-rustc").join(BUILDER_CONFIG_FILENAME);
+
+                        let ci_config_toml = match Self::get_toml(&builder_config_path) {
+                            Ok(ci_config_toml) => ci_config_toml,
+                            Err(e) if e.to_string().contains("unknown field") => {
+                                println!("WARNING: CI rustc has some fields that are no longer supported in bootstrap; download-rustc will be disabled.");
+                                println!("HELP: Consider rebasing to a newer commit if available.");
+                                return None;
+                            },
+                            Err(e) => {
+                                eprintln!("ERROR: Failed to parse CI rustc config at '{}': {e}", builder_config_path.display());
+                                exit!(2);
+                            },
+                        };
+
+                        let current_config_toml = Self::get_toml(config_path).unwrap();
+
+                        // Check the config compatibility
+                        // FIXME: this doesn't cover `--set` flags yet.
+                        let res = check_incompatible_options_for_ci_rustc(
+                            current_config_toml,
+                            ci_config_toml,
+                        );
+
+                        let disable_ci_rustc_if_incompatible =
+                            env::var_os("DISABLE_CI_RUSTC_IF_INCOMPATIBLE")
+                            .is_some_and(|s| s == "1" || s == "true");
+
+                        if disable_ci_rustc_if_incompatible && res.is_err() {
+                            println!("WARNING: download-rustc is disabled with `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` env.");
+                            return None;
+                        }
+
+                        res.unwrap();
+                    }
+
                     Some(commit.clone())
                 }
             })
@@ -2650,31 +2692,52 @@ impl Config {
     }
 }
 
-/// Checks the CI rustc incompatible options by destructuring the `Rust` instance
-/// and makes sure that no rust options from config.toml are missed.
-fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> {
+/// Compares the current Rust options against those in the CI rustc builder and detects any incompatible options.
+/// It does this by destructuring the `Rust` instance to make sure every `Rust` field is covered and not missing.
+fn check_incompatible_options_for_ci_rustc(
+    current_config_toml: TomlConfig,
+    ci_config_toml: TomlConfig,
+) -> Result<(), String> {
     macro_rules! err {
-        ($name:expr) => {
-            if $name.is_some() {
-                return Err(format!(
-                    "ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`.",
-                    stringify!($name).replace("_", "-")
-                ));
-            }
+        ($current:expr, $expected:expr) => {
+            if let Some(current) = &$current {
+                if Some(current) != $expected.as_ref() {
+                    return Err(format!(
+                        "ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`. \
+                        Current value: {:?}, Expected value(s): {}{:?}",
+                        stringify!($expected).replace("_", "-"),
+                        $current,
+                        if $expected.is_some() { "None/" } else { "" },
+                        $expected,
+                    ));
+                };
+            };
         };
     }
 
     macro_rules! warn {
-        ($name:expr) => {
-            if $name.is_some() {
-                println!(
-                    "WARNING: `rust.{}` has no effect with `rust.download-rustc`.",
-                    stringify!($name).replace("_", "-")
-                );
-            }
+        ($current:expr, $expected:expr) => {
+            if let Some(current) = &$current {
+                if Some(current) != $expected.as_ref() {
+                    println!(
+                        "WARNING: `rust.{}` has no effect with `rust.download-rustc`. \
+                        Current value: {:?}, Expected value(s): {}{:?}",
+                        stringify!($expected).replace("_", "-"),
+                        $current,
+                        if $expected.is_some() { "None/" } else { "" },
+                        $expected,
+                    );
+                };
+            };
         };
     }
 
+    let (Some(current_rust_config), Some(ci_rust_config)) =
+        (current_config_toml.rust, ci_config_toml.rust)
+    else {
+        return Ok(());
+    };
+
     let Rust {
         // Following options are the CI rustc incompatible ones.
         optimize,
@@ -2732,30 +2795,31 @@ fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> {
         download_rustc: _,
         validate_mir_opts: _,
         frame_pointers: _,
-    } = rust;
+    } = ci_rust_config;
 
     // There are two kinds of checks for CI rustc incompatible options:
     //    1. Checking an option that may change the compiler behaviour/output.
     //    2. Checking an option that have no effect on the compiler behaviour/output.
     //
     // If the option belongs to the first category, we call `err` macro for a hard error;
     // otherwise, we just print a warning with `warn` macro.
-    err!(optimize);
-    err!(debug_logging);
-    err!(debuginfo_level_rustc);
-    err!(default_linker);
-    err!(rpath);
-    err!(strip);
-    err!(stack_protector);
-    err!(lld_mode);
-    err!(llvm_tools);
-    err!(llvm_bitcode_linker);
-    err!(jemalloc);
-    err!(lto);
-
-    warn!(channel);
-    warn!(description);
-    warn!(incremental);
+
+    err!(current_rust_config.optimize, optimize);
+    err!(current_rust_config.debug_logging, debug_logging);
+    err!(current_rust_config.debuginfo_level_rustc, debuginfo_level_rustc);
+    err!(current_rust_config.rpath, rpath);
+    err!(current_rust_config.strip, strip);
+    err!(current_rust_config.lld_mode, lld_mode);
+    err!(current_rust_config.llvm_tools, llvm_tools);
+    err!(current_rust_config.llvm_bitcode_linker, llvm_bitcode_linker);
+    err!(current_rust_config.jemalloc, jemalloc);
+    err!(current_rust_config.default_linker, default_linker);
+    err!(current_rust_config.stack_protector, stack_protector);
+    err!(current_rust_config.lto, lto);
+
+    warn!(current_rust_config.channel, channel);
+    warn!(current_rust_config.description, description);
+    warn!(current_rust_config.incremental, incremental);
 
     Ok(())
 }

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

@@ -14,7 +14,7 @@ use crate::core::config::{LldMode, Target, TargetSelection, TomlConfig};
 fn parse(config: &str) -> Config {
     Config::parse_inner(
         Flags::parse(&["check".to_string(), "--config=/does/not/exist".to_string()]),
-        |&_| toml::from_str(&config).unwrap(),
+        |&_| toml::from_str(&config),
     )
 }
 
@@ -151,7 +151,6 @@ runner = "x86_64-runner"
 
                 "#,
             )
-            .unwrap()
         },
     );
     assert_eq!(config.change_id, Some(1), "setting top-level value");
@@ -208,13 +207,13 @@ fn override_toml_duplicate() {
             "--set=change-id=1".to_owned(),
             "--set=change-id=2".to_owned(),
         ]),
-        |&_| toml::from_str("change-id = 0").unwrap(),
+        |&_| toml::from_str("change-id = 0"),
     );
 }
 
 #[test]
 fn profile_user_dist() {
-    fn get_toml(file: &Path) -> TomlConfig {
+    fn get_toml(file: &Path) -> Result<TomlConfig, toml::de::Error> {
         let contents =
             if file.ends_with("config.toml") || env::var_os("RUST_BOOTSTRAP_CONFIG").is_some() {
                 "profile = \"user\"".to_owned()
@@ -223,9 +222,7 @@ fn profile_user_dist() {
                 std::fs::read_to_string(file).unwrap()
             };
 
-        toml::from_str(&contents)
-            .and_then(|table: toml::Value| TomlConfig::deserialize(table))
-            .unwrap()
+        toml::from_str(&contents).and_then(|table: toml::Value| TomlConfig::deserialize(table))
     }
     Config::parse_inner(Flags::parse(&["check".to_owned()]), get_toml);
 }