Skip to content

Commit 91f7f26

Browse files
committed
Auto merge of rust-lang#117815 - onur-ozkan:update-change-tracking-impl, r=albertlarsan68
improve bootstrap change-tracking system This PR aims to improve how change-tracking system works for bootstrap changes by doing the followings: - Enforce embedding directive informations about the changes on `bootstrap/src/lib.rs`. - Give more informative change inputs on the terminal (rust-lang#117815 (comment)). - Avoid spamming the change informations(by reading and creating `.last-warned-change-id` under build output dir). see the zulip conversation for more details: https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/config.2Etoml.20change.20tracking cc `@RalfJung`
2 parents e7b2285 + 9a6afd0 commit 91f7f26

File tree

4 files changed

+90
-29
lines changed

4 files changed

+90
-29
lines changed

Diff for: src/bootstrap/src/bin/main.rs

+30-16
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
use std::io::Write;
1010
#[cfg(all(any(unix, windows), not(target_os = "solaris")))]
1111
use std::process;
12-
use std::{env, fs};
12+
use std::{
13+
env, fs,
14+
io::{self, IsTerminal},
15+
};
1316

1417
#[cfg(all(any(unix, windows), not(target_os = "solaris")))]
1518
use bootstrap::t;
@@ -108,35 +111,46 @@ fn check_version(config: &Config) -> Option<String> {
108111
msg.push_str("WARNING: The use of `changelog-seen` is deprecated. Please refer to `change-id` option in `config.example.toml` instead.\n");
109112
}
110113

111-
let latest_config_id = CONFIG_CHANGE_HISTORY.last().unwrap();
114+
let latest_change_id = CONFIG_CHANGE_HISTORY.last().unwrap().change_id;
115+
let warned_id_path = config.out.join("bootstrap").join(".last-warned-change-id");
116+
112117
if let Some(id) = config.change_id {
113-
if &id == latest_config_id {
118+
if id == latest_change_id {
114119
return None;
115120
}
116121

117-
let change_links: Vec<String> = find_recent_config_change_ids(id)
118-
.iter()
119-
.map(|id| format!("https://github.com/rust-lang/rust/pull/{id}"))
120-
.collect();
121-
if !change_links.is_empty() {
122-
msg.push_str("WARNING: there have been changes to x.py since you last updated.\n");
123-
msg.push_str("To see more detail about these changes, visit the following PRs:\n");
124-
125-
for link in change_links {
126-
msg.push_str(&format!(" - {link}\n"));
122+
if let Ok(last_warned_id) = fs::read_to_string(&warned_id_path) {
123+
if id.to_string() == last_warned_id {
124+
return None;
127125
}
126+
}
127+
128+
let changes = find_recent_config_change_ids(id);
128129

129-
msg.push_str("WARNING: there have been changes to x.py since you last updated.\n");
130+
if !changes.is_empty() {
131+
msg.push_str("There have been changes to x.py since you last updated:\n");
132+
133+
for change in changes {
134+
msg.push_str(&format!(" [{}] {}\n", change.severity.to_string(), change.summary));
135+
msg.push_str(&format!(
136+
" - PR Link https://github.com/rust-lang/rust/pull/{}\n",
137+
change.change_id
138+
));
139+
}
130140

131141
msg.push_str("NOTE: to silence this warning, ");
132142
msg.push_str(&format!(
133-
"update `config.toml` to use `change-id = {latest_config_id}` instead"
143+
"update `config.toml` to use `change-id = {latest_change_id}` instead"
134144
));
145+
146+
if io::stdout().is_terminal() {
147+
t!(fs::write(warned_id_path, id.to_string()));
148+
}
135149
}
136150
} else {
137151
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");
138152
msg.push_str("NOTE: to silence this warning, ");
139-
msg.push_str(&format!("add `change-id = {latest_config_id}` at the top of `config.toml`"));
153+
msg.push_str(&format!("add `change-id = {latest_change_id}` at the top of `config.toml`"));
140154
};
141155

142156
Some(msg)

Diff for: src/bootstrap/src/core/build_steps/clean.rs

+1
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ fn clean_specific_stage(build: &Build, stage: u32) {
145145
fn clean_default(build: &Build) {
146146
rm_rf(&build.out.join("tmp"));
147147
rm_rf(&build.out.join("dist"));
148+
rm_rf(&build.out.join("bootstrap").join(".last-warned-change-id"));
148149
rm_rf(&build.out.join("rustfmt.stamp"));
149150

150151
for host in &build.hosts {

Diff for: src/bootstrap/src/core/build_steps/setup.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) {
226226
return;
227227
}
228228

229-
let latest_change_id = CONFIG_CHANGE_HISTORY.last().unwrap();
229+
let latest_change_id = CONFIG_CHANGE_HISTORY.last().unwrap().change_id;
230230
let settings = format!(
231231
"# Includes one of the default files in src/bootstrap/defaults\n\
232232
profile = \"{profile}\"\n\

Diff for: src/bootstrap/src/lib.rs

+58-12
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,61 @@ const LLVM_TOOLS: &[&str] = &[
6969
/// LLD file names for all flavors.
7070
const LLD_FILE_NAMES: &[&str] = &["ld.lld", "ld64.lld", "lld-link", "wasm-ld"];
7171

72+
#[derive(Clone, Debug)]
73+
pub struct ChangeInfo {
74+
/// Represents the ID of PR caused major change on bootstrap.
75+
pub change_id: usize,
76+
pub severity: ChangeSeverity,
77+
/// Provides a short summary of the change that will guide developers
78+
/// on "how to handle/behave" in response to the changes.
79+
pub summary: &'static str,
80+
}
81+
82+
#[derive(Clone, Debug)]
83+
pub enum ChangeSeverity {
84+
/// Used when build configurations continue working as before.
85+
Info,
86+
/// Used when the default value of an option changes, or support for an option is removed entirely,
87+
/// potentially requiring developers to update their build configurations.
88+
Warning,
89+
}
90+
91+
impl ToString for ChangeSeverity {
92+
fn to_string(&self) -> String {
93+
match self {
94+
ChangeSeverity::Info => "INFO".to_string(),
95+
ChangeSeverity::Warning => "WARNING".to_string(),
96+
}
97+
}
98+
}
99+
72100
/// Keeps track of major changes made to the bootstrap configuration.
73101
///
74-
/// These values also represent the IDs of the PRs that caused major changes.
75-
/// You can visit `https://github.com/rust-lang/rust/pull/{any-id-from-the-list}` to
76-
/// check for more details regarding each change.
77-
///
78102
/// If you make any major changes (such as adding new values or changing default values),
79-
/// please ensure that the associated PR ID is added to the end of this list.
80-
/// This is necessary because the list must be sorted by the merge date.
81-
pub const CONFIG_CHANGE_HISTORY: &[usize] = &[115898, 116998, 117435, 116881];
103+
/// please ensure adding `ChangeInfo` to the end(because the list must be sorted by the merge date)
104+
/// of this list.
105+
pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
106+
ChangeInfo {
107+
change_id: 115898,
108+
severity: ChangeSeverity::Info,
109+
summary: "Implementation of this change-tracking system. Ignore this.",
110+
},
111+
ChangeInfo {
112+
change_id: 116998,
113+
severity: ChangeSeverity::Info,
114+
summary: "Removed android-ndk r15 support in favor of android-ndk r25b.",
115+
},
116+
ChangeInfo {
117+
change_id: 117435,
118+
severity: ChangeSeverity::Info,
119+
summary: "New option `rust.parallel-compiler` added to config.toml.",
120+
},
121+
ChangeInfo {
122+
change_id: 116881,
123+
severity: ChangeSeverity::Warning,
124+
summary: "Default value of `download-ci-llvm` was changed for `codegen` profile.",
125+
},
126+
];
82127

83128
/// Extra --check-cfg to add when building
84129
/// (Mode restriction, config name, config values (if any))
@@ -1849,22 +1894,23 @@ fn envify(s: &str) -> String {
18491894
.collect()
18501895
}
18511896

1852-
pub fn find_recent_config_change_ids(current_id: usize) -> Vec<usize> {
1853-
if !CONFIG_CHANGE_HISTORY.contains(&current_id) {
1897+
pub fn find_recent_config_change_ids(current_id: usize) -> Vec<ChangeInfo> {
1898+
if !CONFIG_CHANGE_HISTORY.iter().any(|config| config.change_id == current_id) {
18541899
// If the current change-id is greater than the most recent one, return
18551900
// an empty list (it may be due to switching from a recent branch to an
18561901
// older one); otherwise, return the full list (assuming the user provided
18571902
// the incorrect change-id by accident).
1858-
if let Some(max_id) = CONFIG_CHANGE_HISTORY.iter().max() {
1859-
if &current_id > max_id {
1903+
if let Some(config) = CONFIG_CHANGE_HISTORY.iter().max_by_key(|config| config.change_id) {
1904+
if &current_id > &config.change_id {
18601905
return Vec::new();
18611906
}
18621907
}
18631908

18641909
return CONFIG_CHANGE_HISTORY.to_vec();
18651910
}
18661911

1867-
let index = CONFIG_CHANGE_HISTORY.iter().position(|&id| id == current_id).unwrap();
1912+
let index =
1913+
CONFIG_CHANGE_HISTORY.iter().position(|config| config.change_id == current_id).unwrap();
18681914

18691915
CONFIG_CHANGE_HISTORY
18701916
.iter()

0 commit comments

Comments
 (0)