From 576a17ecdbd30ed427c25343942f43185ebc6d98 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 24 Nov 2023 00:18:57 +0300 Subject: [PATCH] move CONFIG_CHANGE_HISTORY to its own module Because bootstrap lib is already large and complicated, this should make the "bumping change-id" process easier. Signed-off-by: onur-ozkan --- src/bootstrap/README.md | 2 +- src/bootstrap/src/core/build_steps/setup.rs | 3 +- src/bootstrap/src/lib.rs | 93 +-------------------- src/bootstrap/src/utils/change_tracker.rs | 89 ++++++++++++++++++++ src/bootstrap/src/utils/mod.rs | 1 + triagebot.toml | 18 +++- 6 files changed, 112 insertions(+), 94 deletions(-) create mode 100644 src/bootstrap/src/utils/change_tracker.rs diff --git a/src/bootstrap/README.md b/src/bootstrap/README.md index e7998a40a65..d0a069f45e1 100644 --- a/src/bootstrap/README.md +++ b/src/bootstrap/README.md @@ -183,7 +183,7 @@ Some general areas that you may be interested in modifying are: If you make a major change on bootstrap configuration, please remember to: -+ Update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/lib.rs`. ++ Update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/src/utils/change_tracker.rs`. * Update `change-id = {pull-request-id}` in `config.example.toml`. A 'major change' includes diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index bbbdb4c3186..49373177abe 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -1,6 +1,7 @@ use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; +use crate::t; +use crate::utils::change_tracker::CONFIG_CHANGE_HISTORY; use crate::Config; -use crate::{t, CONFIG_CHANGE_HISTORY}; use sha2::Digest; use std::env::consts::EXE_SUFFIX; use std::fmt::Write as _; diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 785a293b45a..480dd757915 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -47,9 +47,10 @@ mod core; mod utils; -pub use crate::core::builder::PathSet; -pub use crate::core::config::flags::Subcommand; -pub use crate::core::config::Config; +pub use core::builder::PathSet; +pub use core::config::flags::Subcommand; +pub use core::config::Config; +pub use utils::change_tracker::{find_recent_config_change_ids, CONFIG_CHANGE_HISTORY}; const LLVM_TOOLS: &[&str] = &[ "llvm-cov", // used to generate coverage report @@ -70,67 +71,6 @@ /// 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. -/// -/// If you make any major changes (such as adding new values or changing default values), -/// 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.", - }, - ChangeInfo { - change_id: 117813, - severity: ChangeSeverity::Info, - summary: "Use of the `if-available` value for `download-ci-llvm` is deprecated; prefer using the new `if-unchanged` value.", - }, -]; - /// Extra --check-cfg to add when building /// (Mode restriction, config name, config values (if any)) const EXTRA_CHECK_CFGS: &[(Option, &str, Option<&[&'static str]>)] = &[ @@ -1900,31 +1840,6 @@ fn envify(s: &str) -> String { .collect() } -pub fn find_recent_config_change_ids(current_id: usize) -> Vec { - 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(config) = CONFIG_CHANGE_HISTORY.iter().max_by_key(|config| config.change_id) { - if ¤t_id > &config.change_id { - return Vec::new(); - } - } - - return CONFIG_CHANGE_HISTORY.to_vec(); - } - - let index = - CONFIG_CHANGE_HISTORY.iter().position(|config| config.change_id == current_id).unwrap(); - - CONFIG_CHANGE_HISTORY - .iter() - .skip(index + 1) // Skip the current_id and IDs before it - .cloned() - .collect() -} - /// Computes a hash representing the state of a repository/submodule and additional input. /// /// It uses `git diff` for the actual changes, and `git status` for including the untracked diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs new file mode 100644 index 00000000000..887b596b786 --- /dev/null +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -0,0 +1,89 @@ +//! This module facilitates the tracking system for major changes made to the bootstrap, +//! with the goal of keeping developers synchronized with important modifications in +//! the bootstrap. + +#[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(), + } + } +} + +pub fn find_recent_config_change_ids(current_id: usize) -> Vec { + 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(config) = CONFIG_CHANGE_HISTORY.iter().max_by_key(|config| config.change_id) { + if ¤t_id > &config.change_id { + return Vec::new(); + } + } + + return CONFIG_CHANGE_HISTORY.to_vec(); + } + + let index = + CONFIG_CHANGE_HISTORY.iter().position(|config| config.change_id == current_id).unwrap(); + + CONFIG_CHANGE_HISTORY + .iter() + .skip(index + 1) // Skip the current_id and IDs before it + .cloned() + .collect() +} + +/// Keeps track of major changes made to the bootstrap configuration. +/// +/// If you make any major changes (such as adding new values or changing default values), +/// 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.", + }, + ChangeInfo { + change_id: 117813, + severity: ChangeSeverity::Info, + summary: "Use of the `if-available` value for `download-ci-llvm` is deprecated; prefer using the new `if-unchanged` value.", + }, +]; diff --git a/src/bootstrap/src/utils/mod.rs b/src/bootstrap/src/utils/mod.rs index 8ca22d00865..cb535f0e163 100644 --- a/src/bootstrap/src/utils/mod.rs +++ b/src/bootstrap/src/utils/mod.rs @@ -4,6 +4,7 @@ pub(crate) mod cache; pub(crate) mod cc_detect; +pub(crate) mod change_tracker; pub(crate) mod channel; pub(crate) mod dylib; pub(crate) mod exec; diff --git a/triagebot.toml b/triagebot.toml index a72338d1950..bfb9ab2a792 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -583,11 +583,23 @@ message = "The list of allowed third-party dependencies may have been modified! cc = ["@davidtwco", "@wesleywiser"] [mentions."src/bootstrap/src/core/config"] -message = "This PR modifies `src/bootstrap/src/core/config`. If appropriate, please also update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/src/lib.rs` and `change-id` in `config.example.toml`." +message = """ +This PR modifies `src/bootstrap/src/core/config`. + +If appropriate, please update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/src/utils/change_tracker.rs` and `change-id` in `config.example.toml`. +""" [mentions."src/bootstrap/defaults"] -message = "This PR modifies `src/bootstrap/defaults`. If appropriate, please also update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/src/lib.rs` and `change-id` in `config.example.toml`." +message = """ +This PR modifies `src/bootstrap/defaults`. + +If appropriate, please update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/src/utils/change_tracker.rs` and `change-id` in `config.example.toml`. +""" [mentions."config.example.toml"] -message = "This PR changes `config.example.toml`. If appropriate, please also update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/src/lib.rs` and `change-id` in `config.example.toml`." +message = """ +This PR modifies `config.example.toml`. + +If appropriate, please update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/src/utils/change_tracker.rs` and `change-id` in `config.example.toml`. +""" [mentions."src/bootstrap/defaults/config.compiler.toml"] message = "This PR changes src/bootstrap/defaults/config.compiler.toml. If appropriate, please also update `config.codegen.toml` so the defaults are in sync."