Add a changelog for x.py and nag contributors until they read it #76626

jyn514 · 2020-09-12T03:01:15Z

Add a changelog for x.py

Add a changelog and instructions for updating it
Use changelog-seen in config.toml and VERSION in bootstrap to determine whether the changelog has been read. There's no way to tie reading the changelog to updating the version, so unfortunately they still have to update config.toml manually. Actually reading the changelog is optional, anyone can set changelog-seen = N without reading (although it's not recommended).
Nag people if they haven't read the x.py changelog
- Print message twice to make sure it's seen
Give different error messages depending on whether the version needs to be updated or added

jyn514 · 2020-09-12T03:04:22Z

Example output:

$ x.py check
Updating only changed submodules
Submodules updated in 0.02 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
warning: x.py has made several changes in the past month you may want to look at
help: consider looking at the changes in `src/bootstrap/CHANGELOG.md`
note: to silence this warning, add `version = 0` to `config.toml`
Checking std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Checking core v0.0.0 (/home/joshua/rustc/library/core)
^C  Building [==============>                                           ] 9/33: ...

config.toml.example

src/bootstrap/bin/main.rs

config.toml.example

src/bootstrap/CHANGELOG.md

config.toml.example

src/bootstrap/bin/main.rs

src/bootstrap/config.rs

pickfire

Looks good to me except the last thing mentioned.

Mark-Simulacrum

We should make sure that configure generates the appropriate version such that you don't immediately get nagged.

config.toml.example

src/bootstrap/CHANGELOG.md

Mark-Simulacrum · 2020-09-13T22:00:23Z

src/bootstrap/README.md

