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

general improvements/fixes on bootstrap #118220

Merged
merged 3 commits into from
Nov 25, 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
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
Loading