bootstrap major change detection implementation

[onur-ozkan]

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:

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

[Mark-Simulacrum]

r=me with it kept or removed

[Mark-Simulacrum]

@bors r+

[bors]

📌 Commit 7d3dcd9 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[bors]

⌛ Testing commit 7d3dcd9 with merge 781ebbe...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 781ebbe to master...

[rust-timer]

Finished benchmarking commit (781ebbe): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.9%	[2.9%, 2.9%]	1
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.6%	[-3.6%, -3.6%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 627.987s -> 626.682s (-0.21%)
Artifact size: 273.32 MiB -> 273.33 MiB (0.01%)

[RalfJung]

Doing this without a transition is a bit painful. I often switch between branches that are based on slightly different commits, and now that I updated to change-id, when I check out an old branch I cannot build it any more:

failed to parse TOML configuration 'config.toml': unknown field `change-id`, expected one of `changelog-seen`, `build`, `install`, `llvm`, `rust`, `target`, `dist`, `profile`

[Mark-Simulacrum]

You can just omit both and ignore the warnings. It might not be an amazing option.

[pietroalbini]

Yeah, this is fairly annoying when switching to other commits / versions :(

[pcc]

While I was preparing #116998 I noticed that the change-id is mentioned in several places that will need to be updated and not just the ones mentioned in the automatic comment (otherwise either some documentation will become stale or bootstrap tests will start failing). I expect that developers will notice the test failures, but they won't notice the stale documentation.

[onur-ozkan]

change-id shouldn't have any impact as it's solely used for logging versions in case more recent ones exist. It doesn't directly affect the configuration.

otherwise either some documentation will become stale or bootstrap tests will start failing

This shouldn't be possible.

[pcc]

Without the bootstrap test changes, the following test fails:

======================================================================
FAIL: test_no_args (bootstrap_test.GenerateAndParseConfig.test_no_args)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/disk2/pcc/rust/src/bootstrap/bootstrap_test.py", line 106, in test_no_args
    self.assertEqual(build.get_toml("change-id"), '115898')
AssertionError: '116998' != '115898'
- 116998
+ 115898

Regarding the stale documentation, maybe we should just remove the change-id from as many of these places as possible? Either way there will still be a warning and the user will have the option of adding a change-id if they want change tracking.

[onur-ozkan]

In https://rustc-dev-guide.rust-lang.org, sure, it would be better to remove change-id from sample configurations.

AssertionError: '116998' != '115898'

Removing this assertion shouldn't be problem I think.