+* 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` (although they


Hm, what would an example of such a change be? It feels like if it affects no one, it should not be in the changelog...

Good point, I'll remove that bit.

Do you still think this should warn when a new configuration option is added? That's not a breaking change, although we still might want to tell people about it.

src/bootstrap/bin/main.rs

src/bootstrap/config.rs

pickfire

Looks good to me.

jyn514 · 2020-09-16T13:58:49Z

We should make sure that configure generates the appropriate version such that you don't immediately get nagged.

Missed this comment the first time, sorry. I'm not sure whether that's helpful - distro maintainers (cc @infinity0) will also want to know if x.py had major changes. If configure silently sets changelog-seen to the latest version, they won't see the difference.

If they have seen the changes, they can always pass --set version=1.

Mark-Simulacrum · 2020-09-16T14:01:37Z

Hm, I guess that's an interesting point. Probably given that we regularly generate hundreds of log lines anyway a warning at the beginning doesn't matter too much...

jyn514 · 2020-09-16T14:04:30Z

We could add another option to make the warning fatal? 😆

jyn514 · 2020-09-16T14:05:21Z

given that we regularly generate hundreds of log lines anyway

Might be useful to look into reducing that ... but that's separate from this change.

jyn514 · 2020-09-20T15:11:20Z

ping @Mark-Simulacrum - is this waiting on anything from me? Did you want me to implement the (optional) hard error if the versions don't match?

src/bootstrap/README.md

Mark-Simulacrum · 2020-09-20T20:17:17Z

src/bootstrap/bin/main.rs

@@ -12,5 +12,26 @@ use bootstrap::{Build, Config};
 fn main() {
    let args = env::args().skip(1).collect::<Vec<_>>();
    let config = Config::parse(&args);
+    check_version(&config);


Can we re-print this at the end of the build? I worry that in the not atypical case of quick screen-filling output the helpful message will otherwise get lost. And in cases where very little output is given, it seems not horrible to have a couple extra lines, maybe with some kind of prefix to make it clear it's a repeat.

I changed this to print twice, but because rustbuild uses std::process::exit, it won't get printed if an error occurs.

Successful message:

$ x.py test tidy warning: x.py has made several changes recently you may want to look at help: consider looking at the changes in `src/bootstrap/CHANGELOG.md` note: to silence this warning, add `changelog-seen = 1` to `config.toml` Building stage0 tool tidy (x86_64-unknown-linux-gnu) Finished release [optimized + debuginfo] target(s) in 0.08s tidy check * 617 error codes * highest error code: E0774 Checking which error codes lack tests... Found 430 error codes Found 0 error codes with no tests Done! * 298 features fmt check skip untracked path src/bootstrap/defaults/ during rustfmt invocations skip untracked path src/tmp.txt during rustfmt invocations skip untracked path tmp.rs during rustfmt invocations skip untracked path x.rs during rustfmt invocations skip untracked path y.rs during rustfmt invocations warning: x.py has made several changes recently you may want to look at help: consider looking at the changes in `src/bootstrap/CHANGELOG.md` note: to silence this warning, add `changelog-seen = 1` to `config.toml` note: this message was printed twice to make it more likely to be seen Build completed successfully in 0:00:06

Message on error:

warning: x.py has made several changes recently you may want to look at help: consider looking at the changes in `src/bootstrap/CHANGELOG.md` note: to silence this warning, add `changelog-seen = 1` to `config.toml` Building stage0 tool tidy (x86_64-unknown-linux-gnu) Finished release [optimized + debuginfo] target(s) in 0.09s tidy check * 617 error codes * highest error code: E0774 Checking which error codes lack tests... Found 430 error codes Found 0 error codes with no tests Done! tidy error: /home/joshua/rustc/src/bootstrap/bin/main.rs:6: trailing whitespace some tidy checks failed command did not execute successfully: "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/home/joshua/rustc" "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" expected success, got: exit code: 1 failed to run: /home/joshua/rustc/build/bootstrap/debug/bootstrap test tidy Build completed unsuccessfully in 0:00:03

Maybe it could be moved to bootstrap.py or x.py instead?

Hm let's avoid that for now.

Mark-Simulacrum · 2020-09-20T20:18:31Z

Okay, I would appreciate:

Squashing the commits down
Updating the PR description, looks like some stale notes of version still

Also left a few nits/suggestions.

bors · 2020-09-20T21:02:17Z

📌 Commit 8e10905 has been approved by Mark-Simulacrum

@Mark-Simulacrum

…acrum Add a changelog for x.py and nag contributors until they read it Add a changelog for x.py - Add a changelog and instructions for updating it - Use `changelog-seen` in `config.toml` and `VERSION` in bootstrap to determine whether the changelog has been read. There's no way to tie reading the changelog to updating the version, so unfortunately they still have to update `config.toml` manually. Actually reading the changelog is optional, anyone can set `changelog-seen = N` without reading (although it's not recommended). - Nag people if they haven't read the x.py changelog + Print message twice to make sure it's seen - Give different error messages depending on whether the version needs to be updated or added Closes rust-lang#76617 r? @Mark-Simulacrum

Dylan-DPC-zz · 2020-09-21T00:54:57Z

failed in rollup

Dylan-DPC-zz · 2020-09-21T00:55:05Z

@bors r-

jyn514 · 2020-09-21T01:04:56Z

@Dylan-DPC it's a merge conflict with #76628, I can't fix it until that PR's merged.

jyn514 · 2020-09-21T13:03:34Z

@bors r=Mark-Simulacrum

bors · 2020-09-21T13:03:35Z

📌 Commit ee1b6dada962a2f7cb2c8f1b55b83c4a17e43077 has been approved by Mark-Simulacrum

- Add a changelog and instructions for updating it - Use `changelog-seen` in `config.toml` and `VERSION` in bootstrap to determine whether the changelog has been read - Nag people if they haven't read the x.py changelog + Print message twice to make sure it's seen - Give different error messages depending on whether the version needs to be updated or added

jyn514 · 2020-09-21T13:09:02Z

@bors r=Mark-Simulacrum

Forgot to add #76628 to the changelog.

bors · 2020-09-21T13:09:03Z

📌 Commit fe6fc55 has been approved by Mark-Simulacrum

@Mark-Simulacrum

…acrum Add a changelog for x.py and nag contributors until they read it Add a changelog for x.py - Add a changelog and instructions for updating it - Use `changelog-seen` in `config.toml` and `VERSION` in bootstrap to determine whether the changelog has been read. There's no way to tie reading the changelog to updating the version, so unfortunately they still have to update `config.toml` manually. Actually reading the changelog is optional, anyone can set `changelog-seen = N` without reading (although it's not recommended). - Nag people if they haven't read the x.py changelog + Print message twice to make sure it's seen - Give different error messages depending on whether the version needs to be updated or added Closes rust-lang#76617 r? @Mark-Simulacrum

bors · 2020-09-22T10:36:47Z

⌛ Testing commit fe6fc55 with merge cbc5e4d...

bors · 2020-09-22T13:01:22Z

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing cbc5e4d to master...

infinity0 · 2020-10-02T00:11:46Z

@jyn514 I just noticed this, sorry I didn't find time to reply to your other messages. Thanks for pinging, this will be fine. Moving forward the changelog ought to omit minor things (that don't affect external behaviour) otherwise it would be redundant wrt git log.

jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 12, 2020

rust-highfive assigned Mark-Simulacrum Sep 12, 2020

This comment has been minimized.

Sign in to view

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 12, 2020

jyn514 force-pushed the x.py-changelog branch from 185aef5 to 7f7afb1 Compare September 12, 2020 03:02