-
Notifications
You must be signed in to change notification settings - Fork 13.4k
rustbuild: Avoid some extraneous rustc compiles on cross builds #44143
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
When we pass `--host` the `self.hosts` array doesn't contain `self.build`, so check `self.build` to see if we can uplift.
src/bootstrap/doc.rs
Outdated
@@ -671,7 +671,7 @@ impl Step for ErrorIndex { | |||
|
|||
builder.ensure(compile::Rustc { | |||
compiler: builder.compiler(0, build.build), | |||
target, | |||
target: build.config.build, | |||
}); |
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.
ErrorIndex
depends on Rustc
already so this is useless; let's remove it.
@@ -718,7 +718,7 @@ impl Build { | |||
fn force_use_stage1(&self, compiler: Compiler, target: Interned<String>) -> bool { | |||
!self.config.full_bootstrap && | |||
compiler.stage >= 2 && | |||
self.hosts.iter().any(|h| *h == target) | |||
(self.hosts.iter().any(|h| *h == target) || target == self.build) |
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.
Hm, something is wrong if this is necessary. I believe we try to make the first host self.build
in config.rs
; could you check on that logic?
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.
Yeah I tried reordering this in config.rs
but then it ended up building too much b/c it's using config.host
to seed the matrix of what to build presumably? That is, when we do --host foo
, we only want the host binaries for foo
and not for the build triple. Do you think there's a better way to solve this though?
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.
Probably not then -- I think our current separation between target/host/build isn't really well thought out and probably various build steps build on the wrong platform (i.e., not optimal time-wise) today. I'm hoping to categorize the list of steps and make the distinction clearer.
All architectures use the same errors, no need to cross-compile a version only to not look at it.
a172f3f
to
1d70b66
Compare
Ok fixed up the first comment, re-r? @Mark-Simulacrum |
@bors r+ |
📌 Commit 1d70b66 has been approved by |
I'm too wussy to roll this up, but this should help us get rid of our queue |
rustbuild: Avoid some extraneous rustc compiles on cross builds This tweaks a few locations here and there to avoid compiling rustc too many times on our cross-builders on CI. Closes #44132
☀️ Test successful - status-appveyor, status-travis |
This tweaks a few locations here and there to avoid compiling rustc too many times on our cross-builders on CI.
Closes #44132