Skip to content

Commit

Permalink
Auto merge of rust-lang#117815 - onur-ozkan:update-change-tracking-im…
Browse files Browse the repository at this point in the history
…pl, 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`
  • Loading branch information
bors committed Nov 23, 2023
2 parents e7b2285 + 9a6afd0 commit 91f7f26
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 29 deletions.
46 changes: 30 additions & 16 deletions src/bootstrap/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -108,35 +111,46 @@ fn check_version(config: &Config) -> Option<String> {
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;
}

let change_links: Vec<String> = 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");

for link in change_links {
msg.push_str(&format!(" - {link}\n"));
if let Ok(last_warned_id) = fs::read_to_string(&warned_id_path) {
if id.to_string() == last_warned_id {
return None;
}
}

let changes = find_recent_config_change_ids(id);

msg.push_str("WARNING: there have been changes to x.py since you last updated.\n");
if !changes.is_empty() {
msg.push_str("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"
));

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");
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)
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/src/core/build_steps/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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\
Expand Down
70 changes: 58 additions & 12 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,61 @@ 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 {
/// 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,
}

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))
Expand Down Expand Up @@ -1849,22 +1894,23 @@ fn envify(s: &str) -> String {
.collect()
}

pub fn find_recent_config_change_ids(current_id: usize) -> Vec<usize> {
if !CONFIG_CHANGE_HISTORY.contains(&current_id) {
pub fn find_recent_config_change_ids(current_id: usize) -> Vec<ChangeInfo> {
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 &current_id > max_id {
if let Some(config) = CONFIG_CHANGE_HISTORY.iter().max_by_key(|config| config.change_id) {
if &current_id > &config.change_id {
return Vec::new();
}
}

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()
Expand Down

0 comments on commit 91f7f26

Please sign in to comment.