Skip to content

Commit 12ab8c6

Browse files
committed
refactor change-tracking implementation
Signed-off-by: onur-ozkan <[email protected]>
1 parent 056d4e4 commit 12ab8c6

File tree

3 files changed

+70
-27
lines changed

3 files changed

+70
-27
lines changed

src/bootstrap/src/bin/main.rs

+14-15
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,11 @@ fn check_version(config: &Config) -> Option<String> {
108108
msg.push_str("WARNING: The use of `changelog-seen` is deprecated. Please refer to `change-id` option in `config.example.toml` instead.\n");
109109
}
110110

111-
let latest_config_id = CONFIG_CHANGE_HISTORY.last().unwrap();
111+
let latest_change_id = CONFIG_CHANGE_HISTORY.last().unwrap().change_id;
112112
let warned_id_path = config.out.join("bootstrap").join(".last-warned-change-id");
113113

114114
if let Some(id) = config.change_id {
115-
if &id == latest_config_id {
115+
if id == latest_change_id {
116116
return None;
117117
}
118118

@@ -122,31 +122,30 @@ fn check_version(config: &Config) -> Option<String> {
122122
}
123123
}
124124

125-
let change_links: Vec<String> = find_recent_config_change_ids(id)
126-
.iter()
127-
.map(|id| format!("https://github.com/rust-lang/rust/pull/{id}"))
128-
.collect();
129-
if !change_links.is_empty() {
130-
msg.push_str("WARNING: there have been changes to x.py since you last updated.\n");
131-
msg.push_str("To see more detail about these changes, visit the following PRs:\n");
125+
let changes = find_recent_config_change_ids(id);
132126

133-
for link in change_links {
134-
msg.push_str(&format!(" - {link}\n"));
135-
}
127+
if !changes.is_empty() {
128+
msg.push_str("There have been changes to x.py since you last updated:\n");
136129

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

139138
msg.push_str("NOTE: to silence this warning, ");
140139
msg.push_str(&format!(
141-
"update `config.toml` to use `change-id = {latest_config_id}` instead"
140+
"update `config.toml` to use `change-id = {latest_change_id}` instead"
142141
));
143142

144143
t!(fs::write(warned_id_path, id.to_string()));
145144
}
146145
} else {
147146
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");
148147
msg.push_str("NOTE: to silence this warning, ");
149-
msg.push_str(&format!("add `change-id = {latest_config_id}` at the top of `config.toml`"));
148+
msg.push_str(&format!("add `change-id = {latest_change_id}` at the top of `config.toml`"));
150149
};
151150

152151
Some(msg)

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\

src/bootstrap/src/lib.rs

+55-11
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,59 @@ 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+
Info,
85+
Warning,
86+
}
87+
88+
impl ToString for ChangeSeverity {
89+
fn to_string(&self) -> String {
90+
match self {
91+
ChangeSeverity::Info => "INFO".to_string(),
92+
ChangeSeverity::Warning => "WARNING".to_string(),
93+
}
94+
}
95+
}
96+
7297
/// Keeps track of major changes made to the bootstrap configuration.
7398
///
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.
7799
///
78100
/// 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];
101+
/// please ensure adding `ChangeInfo` to the end(because the list must be sorted by the merge date)
102+
/// of this list.
103+
pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
104+
ChangeInfo {
105+
change_id: 115898,
106+
severity: ChangeSeverity::Info,
107+
summary: "Implementation of this change-tracking system. Ignore this.",
108+
},
109+
ChangeInfo {
110+
change_id: 116998,
111+
severity: ChangeSeverity::Info,
112+
summary: "Removed android-ndk r15 support in favor of android-ndk r25b.",
113+
},
114+
ChangeInfo {
115+
change_id: 117435,
116+
severity: ChangeSeverity::Info,
117+
summary: "New option `rust.parallel-compiler` added to config.toml.",
118+
},
119+
ChangeInfo {
120+
change_id: 116881,
121+
severity: ChangeSeverity::Warning,
122+
summary: "Default value of `download-ci-llvm` was changed for `codegen` profile.",
123+
},
124+
];
82125

83126
/// Extra --check-cfg to add when building
84127
/// (Mode restriction, config name, config values (if any))
@@ -1849,22 +1892,23 @@ fn envify(s: &str) -> String {
18491892
.collect()
18501893
}
18511894

1852-
pub fn find_recent_config_change_ids(current_id: usize) -> Vec<usize> {
1853-
if !CONFIG_CHANGE_HISTORY.contains(&current_id) {
1895+
pub fn find_recent_config_change_ids(current_id: usize) -> Vec<ChangeInfo> {
1896+
if !CONFIG_CHANGE_HISTORY.iter().any(|config| config.change_id == current_id) {
18541897
// If the current change-id is greater than the most recent one, return
18551898
// an empty list (it may be due to switching from a recent branch to an
18561899
// older one); otherwise, return the full list (assuming the user provided
18571900
// the incorrect change-id by accident).
1858-
if let Some(max_id) = CONFIG_CHANGE_HISTORY.iter().max() {
1859-
if &current_id > max_id {
1901+
if let Some(config) = CONFIG_CHANGE_HISTORY.iter().max_by_key(|config| config.change_id) {
1902+
if &current_id > &config.change_id {
18601903
return Vec::new();
18611904
}
18621905
}
18631906

18641907
return CONFIG_CHANGE_HISTORY.to_vec();
18651908
}
18661909

1867-
let index = CONFIG_CHANGE_HISTORY.iter().position(|&id| id == current_id).unwrap();
1910+
let index =
1911+
CONFIG_CHANGE_HISTORY.iter().position(|config| config.change_id == current_id).unwrap();
18681912

18691913
CONFIG_CHANGE_HISTORY
18701914
.iter()

0 commit comments

Comments
 (0)