-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Run rustc for information fewer times
#5249
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
|
r? @matklad (rust_highfive has picked a reviewer for you, use r? to override) |
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, interesting, so the previous logic kinda handled this in the case where target was not passed at all....
Perhaps add a helper Context::is_cross_compiling which handles both "no target" and "explicit target the same as the host", so that we don't duplicated this logic incorrectly elsewhere?
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'm gonna hold off on this suggestion for now because the deduction here is different than Cargo's historical deduction of "am I cross compiling?" (aka is --target passed?). I'd hope though we could improve this in the future!
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.
Oh, actually this side-effectual mutating API looks horrible to me :) Do you feel like refactoring it to
self.host_info = self.probe_target_info_kind(Kind::Host);
self.target_info = if host_target_same {
self.host_info.clone()
} else {
self.probe_target_info_kind(Kind::Target)
}?
|
Hm, what about normalizing and will build everything twice, because the command line and thus the fingerprints would differ. |
Currently if you pass `--target` the same as rustc's host Cargo will run `rustc` on extra time to learn information, but we only need to run it once to learn such information!
410adfa to
0eb7edf
Compare
|
Updated! Leaving out the normalization for now as I think it runs the risk for breakage, but we can fix that later for sure I think. |
|
@bors r+ |
|
📌 Commit 0eb7edf has been approved by |
Run `rustc` for information fewer times Currently if you pass `--target` the same as rustc's host Cargo will run `rustc` on extra time to learn information, but we only need to run it once to learn such information!
|
☀️ Test successful - status-appveyor, status-travis |
One hard cs problem Closes #5313 r? @alexcrichton Note that, due to the first commit, this still gives us all the benefits of #5249: if RUSTFLAGS is empty, we will run only a single rustc process, even if we can't cache it across different cargo invocations.
Currently if you pass
--targetthe same as rustc's host Cargo will runrustcon extra time to learn information, but we only need to run it once to learn
such information!