Skip to content

Commit

Permalink
Rollup merge of #118220 - onur-ozkan:followups, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
general improvements/fixes on bootstrap

- adds #117813 into change tracker 6d9b92f
- fixes a bug in change tracker 63a4410
- relocates `CONFIG_CHANGE_HISTORY` a7dcb98
  • Loading branch information
fmease authored Nov 25, 2023
2 parents e2e978f + 576a17e commit fb2c498
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 92 deletions.
2 changes: 1 addition & 1 deletion config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
#
# If `change-id` does not match the version that is currently running,
# `x.py` will prompt you to update it and check the related PR for more details.
change-id = 116881
change-id = 117813

# =============================================================================
# Tweaking how LLVM is compiled
Expand Down
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
4 changes: 2 additions & 2 deletions src/bootstrap/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fn check_version(config: &Config) -> Option<String> {
}

if let Ok(last_warned_id) = fs::read_to_string(&warned_id_path) {
if id.to_string() == last_warned_id {
if latest_change_id.to_string() == last_warned_id {
return None;
}
}
Expand All @@ -144,7 +144,7 @@ fn check_version(config: &Config) -> Option<String> {
));

if io::stdout().is_terminal() {
t!(fs::write(warned_id_path, id.to_string()));
t!(fs::write(warned_id_path, latest_change_id.to_string()));
}
}
} else {
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
88 changes: 4 additions & 84 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,62 +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.",
},
];

/// 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 @@ -1895,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 fb2c498

Please sign in to comment.