Skip to content

Commit 1903c98

Browse files
committed
refactor change-tracking implementation
Signed-off-by: onur-ozkan <[email protected]>
1 parent feff24e commit 1903c98

File tree

3 files changed

+72
-27
lines changed

3 files changed

+72
-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
@@ -218,7 +218,7 @@ fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) {
218218
crate::exit!(1);
219219
}
220220

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

src/bootstrap/src/lib.rs

+57-11
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 ConfigChange {
74+
/// Represents the ID of PR caused major change on bootstrap.
75+
pub change_id: usize,
76+
pub severity: Severity,
77+
/// Provides a short summary of the change that will guide developers
78+
/// on "how to handle/behave" in response to the changes.
79+
///
80+
/// This should contain exclusive and concise information.
81+
pub summary: &'static str,
82+
}
83+
84+
#[derive(Clone, Debug)]
85+
pub enum Severity {
86+
Info,
87+
Warning,
88+
}
89+
90+
impl ToString for Severity {
91+
fn to_string(&self) -> String {
92+
match self {
93+
Severity::Info => "INFO".to_string(),
94+
Severity::Warning => "WARNING".to_string(),
95+
}
96+
}
97+
}
98+
7299
/// Keeps track of major changes made to the bootstrap configuration.
73100
///
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.
77101
///
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 `ConfigChange` to the end(because the list must be sorted by the merge date)
104+
/// of this list.
105+
pub const CONFIG_CHANGE_HISTORY: &[ConfigChange] = &[
106+
ConfigChange {
107+
change_id: 115898,
108+
severity: Severity::Info,
109+
summary: "Implementation of this change-tracking system. Ignore this.",
110+
},
111+
ConfigChange {
112+
change_id: 116998,
113+
severity: Severity::Info,
114+
summary: "Removed android-ndk r15 support in favor of android-ndk r25b.",
115+
},
116+
ConfigChange {
117+
change_id: 117435,
118+
severity: Severity::Info,
119+
summary: "New option `rust.parallel-compiler` added to config.toml.",
120+
},
121+
ConfigChange {
122+
change_id: 116881,
123+
severity: Severity::Info,
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<ConfigChange> {
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)