From aba675fe7597e2aabf53d1d93d52f57aedc97dd6 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 13 Aug 2024 13:48:23 +0300 Subject: [PATCH 01/10] copy `builder-config` file into ci-rustc sysroot Signed-off-by: onur-ozkan --- src/bootstrap/src/core/download.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 4d1aea3cd956a..a88b4a33856cf 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -273,11 +273,12 @@ impl Config { let mut tar = tar::Archive::new(decompressor); + let is_ci_rustc = dst.ends_with("ci-rustc"); + // `compile::Sysroot` needs to know the contents of the `rustc-dev` tarball to avoid adding // it to the sysroot unless it was explicitly requested. But parsing the 100 MB tarball is slow. // Cache the entries when we extract it so we only have to read it once. - let mut recorded_entries = - if dst.ends_with("ci-rustc") { recorded_entries(dst, pattern) } else { None }; + let mut recorded_entries = if is_ci_rustc { recorded_entries(dst, pattern) } else { None }; for member in t!(tar.entries()) { let mut member = t!(member); @@ -287,10 +288,12 @@ impl Config { continue; } let mut short_path = t!(original_path.strip_prefix(directory_prefix)); - if !short_path.starts_with(pattern) { + let is_builder_config = short_path.to_str() == Some("builder-config"); + + if !short_path.starts_with(pattern) && (is_ci_rustc && !is_builder_config) { continue; } - short_path = t!(short_path.strip_prefix(pattern)); + short_path = short_path.strip_prefix(pattern).unwrap_or(short_path); let dst_path = dst.join(short_path); self.verbose(|| { println!("extracting {} to {}", original_path.display(), dst.display()) From b2f1fc16979f9a0c17863ffef3178206071eed83 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 13 Aug 2024 17:20:39 +0300 Subject: [PATCH 02/10] separate inner function (`get_toml`) of `Config::parse` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 70 +++++++++++++------------ 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 35ee4a29c6822..91f73c2a48fb2 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1191,37 +1191,39 @@ impl Config { } } - pub fn parse(flags: Flags) -> Config { - #[cfg(test)] - fn get_toml(_: &Path) -> TomlConfig { - TomlConfig::default() - } + #[cfg(test)] + fn get_toml(_: &Path) -> TomlConfig { + 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) -> 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) + ); } + } - eprintln!("failed to parse TOML configuration '{}': {err}", file.display()); - exit!(2); - }) - } - Self::parse_inner(flags, get_toml) + eprintln!("failed to parse TOML configuration '{}': {err}", file.display()); + exit!(2); + }) + } + + 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 { @@ -2656,10 +2658,10 @@ fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> { macro_rules! err { ($name:expr) => { if $name.is_some() { - return Err(format!( - "ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`.", + return Err(format!( + "ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`.", stringify!($name).replace("_", "-") - )); + )); } }; } @@ -2667,10 +2669,10 @@ fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> { macro_rules! warn { ($name:expr) => { if $name.is_some() { - println!( - "WARNING: `rust.{}` has no effect with `rust.download-rustc`.", + println!( + "WARNING: `rust.{}` has no effect with `rust.download-rustc`.", stringify!($name).replace("_", "-") - ); + ); } }; } From 0518e8c7fd8c7e890b7a3f603944136374596639 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 13 Aug 2024 17:22:13 +0300 Subject: [PATCH 03/10] 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 --- src/bootstrap/src/core/config/config.rs | 117 ++++++++++++++---------- 1 file changed, 69 insertions(+), 48 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 91f73c2a48fb2..2f28d1d2cbe5b 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -117,7 +117,7 @@ impl Display for DebuginfoLevel { /// 2) MSVC /// - Self-contained: `-Clinker=` /// - External: `-Clinker=lld` -#[derive(Default, Copy, Clone)] +#[derive(Copy, Clone, Default, PartialEq)] pub enum LldMode { /// Do not use LLD #[default] @@ -1581,24 +1581,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, @@ -1646,7 +1628,7 @@ impl Config { new_symbol_mangling, profile_generate, profile_use, - download_rustc: _, + download_rustc, lto, validate_mir_opts, frame_pointers, @@ -1658,6 +1640,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; @@ -2335,6 +2319,29 @@ impl Config { None => None, Some(commit) => { self.download_ci_rustc(commit); + + let builder_config_path = + self.out.join(self.build.triple).join("ci-rustc/builder-config"); + let ci_config_toml = Self::get_toml(&builder_config_path); + let current_config_toml = Self::get_toml(self.config.as_ref().unwrap()); + + // Check the config compatibility + 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()) } }) @@ -2652,31 +2659,44 @@ 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() { + ($current:expr, $expected:expr) => { + if let Some(current) = $current { + if Some(current) != $expected { return Err(format!( "ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`.", - stringify!($name).replace("_", "-") + stringify!($expected).replace("_", "-") )); - } + }; + }; }; } macro_rules! warn { - ($name:expr) => { - if $name.is_some() { + ($current:expr, $expected:expr) => { + if let Some(current) = $current { + if Some(current) != $expected { println!( "WARNING: `rust.{}` has no effect with `rust.download-rustc`.", - stringify!($name).replace("_", "-") + stringify!($expected).replace("_", "-") ); - } + }; + }; }; } + 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, @@ -2734,7 +2754,7 @@ 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. @@ -2742,22 +2762,23 @@ fn check_incompatible_options_for_ci_rustc(rust: &Rust) -> Result<(), String> { // // 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(()) } From b76f6a3b1a5124e4ade1e54299e74e3026192196 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Aug 2024 08:04:02 +0300 Subject: [PATCH 04/10] print values of incompatible options Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 2f28d1d2cbe5b..6dc3e69d8ba3c 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -47,7 +47,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 +117,7 @@ impl Display for DebuginfoLevel { /// 2) MSVC /// - Self-contained: `-Clinker=` /// - External: `-Clinker=lld` -#[derive(Copy, Clone, Default, PartialEq)] +#[derive(Copy, Clone, Default, Debug, PartialEq)] pub enum LldMode { /// Do not use LLD #[default] @@ -2667,11 +2667,14 @@ fn check_incompatible_options_for_ci_rustc( ) -> Result<(), String> { macro_rules! err { ($current:expr, $expected:expr) => { - if let Some(current) = $current { - if Some(current) != $expected { + if let Some(current) = &$current { + if Some(current) != $expected.as_ref() { return Err(format!( - "ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`.", - stringify!($expected).replace("_", "-") + "ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`. \ + Current value: {:?}, Expected value(s): None/{:?}", + stringify!($expected).replace("_", "-"), + $current, + $expected )); }; }; From 43320d5d6c53d506bfedff3abde40a9e26b0441a Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Aug 2024 08:06:50 +0300 Subject: [PATCH 05/10] do not check incompatibility if `config.toml` isn't present Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 36 +++++++++++++------------ 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 6dc3e69d8ba3c..b955593ecf64d 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -2320,27 +2320,29 @@ impl Config { Some(commit) => { self.download_ci_rustc(commit); - let builder_config_path = - self.out.join(self.build.triple).join("ci-rustc/builder-config"); - let ci_config_toml = Self::get_toml(&builder_config_path); - let current_config_toml = Self::get_toml(self.config.as_ref().unwrap()); - - // Check the config compatibility - let res = check_incompatible_options_for_ci_rustc( - current_config_toml, - ci_config_toml, - ); + if let Some(config_path) = &self.config { + let builder_config_path = + self.out.join(self.build.triple).join("ci-rustc/builder-config"); + let ci_config_toml = Self::get_toml(&builder_config_path); + let current_config_toml = Self::get_toml(config_path); + + // Check the config compatibility + 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") + 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; - } + 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(); + res.unwrap(); + } Some(commit.clone()) } From 4d21c735a27533396da7e8f15bc042ef22b96130 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Aug 2024 08:14:28 +0300 Subject: [PATCH 06/10] create `const BUILDER_CONFIG_FILENAME` for builder-config file Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 10 +++++++++- src/bootstrap/src/core/download.rs | 3 ++- src/bootstrap/src/utils/tarball.rs | 3 ++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index b955593ecf64d..5f1d923b3f852 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/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. @@ -2322,7 +2330,7 @@ impl Config { if let Some(config_path) = &self.config { let builder_config_path = - self.out.join(self.build.triple).join("ci-rustc/builder-config"); + self.out.join(self.build.triple).join("ci-rustc").join(BUILDER_CONFIG_FILENAME); let ci_config_toml = Self::get_toml(&builder_config_path); let current_config_toml = Self::get_toml(config_path); diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index a88b4a33856cf..3c4e47bf4434d 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -9,6 +9,7 @@ use std::sync::OnceLock; use build_helper::ci::CiEnv; use xz2::bufread::XzDecoder; +use crate::core::config::BUILDER_CONFIG_FILENAME; use crate::utils::exec::{command, BootstrapCommand}; use crate::utils::helpers::{check_run, exe, hex_encode, move_file, program_out_of_date}; use crate::{t, Config}; @@ -288,7 +289,7 @@ impl Config { continue; } let mut short_path = t!(original_path.strip_prefix(directory_prefix)); - let is_builder_config = short_path.to_str() == Some("builder-config"); + let is_builder_config = short_path.to_str() == Some(BUILDER_CONFIG_FILENAME); if !short_path.starts_with(pattern) && (is_ci_rustc && !is_builder_config) { continue; diff --git a/src/bootstrap/src/utils/tarball.rs b/src/bootstrap/src/utils/tarball.rs index 3f7f6214cf682..3c6c7a7fa180a 100644 --- a/src/bootstrap/src/utils/tarball.rs +++ b/src/bootstrap/src/utils/tarball.rs @@ -9,6 +9,7 @@ use std::path::{Path, PathBuf}; use crate::core::build_steps::dist::distdir; use crate::core::builder::{Builder, Kind}; +use crate::core::config::BUILDER_CONFIG_FILENAME; use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{move_file, t}; use crate::utils::{channel, helpers}; @@ -320,7 +321,7 @@ impl<'a> Tarball<'a> { // Add config file if present. if let Some(config) = &self.builder.config.config { - self.add_renamed_file(config, &self.overlay_dir, "builder-config"); + self.add_renamed_file(config, &self.overlay_dir, BUILDER_CONFIG_FILENAME); } for file in self.overlay.legal_and_readme() { From 4d13470834bb651362c9af0dd56419ebc83d6293 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Aug 2024 08:29:47 +0300 Subject: [PATCH 07/10] fix the incorrect `unpack`ing logic Signed-off-by: onur-ozkan --- src/bootstrap/src/core/download.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 3c4e47bf4434d..ef178fb633437 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -291,7 +291,7 @@ impl Config { let mut short_path = t!(original_path.strip_prefix(directory_prefix)); let is_builder_config = short_path.to_str() == Some(BUILDER_CONFIG_FILENAME); - if !short_path.starts_with(pattern) && (is_ci_rustc && !is_builder_config) { + if !(short_path.starts_with(pattern) || (is_ci_rustc && is_builder_config)) { continue; } short_path = short_path.strip_prefix(pattern).unwrap_or(short_path); From 151e8cd2f84894c62b76d1bffa2b50ddb0bfcd9b Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Aug 2024 11:11:40 +0300 Subject: [PATCH 08/10] improve `config::check_incompatible_options_for_ci_rustc` logs Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 5f1d923b3f852..00c564a0e29e8 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -2681,10 +2681,11 @@ fn check_incompatible_options_for_ci_rustc( if Some(current) != $expected.as_ref() { return Err(format!( "ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`. \ - Current value: {:?}, Expected value(s): None/{:?}", + Current value: {:?}, Expected value(s): {}{:?}", stringify!($expected).replace("_", "-"), $current, - $expected + if $expected.is_some() { "None/" } else { "" }, + $expected, )); }; }; @@ -2693,11 +2694,15 @@ fn check_incompatible_options_for_ci_rustc( macro_rules! warn { ($current:expr, $expected:expr) => { - if let Some(current) = $current { - if Some(current) != $expected { + if let Some(current) = &$current { + if Some(current) != $expected.as_ref() { println!( - "WARNING: `rust.{}` has no effect with `rust.download-rustc`.", - stringify!($expected).replace("_", "-") + "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, ); }; }; From 0935c8660355c30e890b3c0782d5d57f2f3f240f Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Aug 2024 11:16:26 +0300 Subject: [PATCH 09/10] leave a FIXME note for `--set` flags coverage Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 00c564a0e29e8..6564641f54012 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -2335,6 +2335,7 @@ impl Config { let current_config_toml = Self::get_toml(config_path); // 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, From 469d5937bf392d79557dc2c6b0548a7d32ab6465 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Aug 2024 16:55:19 +0300 Subject: [PATCH 10/10] disable download-rustc if CI rustc has unsupported options Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 46 ++++++++++++++++++------- src/bootstrap/src/core/config/tests.rs | 11 +++--- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 6564641f54012..dac1fd3e4a524 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1200,19 +1200,19 @@ impl Config { } #[cfg(test)] - fn get_toml(_: &Path) -> TomlConfig { - TomlConfig::default() + fn get_toml(_: &Path) -> Result { + Ok(TomlConfig::default()) } #[cfg(not(test))] - fn get_toml(file: &Path) -> TomlConfig { + fn get_toml(file: &Path) -> Result { 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| { + .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)) @@ -1224,9 +1224,6 @@ impl Config { ); } } - - eprintln!("failed to parse TOML configuration '{}': {err}", file.display()); - exit!(2); }) } @@ -1234,7 +1231,10 @@ impl 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, + ) -> Config { let mut config = Config::default_opts(); // Set flags. @@ -1342,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() @@ -1373,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); } @@ -2331,8 +2340,21 @@ impl Config { 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 = Self::get_toml(&builder_config_path); - let current_config_toml = Self::get_toml(config_path); + + 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. diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index 40f3e5e7222fa..378d069672f5d 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/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 { 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); }