Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve bootstrap change-tracking system #117815

Merged
merged 5 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
));
Comment on lines +134 to +138
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change results this format:

Building bootstrap
   Compiling bootstrap v0.0.0 (/home/nimda/devspace/onur-ozkan/rust/src/bootstrap)
    Finished dev [unoptimized] target(s) in 4.85s
There have been changes to x.py since you last updated:
  [INFO] New option `rust.parallel-compiler` added to config.toml.
    - PR Link https://github.com/rust-lang/rust/pull/117435
  [INFO] Default value of `download-ci-llvm` was changed for `codegen` profile.
    - PR Link https://github.com/rust-lang/rust/pull/116881
note: to silence this warning, update `config.toml` to use `change-id = 116881` instead
Building stage0 tool tidy (x86_64-unknown-linux-gnu)

}

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,
Kobzol marked this conversation as resolved.
Show resolved Hide resolved
/// 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
Loading