-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Move toolstate checking into bootstrap #66681
Move toolstate checking into bootstrap #66681
Conversation
This is preparation for #65000 -- but not the changes that the FCP there is about; that will be done in a separate, future PR. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
(CI failure is expected and due to the last commit which tests that we're still verifying submodules) |
☔ The latest upstream changes (presumably #66739) made this pull request unmergeable. Please resolve the merge conflicts. |
(If we're down with the approach the rebase and such here should be mostly painless) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One problematic area that I'd like to avoid but wasn't quite able to figure out how is the master branch script which is still in bash/python -- I cared less about that since it is orthogonal to the actual checking that we're doing, though as-is we're duplicating some code across Rust and that script.
I don't think it's that bad if we initialize the full CI environment in the master builder (it's like a couple of minutes?). Then we can just do ./x.py test publish-toolstate
(switching to run
once my GHA PR lands).
/// | ||
/// * See <https://help.github.com/articles/about-commit-email-addresses/> | ||
/// if a private email by GitHub is wanted. | ||
fn commit_toolstate_change( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we switch to use deploy keys instead? Dunno if you want to do this in this PR or another, but it's something I think we should do eventually. The code to push with a deploy key is here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather do this in a separate PR, seems cleaner.
c83a2fd
to
3e44c9c
Compare
Yeah, this seems reasonable. I would prefer that we land this and then do that in a followup PR -- I can work on it pretty quickly (mostly because I worry about changing too much in one PR, and the existing duplication is pretty minimal). I believe I've resolve the other review comments you left. |
@bors r+ rollup=never Thanks! |
📌 Commit 3e44c9c98b080561104dec7a797fe1901de43704 has been approved by |
⌛ Testing commit 3e44c9c98b080561104dec7a797fe1901de43704 with merge a51db9dafcc9ea5695bbde6cd14d0cdc20221b9f... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
This is not yet actually used by CI, but implements the logic for checking that tools are properly building on beta/stable and during beta cutoff week. This attempts to mirror the checking functionality in src/ci/docker/x86_64-gnu-tools/checktools.sh, and called scripts. It does not attempt to run the relevant steps (that functionality was originally desired to be moved into bootstrap as well, but doing so proved more difficult than expected). This is intended as a way to centralize and make clearer the logic involved in toolstate checking. In particular, the previous logic was spread across numerous python and shell scripts in such a way that made interpretation quite difficult.
3e44c9c
to
97d9364
Compare
@bors r=pietroalbini |
📌 Commit 97d9364 has been approved by |
…etroalbini Move toolstate checking into bootstrap This intends no functional changes, merely translates the spread of shell/python into Rust. One problematic area that I'd like to avoid but wasn't quite able to figure out how is the master branch script which is still in bash/python -- I cared less about that since it is orthogonal to the actual checking that we're doing, though as-is we're duplicating some code across Rust and that script. r? @kennytm or maybe @pietroalbini
☀️ Test successful - checks-azure |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This intends no functional changes, merely translates the spread of shell/python into Rust.
One problematic area that I'd like to avoid but wasn't quite able to figure out how is the master branch script which is still in bash/python -- I cared less about that since it is orthogonal to the actual checking that we're doing, though as-is we're duplicating some code across Rust and that script.
r? @kennytm or maybe @pietroalbini