-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Consolidate stage directories and group logs in bootstrap #145295
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
Conversation
Now stageN-X corresponds to stage N X, as it should.
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.
Looks good, tiny nits, r=me after.
@rustbot author |
Resolved review remarks. I would bump the priority of this, because it will be very conflict-prone with anything done in bootstrap. |
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.
Thanks
@bors r+ rollup=5 (conflicts with other bootstrap work, conflict-prone) |
Aaa |
Consolidate stage directories and group logs in bootstrap My post-stage-0-redesign bootstrap fixes aren't done yet, but I think that enough steps have been migrated to the new system that it makes sense to actually modify the directories on disk, and what gets printed when bootstrap runs, so that it actually corresponds to the new system. Before, the printed stages didn't always make sense. This PR: - Fixes the numbering of `stageN` directories in the build directory. It was not corresponding to the correct stages before; notice that I did not modify `bootstrap/README.md`, as it was essentially describing what happens after this PR (first commit). - Unifies all steps that output a build group to use the `Builder::msg` method. It's probably not the final stage, and some of the test steps might not be fully accurate yet, because I didn't fix test step numbering yet, but I think that it's a clear improvement from before, and now that everything uses the same method, we can easily make changes across the board, to ensure that it stays unified (second commit). r? `@jieyouxu`
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes. |
@bors try |
This comment has been minimized.
This comment has been minimized.
Consolidate stage directories and group logs in bootstrap try-job: dist-x86_64-msvc try-job: dist-x86_64-linux
Fixes CI failure. @bors r=jieyouxu |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 8e62bfd (parent) -> 1553adf (this PR) Test differencesShow 4 test diffs4 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 1553adfe6884a8f6c28f5a673d3e605535ee0113 --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (1553adf): comparison URL. Overall result: ❌✅ regressions and improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 3.6%, secondary -3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 465.671s -> 464.093s (-0.34%) |
My post-stage-0-redesign bootstrap fixes aren't done yet, but I think that enough steps have been migrated to the new system that it makes sense to actually modify the directories on disk, and what gets printed when bootstrap runs, so that it actually corresponds to the new system. Before, the printed stages didn't always make sense.
This PR:
stageN
directories in the build directory. It was not corresponding to the correct stages before; notice that I did not modifybootstrap/README.md
, as it was essentially describing what happens after this PR (first commit). This means thatstageN-<foo>
now actually corresponds to<foo>
(e.g.rustc
ortools
) that is stage N. Before in some cases it means that it was built with the stage N compiler. So for examplestage0-rustc
becamestage1-rustc
.Builder::msg
method. It's probably not the final stage, and some of the test steps might not be fully accurate yet, because I didn't fix test step numbering yet, but I think that it's a clear improvement from before, and now that everything uses the same method, we can easily make changes across the board, to ensure that it stays unified (second commit).r? @jieyouxu
try-job: dist-x86_64-msvc
try-job: dist-x86_64-linux