Skip to content

feat(config): Add ChangeId enum for suppressing warnings #138986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions bootstrap.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@
# - A new option
# - A change in the default values
#
# If `change-id` does not match the version that is currently running,
# `x.py` will inform you about the changes made on bootstrap.
# If the change-id does not match the version currently in use, x.py will
# display the changes made to the bootstrap.
# To suppress these warnings, you can set change-id = "ignore".
#change-id = <latest change id in src/bootstrap/src/utils/change_tracker.rs>

# =============================================================================
Expand Down
80 changes: 41 additions & 39 deletions src/bootstrap/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use std::str::FromStr;
use std::{env, process};

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

if let Some(mut id) = config.change_id {
if id == latest_change_id {
return None;
let mut id = match config.change_id {
Some(ChangeId::Id(id)) if id == latest_change_id => return None,
Some(ChangeId::Ignore) => return None,
Some(ChangeId::Id(id)) => id,
None => {
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");
msg.push_str("NOTE: to silence this warning, ");
msg.push_str(&format!(
"add `change-id = {latest_change_id}` or change-id = \"ignore\" at the top of `bootstrap.toml`"
));
return Some(msg);
}
};

// Always try to use `change-id` from .last-warned-change-id first. If it doesn't exist,
// then use the one from the bootstrap.toml. This way we never show the same warnings
// more than once.
if let Ok(t) = fs::read_to_string(&warned_id_path) {
let last_warned_id = usize::from_str(&t)
.unwrap_or_else(|_| panic!("{} is corrupted.", warned_id_path.display()));

// We only use the last_warned_id if it exists in `CONFIG_CHANGE_HISTORY`.
// Otherwise, we may retrieve all the changes if it's not the highest value.
// For better understanding, refer to `change_tracker::find_recent_config_change_ids`.
if CONFIG_CHANGE_HISTORY.iter().any(|config| config.change_id == last_warned_id) {
id = last_warned_id;
}
};
// Always try to use `change-id` from .last-warned-change-id first. If it doesn't exist,
// then use the one from the bootstrap.toml. This way we never show the same warnings
// more than once.
if let Ok(t) = fs::read_to_string(&warned_id_path) {
let last_warned_id = usize::from_str(&t)
.unwrap_or_else(|_| panic!("{} is corrupted.", warned_id_path.display()));

// We only use the last_warned_id if it exists in `CONFIG_CHANGE_HISTORY`.
// Otherwise, we may retrieve all the changes if it's not the highest value.
// For better understanding, refer to `change_tracker::find_recent_config_change_ids`.
if CONFIG_CHANGE_HISTORY.iter().any(|config| config.change_id == last_warned_id) {
id = last_warned_id;
}
};

let changes = find_recent_config_change_ids(id);
let changes = find_recent_config_change_ids(id);

if changes.is_empty() {
return None;
}
if changes.is_empty() {
return None;
}

msg.push_str("There have been changes to x.py since you last updated:\n");
msg.push_str(&human_readable_changes(&changes));
msg.push_str("There have been changes to x.py since you last updated:\n");
msg.push_str(&human_readable_changes(&changes));

msg.push_str("NOTE: to silence this warning, ");
msg.push_str(&format!(
"update `bootstrap.toml` to use `change-id = {latest_change_id}` instead"
));
msg.push_str("NOTE: to silence this warning, ");
msg.push_str(&format!(
"update `bootstrap.toml` to use `change-id = {latest_change_id}` or change-id = \"ignore\" instead"
));

if io::stdout().is_terminal() {
t!(fs::write(warned_id_path, latest_change_id.to_string()));
}
} else {
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");
msg.push_str("NOTE: to silence this warning, ");
msg.push_str(&format!(
"add `change-id = {latest_change_id}` at the top of `bootstrap.toml`"
));
};
if io::stdout().is_terminal() {
t!(fs::write(warned_id_path, latest_change_id.to_string()));
}

Some(msg)
}
Expand Down
35 changes: 29 additions & 6 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ pub enum GccCiMode {
/// `bootstrap.example.toml`.
#[derive(Default, Clone)]
pub struct Config {
pub change_id: Option<usize>,
pub change_id: Option<ChangeId>,
pub bypass_bootstrap_lock: bool,
pub ccache: Option<String>,
/// Call Build::ninja() instead of this.
Expand Down Expand Up @@ -700,14 +700,36 @@ pub(crate) struct TomlConfig {
profile: Option<String>,
}

/// This enum is used for deserializing change IDs from TOML, allowing both numeric values and the string `"ignore"`.
#[derive(Clone, Debug, PartialEq)]
pub enum ChangeId {
Ignore,
Id(usize),
}

/// Since we use `#[serde(deny_unknown_fields)]` on `TomlConfig`, we need a wrapper type
/// for the "change-id" field to parse it even if other fields are invalid. This ensures
/// that if deserialization fails due to other fields, we can still provide the changelogs
/// to allow developers to potentially find the reason for the failure in the logs..
#[derive(Deserialize, Default)]
pub(crate) struct ChangeIdWrapper {
#[serde(alias = "change-id")]
pub(crate) inner: Option<usize>,
#[serde(alias = "change-id", default, deserialize_with = "deserialize_change_id")]
pub(crate) inner: Option<ChangeId>,
}

fn deserialize_change_id<'de, D: Deserializer<'de>>(
deserializer: D,
) -> Result<Option<ChangeId>, D::Error> {
let value = toml::Value::deserialize(deserializer)?;
Ok(match value {
toml::Value::String(s) if s == "ignore" => Some(ChangeId::Ignore),
toml::Value::Integer(i) => Some(ChangeId::Id(i as usize)),
_ => {
return Err(serde::de::Error::custom(
"expected \"ignore\" or an integer for change-id",
));
}
})
}

/// Describes how to handle conflicts in merging two [`TomlConfig`]
Expand Down Expand Up @@ -1351,10 +1373,11 @@ impl Config {
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 let Ok(ChangeIdWrapper { inner: Some(ChangeId::Id(id)) }) =
toml::from_str::<toml::Value>(&contents)
.and_then(|table: toml::Value| ChangeIdWrapper::deserialize(table))
{
let changes = crate::find_recent_config_change_ids(id);
if !changes.is_empty() {
println!(
"WARNING: There have been changes to x.py since you last updated:\n{}",
Expand Down
5 changes: 3 additions & 2 deletions src/bootstrap/src/core/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use serde::Deserialize;

use super::flags::Flags;
use super::{ChangeIdWrapper, Config, RUSTC_IF_UNCHANGED_ALLOWED_PATHS};
use crate::ChangeId;
use crate::core::build_steps::clippy::{LintConfig, get_clippy_rules_in_order};
use crate::core::build_steps::llvm;
use crate::core::build_steps::llvm::LLVM_INVALIDATION_PATHS;
Expand Down Expand Up @@ -171,7 +172,7 @@ runner = "x86_64-runner"
)
},
);
assert_eq!(config.change_id, Some(1), "setting top-level value");
assert_eq!(config.change_id, Some(ChangeId::Id(1)), "setting top-level value");
assert_eq!(
config.rust_lto,
crate::core::config::RustcLto::Fat,
Expand Down Expand Up @@ -311,7 +312,7 @@ fn parse_change_id_with_unknown_field() {
"#;

let change_id_wrapper: ChangeIdWrapper = toml::from_str(config).unwrap();
assert_eq!(change_id_wrapper.inner, Some(3461));
assert_eq!(change_id_wrapper.inner, Some(ChangeId::Id(3461)));
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ mod core;
mod utils;

pub use core::builder::PathSet;
pub use core::config::Config;
pub use core::config::flags::{Flags, Subcommand};
pub use core::config::{ChangeId, Config};

#[cfg(feature = "tracing")]
use tracing::{instrument, span};
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,4 +390,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
severity: ChangeSeverity::Warning,
summary: "The default configuration filename has changed from `config.toml` to `bootstrap.toml`. `config.toml` is deprecated but remains supported for backward compatibility.",
},
ChangeInfo {
change_id: 138986,
severity: ChangeSeverity::Info,
summary: "You can now use `change-id = \"ignore\"` to suppress `change-id ` warnings in the console.",
},
];
Loading