Skip to content

Commit 1eac3e3

Browse files
committed
feat(config): Add ChangeId enum for suppressing warnings
Introduces the `ChangeId` enum to allow suppressing `change_id` warnings. Now, `ChangeId` supports both numeric values and the string literal `"ignore"`. Numeric values behave as expected, while `"ignore"` is used to suppress warning messages.
1 parent 7d49ae9 commit 1eac3e3

File tree

4 files changed

+78
-54
lines changed

4 files changed

+78
-54
lines changed

src/bootstrap/src/bin/main.rs

+41-39
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use std::str::FromStr;
1111
use std::{env, process};
1212

1313
use bootstrap::{
14-
Build, CONFIG_CHANGE_HISTORY, Config, Flags, Subcommand, debug, find_recent_config_change_ids,
15-
human_readable_changes, t,
14+
Build, CONFIG_CHANGE_HISTORY, ChangeId, Config, Flags, Subcommand, debug,
15+
find_recent_config_change_ids, human_readable_changes, t,
1616
};
1717
#[cfg(feature = "tracing")]
1818
use tracing::instrument;
@@ -155,50 +155,52 @@ fn check_version(config: &Config) -> Option<String> {
155155
let latest_change_id = CONFIG_CHANGE_HISTORY.last().unwrap().change_id;
156156
let warned_id_path = config.out.join("bootstrap").join(".last-warned-change-id");
157157

158-
if let Some(mut id) = config.change_id {
159-
if id == latest_change_id {
160-
return None;
158+
let mut id = match config.change_id {
159+
Some(ChangeId::Id(id)) if id == latest_change_id => return None,
160+
Some(ChangeId::Ignore) => return None,
161+
Some(ChangeId::Id(id)) => id,
162+
None => {
163+
msg.push_str("WARNING: The `change-id` is missing in the `bootstrap.toml`. This means that you will not be able to track the major changes made to the bootstrap configurations.\n");
164+
msg.push_str("NOTE: to silence this warning, ");
165+
msg.push_str(&format!(
166+
"add `change-id = {latest_change_id}` at the top of `bootstrap.toml`"
167+
));
168+
return Some(msg);
161169
}
170+
};
162171

163-
// Always try to use `change-id` from .last-warned-change-id first. If it doesn't exist,
164-
// then use the one from the bootstrap.toml. This way we never show the same warnings
165-
// more than once.
166-
if let Ok(t) = fs::read_to_string(&warned_id_path) {
167-
let last_warned_id = usize::from_str(&t)
168-
.unwrap_or_else(|_| panic!("{} is corrupted.", warned_id_path.display()));
169-
170-
// We only use the last_warned_id if it exists in `CONFIG_CHANGE_HISTORY`.
171-
// Otherwise, we may retrieve all the changes if it's not the highest value.
172-
// For better understanding, refer to `change_tracker::find_recent_config_change_ids`.
173-
if CONFIG_CHANGE_HISTORY.iter().any(|config| config.change_id == last_warned_id) {
174-
id = last_warned_id;
175-
}
176-
};
172+
// Always try to use `change-id` from .last-warned-change-id first. If it doesn't exist,
173+
// then use the one from the bootstrap.toml. This way we never show the same warnings
174+
// more than once.
175+
if let Ok(t) = fs::read_to_string(&warned_id_path) {
176+
let last_warned_id = usize::from_str(&t)
177+
.unwrap_or_else(|_| panic!("{} is corrupted.", warned_id_path.display()));
178+
179+
// We only use the last_warned_id if it exists in `CONFIG_CHANGE_HISTORY`.
180+
// Otherwise, we may retrieve all the changes if it's not the highest value.
181+
// For better understanding, refer to `change_tracker::find_recent_config_change_ids`.
182+
if CONFIG_CHANGE_HISTORY.iter().any(|config| config.change_id == last_warned_id) {
183+
id = last_warned_id;
184+
}
185+
};
177186

178-
let changes = find_recent_config_change_ids(id);
187+
let changes = find_recent_config_change_ids(id);
179188

180-
if changes.is_empty() {
181-
return None;
182-
}
189+
if changes.is_empty() {
190+
return None;
191+
}
183192

184-
msg.push_str("There have been changes to x.py since you last updated:\n");
185-
msg.push_str(&human_readable_changes(&changes));
193+
msg.push_str("There have been changes to x.py since you last updated:\n");
194+
msg.push_str(&human_readable_changes(&changes));
186195

187-
msg.push_str("NOTE: to silence this warning, ");
188-
msg.push_str(&format!(
189-
"update `bootstrap.toml` to use `change-id = {latest_change_id}` instead"
190-
));
196+
msg.push_str("NOTE: to silence this warning, ");
197+
msg.push_str(&format!(
198+
"update `bootstrap.toml` to use `change-id = {latest_change_id}` instead"
199+
));
191200

192-
if io::stdout().is_terminal() {
193-
t!(fs::write(warned_id_path, latest_change_id.to_string()));
194-
}
195-
} else {
196-
msg.push_str("WARNING: The `change-id` is missing in the `bootstrap.toml`. This means that you will not be able to track the major changes made to the bootstrap configurations.\n");
197-
msg.push_str("NOTE: to silence this warning, ");
198-
msg.push_str(&format!(
199-
"add `change-id = {latest_change_id}` at the top of `bootstrap.toml`"
200-
));
201-
};
201+
if io::stdout().is_terminal() {
202+
t!(fs::write(warned_id_path, latest_change_id.to_string()));
203+
}
202204

203205
Some(msg)
204206
}

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

+33-12
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ pub enum GccCiMode {
193193
/// `bootstrap.example.toml`.
194194
#[derive(Default, Clone)]
195195
pub struct Config {
196-
pub change_id: Option<usize>,
196+
pub change_id: Option<ChangeId>,
197197
pub bypass_bootstrap_lock: bool,
198198
pub ccache: Option<String>,
199199
/// Call Build::ninja() instead of this.
@@ -700,14 +700,32 @@ pub(crate) struct TomlConfig {
700700
profile: Option<String>,
701701
}
702702

703+
/// This enum is used for deserializing change IDs from TOML, allowing both numeric values and the string `"ignore"`.
704+
#[derive(Clone, Debug, PartialEq)]
705+
pub enum ChangeId {
706+
Ignore,
707+
Id(usize),
708+
}
709+
703710
/// Since we use `#[serde(deny_unknown_fields)]` on `TomlConfig`, we need a wrapper type
704711
/// for the "change-id" field to parse it even if other fields are invalid. This ensures
705712
/// that if deserialization fails due to other fields, we can still provide the changelogs
706713
/// to allow developers to potentially find the reason for the failure in the logs..
707714
#[derive(Deserialize, Default)]
708715
pub(crate) struct ChangeIdWrapper {
709-
#[serde(alias = "change-id")]
710-
pub(crate) inner: Option<usize>,
716+
#[serde(alias = "change-id", default, deserialize_with = "deserialize_change_id")]
717+
pub(crate) inner: Option<ChangeId>,
718+
}
719+
720+
fn deserialize_change_id<'de, D: Deserializer<'de>>(
721+
deserializer: D,
722+
) -> Result<Option<ChangeId>, D::Error> {
723+
let value = toml::Value::deserialize(deserializer)?;
724+
Ok(match value {
725+
toml::Value::String(s) if s == "ignore" => Some(ChangeId::Ignore),
726+
toml::Value::Integer(i) => Some(ChangeId::Id(i as usize)),
727+
_ => return Err(serde::de::Error::custom("expected \"ignore\" or an integer")),
728+
})
711729
}
712730

713731
/// Describes how to handle conflicts in merging two [`TomlConfig`]
@@ -1351,15 +1369,18 @@ impl Config {
13511369
toml::from_str(&contents)
13521370
.and_then(|table: toml::Value| TomlConfig::deserialize(table))
13531371
.inspect_err(|_| {
1354-
if let Ok(Some(changes)) = toml::from_str(&contents)
1355-
.and_then(|table: toml::Value| ChangeIdWrapper::deserialize(table))
1356-
.map(|change_id| change_id.inner.map(crate::find_recent_config_change_ids))
1357-
{
1358-
if !changes.is_empty() {
1359-
println!(
1360-
"WARNING: There have been changes to x.py since you last updated:\n{}",
1361-
crate::human_readable_changes(&changes)
1362-
);
1372+
if let Ok(changes) = toml::from_str(&contents)
1373+
.and_then(|table: toml::Value| ChangeIdWrapper::deserialize(table)) {
1374+
if let Some(change_id) = changes.inner {
1375+
if let ChangeId::Id(id) = change_id {
1376+
let changes = crate::find_recent_config_change_ids(id);
1377+
if !changes.is_empty() {
1378+
println!(
1379+
"WARNING: There have been changes to x.py since you last updated:\n{}",
1380+
crate::human_readable_changes(&changes)
1381+
);
1382+
}
1383+
}
13631384
}
13641385
}
13651386
})

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use serde::Deserialize;
1010

1111
use super::flags::Flags;
1212
use super::{ChangeIdWrapper, Config, RUSTC_IF_UNCHANGED_ALLOWED_PATHS};
13+
use crate::ChangeId;
1314
use crate::core::build_steps::clippy::{LintConfig, get_clippy_rules_in_order};
1415
use crate::core::build_steps::llvm;
1516
use crate::core::build_steps::llvm::LLVM_INVALIDATION_PATHS;
@@ -171,7 +172,7 @@ runner = "x86_64-runner"
171172
)
172173
},
173174
);
174-
assert_eq!(config.change_id, Some(1), "setting top-level value");
175+
assert_eq!(config.change_id, Some(ChangeId::Id(1)), "setting top-level value");
175176
assert_eq!(
176177
config.rust_lto,
177178
crate::core::config::RustcLto::Fat,
@@ -311,7 +312,7 @@ fn parse_change_id_with_unknown_field() {
311312
"#;
312313

313314
let change_id_wrapper: ChangeIdWrapper = toml::from_str(config).unwrap();
314-
assert_eq!(change_id_wrapper.inner, Some(3461));
315+
assert_eq!(change_id_wrapper.inner, Some(ChangeId::Id(3461)));
315316
}
316317

317318
#[test]

src/bootstrap/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ mod core;
4545
mod utils;
4646

4747
pub use core::builder::PathSet;
48-
pub use core::config::Config;
4948
pub use core::config::flags::{Flags, Subcommand};
49+
pub use core::config::{ChangeId, Config};
5050

5151
#[cfg(feature = "tracing")]
5252
use tracing::{instrument, span};

0 commit comments

Comments
 (0)