- Notifications
You must be signed in to change notification settings - Fork13.6k
Fix empty target_config in apply_rust_config bootstrap#144126
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 ourterms of service andprivacy 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?
Fix empty target_config in apply_rust_config bootstrap#144126
Conversation
This PR modifies If appropriate, please update |
This comment has been minimized.
This comment has been minimized.
| [build] rustdoc 0 <host> | ||
| [doc] std 1 <host> crates=[core] | ||
| "); | ||
| if host_target() == "x86_64-unknown-linux-gnu" { |
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.
Hmm, I don't like this. Let's figure out a different way to do this. Because the config hardcodes the name of a specific target, we'll have to generalize that somehow. For example, we can create a function that will return a list of targets for which we opt into LLD by default. Then in tests, we'll have a#[cfg(test)] version of that function, which will temporarily allow a test to override this set of targets.
Then we should implement explicit tests that will override the LLD opt in target list and check that bootstrap behaves correctly, i.e. disables the opt in if external LLVM config is provided and if LLD is disabled in the config.
This PR fixes the issue of an empty target_config in apply_rust_config, which was caused by the ordering of TOML config parsing. This was inadvertently introduced during the last config refactor. The test and the corresponding configuration order have been corrected in this PR.
r?@Kobzol