From 056d4e4c51dce1fddb1773e5a523525b8fd24d65 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sat, 11 Nov 2023 16:20:06 +0300 Subject: [PATCH 1/5] print the change warnings once for per id Signed-off-by: onur-ozkan --- src/bootstrap/src/bin/main.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bootstrap/src/bin/main.rs b/src/bootstrap/src/bin/main.rs index 0a6072ae1a5d5..1cbe3991abfd0 100644 --- a/src/bootstrap/src/bin/main.rs +++ b/src/bootstrap/src/bin/main.rs @@ -109,11 +109,19 @@ fn check_version(config: &Config) -> Option { } let latest_config_id = CONFIG_CHANGE_HISTORY.last().unwrap(); + let warned_id_path = config.out.join("bootstrap").join(".last-warned-change-id"); + if let Some(id) = config.change_id { if &id == latest_config_id { return None; } + if let Ok(last_warned_id) = fs::read_to_string(&warned_id_path) { + if id.to_string() == last_warned_id { + return None; + } + } + let change_links: Vec = find_recent_config_change_ids(id) .iter() .map(|id| format!("https://github.com/rust-lang/rust/pull/{id}")) @@ -132,6 +140,8 @@ fn check_version(config: &Config) -> Option { msg.push_str(&format!( "update `config.toml` to use `change-id = {latest_config_id}` instead" )); + + t!(fs::write(warned_id_path, id.to_string())); } } else { msg.push_str("WARNING: The `change-id` is missing in the `config.toml`. This means that you will not be able to track the major changes made to the bootstrap configurations.\n"); From 12ab8c638fefb182e24ccfe05924ae35eb1926a2 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sat, 11 Nov 2023 18:01:40 +0300 Subject: [PATCH 2/5] refactor change-tracking implementation Signed-off-by: onur-ozkan --- src/bootstrap/src/bin/main.rs | 29 +++++---- src/bootstrap/src/core/build_steps/setup.rs | 2 +- src/bootstrap/src/lib.rs | 66 +++++++++++++++++---- 3 files changed, 70 insertions(+), 27 deletions(-) diff --git a/src/bootstrap/src/bin/main.rs b/src/bootstrap/src/bin/main.rs index 1cbe3991abfd0..eb3fa0783a190 100644 --- a/src/bootstrap/src/bin/main.rs +++ b/src/bootstrap/src/bin/main.rs @@ -108,11 +108,11 @@ fn check_version(config: &Config) -> Option { msg.push_str("WARNING: The use of `changelog-seen` is deprecated. Please refer to `change-id` option in `config.example.toml` instead.\n"); } - let latest_config_id = CONFIG_CHANGE_HISTORY.last().unwrap(); + 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(id) = config.change_id { - if &id == latest_config_id { + if id == latest_change_id { return None; } @@ -122,23 +122,22 @@ fn check_version(config: &Config) -> Option { } } - let change_links: Vec = find_recent_config_change_ids(id) - .iter() - .map(|id| format!("https://github.com/rust-lang/rust/pull/{id}")) - .collect(); - if !change_links.is_empty() { - msg.push_str("WARNING: there have been changes to x.py since you last updated.\n"); - msg.push_str("To see more detail about these changes, visit the following PRs:\n"); + let changes = find_recent_config_change_ids(id); - for link in change_links { - msg.push_str(&format!(" - {link}\n")); - } + if !changes.is_empty() { + msg.push_str("There have been changes to x.py since you last updated:\n"); - msg.push_str("WARNING: there have been changes to x.py since you last updated.\n"); + for change in changes { + msg.push_str(&format!(" [{}] {}\n", change.severity.to_string(), change.summary)); + msg.push_str(&format!( + " - PR Link https://github.com/rust-lang/rust/pull/{}\n", + change.change_id + )); + } msg.push_str("NOTE: to silence this warning, "); msg.push_str(&format!( - "update `config.toml` to use `change-id = {latest_config_id}` instead" + "update `config.toml` to use `change-id = {latest_change_id}` instead" )); t!(fs::write(warned_id_path, id.to_string())); @@ -146,7 +145,7 @@ fn check_version(config: &Config) -> Option { } else { msg.push_str("WARNING: The `change-id` is missing in the `config.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_config_id}` at the top of `config.toml`")); + msg.push_str(&format!("add `change-id = {latest_change_id}` at the top of `config.toml`")); }; Some(msg) diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index 486a1e20f1835..bbbdb4c3186d6 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -226,7 +226,7 @@ fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) { return; } - let latest_change_id = CONFIG_CHANGE_HISTORY.last().unwrap(); + let latest_change_id = CONFIG_CHANGE_HISTORY.last().unwrap().change_id; let settings = format!( "# Includes one of the default files in src/bootstrap/defaults\n\ profile = \"{profile}\"\n\ diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 33b8f1a7ce720..0351ec53824f0 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -69,16 +69,59 @@ const LLVM_TOOLS: &[&str] = &[ /// LLD file names for all flavors. const LLD_FILE_NAMES: &[&str] = &["ld.lld", "ld64.lld", "lld-link", "wasm-ld"]; +#[derive(Clone, Debug)] +pub struct ChangeInfo { + /// Represents the ID of PR caused major change on bootstrap. + pub change_id: usize, + pub severity: ChangeSeverity, + /// Provides a short summary of the change that will guide developers + /// on "how to handle/behave" in response to the changes. + pub summary: &'static str, +} + +#[derive(Clone, Debug)] +pub enum ChangeSeverity { + Info, + Warning, +} + +impl ToString for ChangeSeverity { + fn to_string(&self) -> String { + match self { + ChangeSeverity::Info => "INFO".to_string(), + ChangeSeverity::Warning => "WARNING".to_string(), + } + } +} + /// Keeps track of major changes made to the bootstrap configuration. /// -/// These values also represent the IDs of the PRs that caused major changes. -/// You can visit `https://github.com/rust-lang/rust/pull/{any-id-from-the-list}` to -/// check for more details regarding each change. /// /// If you make any major changes (such as adding new values or changing default values), -/// please ensure that the associated PR ID is added to the end of this list. -/// This is necessary because the list must be sorted by the merge date. -pub const CONFIG_CHANGE_HISTORY: &[usize] = &[115898, 116998, 117435, 116881]; +/// please ensure adding `ChangeInfo` to the end(because the list must be sorted by the merge date) +/// of this list. +pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ + ChangeInfo { + change_id: 115898, + severity: ChangeSeverity::Info, + summary: "Implementation of this change-tracking system. Ignore this.", + }, + ChangeInfo { + change_id: 116998, + severity: ChangeSeverity::Info, + summary: "Removed android-ndk r15 support in favor of android-ndk r25b.", + }, + ChangeInfo { + change_id: 117435, + severity: ChangeSeverity::Info, + summary: "New option `rust.parallel-compiler` added to config.toml.", + }, + ChangeInfo { + change_id: 116881, + severity: ChangeSeverity::Warning, + summary: "Default value of `download-ci-llvm` was changed for `codegen` profile.", + }, +]; /// Extra --check-cfg to add when building /// (Mode restriction, config name, config values (if any)) @@ -1849,14 +1892,14 @@ fn envify(s: &str) -> String { .collect() } -pub fn find_recent_config_change_ids(current_id: usize) -> Vec { - if !CONFIG_CHANGE_HISTORY.contains(¤t_id) { +pub fn find_recent_config_change_ids(current_id: usize) -> Vec { + if !CONFIG_CHANGE_HISTORY.iter().any(|config| config.change_id == current_id) { // If the current change-id is greater than the most recent one, return // an empty list (it may be due to switching from a recent branch to an // older one); otherwise, return the full list (assuming the user provided // the incorrect change-id by accident). - if let Some(max_id) = CONFIG_CHANGE_HISTORY.iter().max() { - if ¤t_id > max_id { + if let Some(config) = CONFIG_CHANGE_HISTORY.iter().max_by_key(|config| config.change_id) { + if ¤t_id > &config.change_id { return Vec::new(); } } @@ -1864,7 +1907,8 @@ pub fn find_recent_config_change_ids(current_id: usize) -> Vec { return CONFIG_CHANGE_HISTORY.to_vec(); } - let index = CONFIG_CHANGE_HISTORY.iter().position(|&id| id == current_id).unwrap(); + let index = + CONFIG_CHANGE_HISTORY.iter().position(|config| config.change_id == current_id).unwrap(); CONFIG_CHANGE_HISTORY .iter() From d3d3eb91f72690a3d8a417c7dabe500a6256daf1 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sat, 11 Nov 2023 18:11:02 +0300 Subject: [PATCH 3/5] remove .last-warned-change-id on `x clean` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/clean.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/src/core/build_steps/clean.rs b/src/bootstrap/src/core/build_steps/clean.rs index cbb6b5f46488a..a3fb330ddfb6f 100644 --- a/src/bootstrap/src/core/build_steps/clean.rs +++ b/src/bootstrap/src/core/build_steps/clean.rs @@ -145,6 +145,7 @@ fn clean_specific_stage(build: &Build, stage: u32) { fn clean_default(build: &Build) { rm_rf(&build.out.join("tmp")); rm_rf(&build.out.join("dist")); + rm_rf(&build.out.join("bootstrap").join(".last-warned-change-id")); rm_rf(&build.out.join("rustfmt.stamp")); for host in &build.hosts { From 06f6cd95beca124a54a7346ea2e31b75e1bcfc5a Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sun, 12 Nov 2023 13:26:04 +0300 Subject: [PATCH 4/5] bootstrap: add doc-comments for `ChangeSeverity` Signed-off-by: onur-ozkan --- src/bootstrap/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 0351ec53824f0..00fe4422e5c3f 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -81,7 +81,10 @@ pub struct ChangeInfo { #[derive(Clone, Debug)] pub enum ChangeSeverity { + /// Used when build configurations continue working as before. Info, + /// Used when the default value of an option changes, or support for an option is removed entirely, + /// potentially requiring developers to update their build configurations. Warning, } @@ -96,7 +99,6 @@ impl ToString for ChangeSeverity { /// Keeps track of major changes made to the bootstrap configuration. /// -/// /// If you make any major changes (such as adding new values or changing default values), /// please ensure adding `ChangeInfo` to the end(because the list must be sorted by the merge date) /// of this list. From 9a6afd0362acee1ee9685f50e6c5286a19baf6d9 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sat, 11 Nov 2023 21:29:32 +0300 Subject: [PATCH 5/5] write .last-warned-change-id only if environment is tty As the .last-warned-change-id is only used for change tracking, we don't need to generate/write it outside of the tty. Otherwise, rust-analyzer could create this file, and developers wouldn't be able to see the bootstrap change alerts, assuming that they have already seen them. Signed-off-by: onur-ozkan --- src/bootstrap/src/bin/main.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/bin/main.rs b/src/bootstrap/src/bin/main.rs index eb3fa0783a190..35010bea8187e 100644 --- a/src/bootstrap/src/bin/main.rs +++ b/src/bootstrap/src/bin/main.rs @@ -9,7 +9,10 @@ use std::io::Write; #[cfg(all(any(unix, windows), not(target_os = "solaris")))] use std::process; -use std::{env, fs}; +use std::{ + env, fs, + io::{self, IsTerminal}, +}; #[cfg(all(any(unix, windows), not(target_os = "solaris")))] use bootstrap::t; @@ -140,7 +143,9 @@ fn check_version(config: &Config) -> Option { "update `config.toml` to use `change-id = {latest_change_id}` instead" )); - t!(fs::write(warned_id_path, id.to_string())); + if io::stdout().is_terminal() { + t!(fs::write(warned_id_path, id.to_string())); + } } } else { msg.push_str("WARNING: The `change-id` is missing in the `config.toml`. This means that you will not be able to track the major changes made to the bootstrap configurations.\n");