Skip to content

Commit

Permalink
Auto merge of #115898 - onur-ozkan:config-change-tracking, r=Mark-Sim…
Browse files Browse the repository at this point in the history
…ulacrum

bootstrap major change detection implementation

The use of `changelog-seen` and `bootstrap/CHANGELOG.md` has not been functional in any way for many years. We often do major/breaking changes but never update the changelog file or the `changelog-seen`. This is an alternative method for tracking major or breaking changes and informing developers when such changes occur.

Example output when bootstrap detects a major change:
![image](https://github.com/rust-lang/rust/assets/39852038/ee802dfa-a02b-488b-a433-f853ce079b8a)
  • Loading branch information
bors committed Oct 2, 2023
2 parents 1578329 + 7d3dcd9 commit 781ebbe
Show file tree
Hide file tree
Showing 16 changed files with 109 additions and 113 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ index d95b5b7f17f..00b6f0e3635 100644
EOF

cat > config.toml <<EOF
changelog-seen = 2
change-id = 115898
[llvm]
ninja = false
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ function setup_rustc() {
rm config.toml || true

cat > config.toml <<EOF
changelog-seen = 2
change-id = 115898
[rust]
codegen-backends = []
Expand Down
17 changes: 12 additions & 5 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,18 @@
# Note that this has no default value (x.py uses the defaults in `config.example.toml`).
#profile = <none>

# Keeps track of the last version of `x.py` used.
# If `changelog-seen` does not match the version that is currently running,
# `x.py` will prompt you to update it and to read the changelog.
# See `src/bootstrap/CHANGELOG.md` for more information.
changelog-seen = 2
# Keeps track of major changes made to this configuration.
#
# This value also represents ID of the PR that caused major changes. Meaning,
# you can visit github.com/rust-lang/rust/pull/{change-id} to check for more details.
#
# A 'major change' includes any of the following
# - A new option
# - A change in the default values
#
# 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 = 115898

# =============================================================================
# Tweaking how LLVM is compiled
Expand Down
71 changes: 0 additions & 71 deletions src/bootstrap/CHANGELOG.md

This file was deleted.

14 changes: 9 additions & 5 deletions src/bootstrap/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,23 +181,27 @@ Some general areas that you may be interested in modifying are:
`Config` struct.
* Adding a sanity check? Take a look at `bootstrap/sanity.rs`.

If you make a major change, please remember to:
If you make a major change on bootstrap configuration, please remember to:

+ Update `VERSION` in `src/bootstrap/main.rs`.
* Update `changelog-seen = N` in `config.example.toml`.
* Add an entry in `src/bootstrap/CHANGELOG.md`.
+ Update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/lib.rs`.
* Update `change-id = {pull-request-id}` in `config.example.toml`.

A 'major change' includes

* A new option or
* A change in the default options.

Changes that do not affect contributors to the compiler or users
building rustc from source don't need an update to `VERSION`.
building rustc from source don't need an update to `CONFIG_CHANGE_HISTORY`.

If you have any questions, feel free to reach out on the `#t-infra/bootstrap` channel
at [Rust Bootstrap Zulip server][rust-bootstrap-zulip]. When you encounter bugs,
please file issues on the [Rust issue tracker][rust-issue-tracker].

[rust-bootstrap-zulip]: https://rust-lang.zulipchat.com/#narrow/stream/t-infra.2Fbootstrap
[rust-issue-tracker]: https://github.com/rust-lang/rust/issues

## Changelog

Because we do not release bootstrap with versions, we also do not maintain CHANGELOG files. To
review the changes made to bootstrap, simply run `git log --no-merges --oneline -- src/bootstrap`.
41 changes: 28 additions & 13 deletions src/bootstrap/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::{env, fs};

#[cfg(all(any(unix, windows), not(target_os = "solaris")))]
use bootstrap::t;
use bootstrap::{Build, Config, Subcommand, VERSION};
use bootstrap::{find_recent_config_change_ids, Build, Config, Subcommand, CONFIG_CHANGE_HISTORY};

fn main() {
let args = env::args().skip(1).collect::<Vec<_>>();
Expand Down Expand Up @@ -42,7 +42,7 @@ fn main() {
}
err => {
drop(err);
println!("warning: build directory locked by process {pid}, waiting for lock");
println!("WARNING: build directory locked by process {pid}, waiting for lock");
let mut lock = t!(build_lock.write());
t!(lock.write(&process::id().to_string().as_ref()));
lock
Expand All @@ -51,7 +51,7 @@ fn main() {
}

#[cfg(any(not(any(unix, windows)), target_os = "solaris"))]
println!("warning: file locking not supported for target, not locking build directory");
println!("WARNING: file locking not supported for target, not locking build directory");

// check_version warnings are not printed during setup
let changelog_suggestion =
Expand All @@ -61,7 +61,7 @@ fn main() {
// changelog warning, not the `x.py setup` message.
let suggest_setup = config.config.is_none() && !matches!(config.cmd, Subcommand::Setup { .. });
if suggest_setup {
println!("warning: you have not made a `config.toml`");
println!("WARNING: you have not made a `config.toml`");
println!(
"help: consider running `./x.py setup` or copying `config.example.toml` by running \
`cp config.example.toml config.toml`"
Expand All @@ -74,7 +74,7 @@ fn main() {
Build::new(config).build();

if suggest_setup {
println!("warning: you have not made a `config.toml`");
println!("WARNING: you have not made a `config.toml`");
println!(
"help: consider running `./x.py setup` or copying `config.example.toml` by running \
`cp config.example.toml config.toml`"
Expand All @@ -91,7 +91,7 @@ fn main() {
contents.contains("https://github.com/rust-lang/rust/issues/77620#issuecomment-705144570")
}) {
println!(
"warning: You have the pre-push script installed to .git/hooks/pre-commit. \
"WARNING: You have the pre-push script installed to .git/hooks/pre-commit. \
Consider moving it to .git/hooks/pre-push instead, which runs less often."
);
}
Expand All @@ -104,19 +104,34 @@ fn main() {
fn check_version(config: &Config) -> Option<String> {
let mut msg = String::new();

let suggestion = if let Some(seen) = config.changelog_seen {
if seen != VERSION {
msg.push_str("warning: there have been changes to x.py since you last updated.\n");
format!("update `config.toml` to use `changelog-seen = {VERSION}` instead")
if config.changelog_seen.is_some() {
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 suggestion = if let Some(id) = config.change_id {
if &id != latest_config_id {
msg.push_str("WARNING: there have been changes to x.py since you last updated.\n");
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("To see more detail about these changes, visit the following PRs:\n");
for link in change_links {
msg.push_str(&format!(" - {link}\n"));
}
}
msg.push_str("WARNING: there have been changes to x.py since you last updated.\n");
format!("update `config.toml` to use `change-id = {latest_config_id}` instead")
} else {
return None;
}
} else {
msg.push_str("warning: x.py has made several changes recently you may want to look at\n");
format!("add `changelog-seen = {VERSION}` at the top of `config.toml`")
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");
format!("add `change-id = {latest_config_id}` at the top of `config.toml`")
};

msg.push_str("help: consider looking at the changes in `src/bootstrap/CHANGELOG.md`\n");
msg.push_str("note: to silence this warning, ");
msg.push_str(&suggestion);

Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/bootstrap_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class GenerateAndParseConfig(unittest.TestCase):
"""Test that we can serialize and deserialize a config.toml file"""
def test_no_args(self):
build = serialize_and_parse([])
self.assertEqual(build.get_toml("changelog-seen"), '2')
self.assertEqual(build.get_toml("change-id"), '115898')
self.assertEqual(build.get_toml("profile"), 'dist')
self.assertIsNone(build.get_toml("llvm.download-ci-llvm"))

Expand Down
20 changes: 17 additions & 3 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ impl Display for DebuginfoLevel {
/// `config.example.toml`.
#[derive(Default, Clone)]
pub struct Config {
pub changelog_seen: Option<usize>,
pub changelog_seen: Option<usize>, // FIXME: Deprecated field. Remove it at 2024.
pub change_id: Option<usize>,
pub ccache: Option<String>,
/// Call Build::ninja() instead of this.
pub ninja_in_file: bool,
Expand Down Expand Up @@ -546,7 +547,8 @@ impl Target {
#[derive(Deserialize, Default)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct TomlConfig {
changelog_seen: Option<usize>,
changelog_seen: Option<usize>, // FIXME: Deprecated field. Remove it at 2024.
change_id: Option<usize>,
build: Option<Build>,
install: Option<Install>,
llvm: Option<Llvm>,
Expand Down Expand Up @@ -574,7 +576,17 @@ trait Merge {
impl Merge for TomlConfig {
fn merge(
&mut self,
TomlConfig { build, install, llvm, rust, dist, target, profile: _, changelog_seen }: Self,
TomlConfig {
build,
install,
llvm,
rust,
dist,
target,
profile: _,
changelog_seen,
change_id,
}: Self,
replace: ReplaceOpt,
) {
fn do_merge<T: Merge>(x: &mut Option<T>, y: Option<T>, replace: ReplaceOpt) {
Expand All @@ -587,6 +599,7 @@ impl Merge for TomlConfig {
}
}
self.changelog_seen.merge(changelog_seen, replace);
self.change_id.merge(change_id, replace);
do_merge(&mut self.build, build, replace);
do_merge(&mut self.install, install, replace);
do_merge(&mut self.llvm, llvm, replace);
Expand Down Expand Up @@ -1242,6 +1255,7 @@ impl Config {
toml.merge(override_toml, ReplaceOpt::Override);

config.changelog_seen = toml.changelog_seen;
config.change_id = toml.change_id;

let build = toml.build.unwrap_or_default();
if let Some(file_build) = build.build {
Expand Down
12 changes: 6 additions & 6 deletions src/bootstrap/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ fn override_toml() {
&[
"check".to_owned(),
"--config=/does/not/exist".to_owned(),
"--set=changelog-seen=1".to_owned(),
"--set=change-id=1".to_owned(),
"--set=rust.lto=fat".to_owned(),
"--set=rust.deny-warnings=false".to_owned(),
"--set=build.gdb=\"bar\"".to_owned(),
Expand All @@ -112,7 +112,7 @@ fn override_toml() {
|&_| {
toml::from_str(
r#"
changelog-seen = 0
change-id = 0
[rust]
lto = "off"
deny-warnings = true
Expand All @@ -129,7 +129,7 @@ build-config = {}
.unwrap()
},
);
assert_eq!(config.changelog_seen, Some(1), "setting top-level value");
assert_eq!(config.change_id, Some(1), "setting top-level value");
assert_eq!(
config.rust_lto,
crate::config::RustcLto::Fat,
Expand All @@ -156,10 +156,10 @@ fn override_toml_duplicate() {
&[
"check".to_owned(),
"--config=/does/not/exist".to_owned(),
"--set=changelog-seen=1".to_owned(),
"--set=changelog-seen=2".to_owned(),
"--set=change-id=1".to_owned(),
"--set=change-id=2".to_owned(),
],
|&_| toml::from_str("changelog-seen = 0").unwrap(),
|&_| toml::from_str("change-id = 0").unwrap(),
);
}

Expand Down
23 changes: 22 additions & 1 deletion src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,15 @@ const LLVM_TOOLS: &[&str] = &[
/// LLD file names for all flavors.
const LLD_FILE_NAMES: &[&str] = &["ld.lld", "ld64.lld", "lld-link", "wasm-ld"];

pub const VERSION: usize = 2;
/// 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.
pub const CONFIG_CHANGE_HISTORY: &[usize] = &[115898];

/// Extra --check-cfg to add when building
/// (Mode restriction, config name, config values (if any))
Expand Down Expand Up @@ -1844,3 +1852,16 @@ fn envify(s: &str) -> String {
.flat_map(|c| c.to_uppercase())
.collect()
}

pub fn find_recent_config_change_ids(current_id: usize) -> Vec<usize> {
let index = CONFIG_CHANGE_HISTORY
.iter()
.position(|&id| id == current_id)
.expect(&format!("Value `{}` was not found in `CONFIG_CHANGE_HISTORY`.", current_id));

CONFIG_CHANGE_HISTORY
.iter()
.skip(index + 1) // Skip the current_id and IDs before it
.cloned()
.collect()
}
Loading

0 comments on commit 781ebbe

Please sign in to comment.