Skip to content

Commit 41ef477

Browse files
committed
run change tracker even when config parse fails
Please note that we are currently validating the build configuration on two entry points (e.g., profile validation is handled on the python side), and change-tracker system is handled on the rust side. Once #94829 is completed (scheduled for 2024), we will be able to handle this more effectively. Signed-off-by: onur-ozkan <[email protected]>
1 parent b6e4299 commit 41ef477

File tree

4 files changed

+48
-13
lines changed

4 files changed

+48
-13
lines changed

src/bootstrap/src/bin/main.rs

+3-9
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ use std::{
1414
};
1515

1616
use bootstrap::{
17-
find_recent_config_change_ids, t, Build, Config, Subcommand, CONFIG_CHANGE_HISTORY,
17+
find_recent_config_change_ids, into_human_readable_changes, t, Build, Config, Subcommand,
18+
CONFIG_CHANGE_HISTORY,
1819
};
1920

2021
fn main() {
@@ -154,14 +155,7 @@ fn check_version(config: &Config) -> Option<String> {
154155
}
155156

156157
msg.push_str("There have been changes to x.py since you last updated:\n");
157-
158-
for change in changes {
159-
msg.push_str(&format!(" [{}] {}\n", change.severity, change.summary));
160-
msg.push_str(&format!(
161-
" - PR Link https://github.com/rust-lang/rust/pull/{}\n",
162-
change.change_id
163-
));
164-
}
158+
msg.push_str(&into_human_readable_changes(&changes));
165159

166160
msg.push_str("NOTE: to silence this warning, ");
167161
msg.push_str(&format!(

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

+28-3
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,8 @@ impl Target {
600600
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
601601
pub(crate) struct TomlConfig {
602602
changelog_seen: Option<usize>, // FIXME: Deprecated field. Remove it at 2024.
603-
change_id: Option<usize>,
603+
#[serde(flatten)]
604+
change_id: ChangeIdWrapper,
604605
build: Option<Build>,
605606
install: Option<Install>,
606607
llvm: Option<Llvm>,
@@ -610,6 +611,16 @@ pub(crate) struct TomlConfig {
610611
profile: Option<String>,
611612
}
612613

614+
/// Since we use `#[serde(deny_unknown_fields)]` on `TomlConfig`, we need a wrapper type
615+
/// for the "change-id" field to parse it even if other fields are invalid. This ensures
616+
/// that if deserialization fails due to other fields, we can still provide the changelogs
617+
/// to allow developers to potentially find the reason for the failure in the logs..
618+
#[derive(Deserialize, Default)]
619+
pub(crate) struct ChangeIdWrapper {
620+
#[serde(alias = "change-id")]
621+
inner: Option<usize>,
622+
}
623+
613624
/// Describes how to handle conflicts in merging two [`TomlConfig`]
614625
#[derive(Copy, Clone, Debug)]
615626
enum ReplaceOpt {
@@ -651,7 +662,7 @@ impl Merge for TomlConfig {
651662
}
652663
}
653664
self.changelog_seen.merge(changelog_seen, replace);
654-
self.change_id.merge(change_id, replace);
665+
self.change_id.inner.merge(change_id.inner, replace);
655666
do_merge(&mut self.build, build, replace);
656667
do_merge(&mut self.install, install, replace);
657668
do_merge(&mut self.llvm, llvm, replace);
@@ -1200,6 +1211,20 @@ impl Config {
12001211
toml::from_str(&contents)
12011212
.and_then(|table: toml::Value| TomlConfig::deserialize(table))
12021213
.unwrap_or_else(|err| {
1214+
if let Ok(Some(changes)) = toml::from_str(&contents)
1215+
.and_then(|table: toml::Value| ChangeIdWrapper::deserialize(table))
1216+
.and_then(|change_id| {
1217+
Ok(change_id.inner.map(|id| crate::find_recent_config_change_ids(id)))
1218+
})
1219+
{
1220+
if !changes.is_empty() {
1221+
println!(
1222+
"WARNING: There have been changes to x.py since you last updated:\n{}",
1223+
crate::into_human_readable_changes(&changes)
1224+
);
1225+
}
1226+
}
1227+
12031228
eprintln!("failed to parse TOML configuration '{}': {err}", file.display());
12041229
exit!(2);
12051230
})
@@ -1366,7 +1391,7 @@ impl Config {
13661391
toml.merge(override_toml, ReplaceOpt::Override);
13671392

13681393
config.changelog_seen = toml.changelog_seen;
1369-
config.change_id = toml.change_id;
1394+
config.change_id = toml.change_id.inner;
13701395

13711396
let Build {
13721397
build,

src/bootstrap/src/lib.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ mod utils;
5151
pub use core::builder::PathSet;
5252
pub use core::config::flags::Subcommand;
5353
pub use core::config::Config;
54-
pub use utils::change_tracker::{find_recent_config_change_ids, CONFIG_CHANGE_HISTORY};
54+
pub use utils::change_tracker::{
55+
find_recent_config_change_ids, into_human_readable_changes, CONFIG_CHANGE_HISTORY,
56+
};
5557

5658
const LLVM_TOOLS: &[&str] = &[
5759
"llvm-cov", // used to generate coverage report

src/bootstrap/src/utils/change_tracker.rs

+14
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,20 @@ pub fn find_recent_config_change_ids(current_id: usize) -> Vec<ChangeInfo> {
6060
.collect()
6161
}
6262

63+
pub fn into_human_readable_changes(changes: &[ChangeInfo]) -> String {
64+
let mut message = String::new();
65+
66+
for change in changes {
67+
message.push_str(&format!(" [{}] {}\n", change.severity, change.summary));
68+
message.push_str(&format!(
69+
" - PR Link https://github.com/rust-lang/rust/pull/{}\n",
70+
change.change_id
71+
));
72+
}
73+
74+
message
75+
}
76+
6377
/// Keeps track of major changes made to the bootstrap configuration.
6478
///
6579
/// If you make any major changes (such as adding new values or changing default values),

0 commit comments

Comments
 (0)