-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Consolidate staging for rustc_private
tools
#144303
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
base: master
Are you sure you want to change the base?
Conversation
|
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #143412) made this pull request unmergeable. Please resolve the merge conflicts. |
To clarify what it does.
2e63ce8
to
7bf4a1a
Compare
Will start looking at this today, might take me some time to get through. |
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, looks good overall. You can r=me after comment nit.
[build] rustc 0 <host> -> rustc_codegen_cranelift 1 <host> | ||
[build] rustc 1 <host> -> std 1 <host> | ||
[build] rustc 1 <host> -> std 1 <host> | ||
[build] rustc 1 <host> -> rustc 2 <host> | ||
[build] rustc 1 <host> -> rustc_codegen_cranelift 2 <host> |
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.
Discussion (non-blocking): we still have the problem where cg backends fit "weirdly" into builds, right?
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.
Yes, we should probably overhaul the way we manage and apply codegen backends.
@rustbot author |
@bors r=jieyouxu |
This PR continues bootstrap refactoring, this time by consolidating staging for
Mode::ToolRustc
tools. This refactoring was in the critical path of refactoringtest
/dist
/clippy
/doc
steps, and getting rid of the rmeta/rlib sysroot copy, because tools are pervasive and they are being used for a lot of things in bootstrap.The main idea is to explicitly model the fact that a stage N
Mode::ToolRustc
tool always works with two different compilers:build_compiler
) builds stage N rustc (target_compiler
)Before, the code often used
compiler
, which meant sometimes the build compiler, sometimes the target compiler, and sometimes neither (looking at you,download-rustc
). This is especially annoying when you get to a situation where you have an install step that invokes a dist step that invokes a tool build step, where some compiler is being propagated through, without it being clear what does that compiler represent. This refactoring hopefully makes that clearer and more explicit. It also gets rid of a fewbuilder.ensure(Rustc(...))
calls within bootstrap, which is always nice.Rustdoc
needs to be handled a bit specially, because it acts as a compiler itself, I documented that in the changes.It wasn't practical to do these refactorings in multiple PRs, so I did it all in one PR. The meat of the change is 9ee6d1c.
I tested manually that
x build rustdoc
andx build miri
still works even withdownload-rustc
, although I cannot promise any extra support fordownload-rustc
, IMO we will just have to reimplement it from scratch in a different way.As usually, I did some drive-by refactorings to bootstrap, trying to document and clarify things, add more step metadata and tests.
Best reviewed commit-by-commit (note that I renamed
link_compiler
totarget_compiler
, in accordance to the rest of bootstrap, in the last commit).r? @jieyouxu