Skip to content

Commit

Permalink
move CONFIG_CHANGE_HISTORY to its own module
Browse files Browse the repository at this point in the history
Because bootstrap lib is already large and complicated, this should
make the "bumping change-id" process easier.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
  • Loading branch information
onur-ozkan committed Nov 24, 2023
1 parent 38aba2c commit 576a17e
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 94 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/bootstrap/src/core/build_steps/setup.rs
Original file line number Diff line number Diff line change
@@ -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 _;
Expand Down
93 changes: 4 additions & 89 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,10 @@ use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, syml
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
Expand All @@ -70,67 +71,6 @@ 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.
///
/// 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<Mode>, &str, Option<&[&'static str]>)] = &[
Expand Down Expand Up @@ -1900,31 +1840,6 @@ fn envify(s: &str) -> String {
.collect()
}

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(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(|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
Expand Down
89 changes: 89 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
@@ -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<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(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(|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.",
},
];
1 change: 1 addition & 0 deletions src/bootstrap/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 15 additions & 3 deletions triagebot.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down

0 comments on commit 576a17e

Please sign in to comment.