-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
rustbuild: allow overriding list of LLVM targets to build #38657
Conversation
A new option is introduced under the `[llvm]` section of `config.toml`, `targets`, for overriding the list of LLVM targets to build support for. The option is passed through to LLVM configure script. Also notes are added about the implications of (ab)using the option; since the default is not changed, and users of the option are expected to know what they're doing anyway (as every porter should), the impact should be minimal. Fixes rust-lang#38200.
@@ -75,13 +75,18 @@ pub fn llvm(build: &Build, target: &str) { | |||
(true, true) => "RelWithDebInfo", | |||
}; | |||
|
|||
// NOTE: remember to also update `config.toml.example` when changing the defaults! |
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.
👍
Cool! |
Wait there's the problem of stale cache when the target list has been changed. Most likely we'd like to revamp the current stamping logic to include the targets also, maybe ditching the date part altogether. I'll come up with something like that later in the day. |
@bors: r+ Looks good to me! I don't think we have to worry too much about stale issues here, presumably you know what you're doing if you're tweaking this configuration. |
📌 Commit 0f8e931 has been approved by |
Auto-rebuild would be cool though. |
@Yamakaky I've already implemented that in a local branch. Waiting for this PR to land, then I could publish it for review as it's directly built upon this PR. |
Here we go!
r? @alexcrichton
cc @Yamakaky