Apple: Refactor deployment target version parsing#129341
Merged
bors merged 3 commits intorust-lang:masterfrom Sep 7, 2024
Merged
Apple: Refactor deployment target version parsing#129341bors merged 3 commits intorust-lang:masterfrom
bors merged 3 commits intorust-lang:masterfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor deployment target parsing to make it easier to do #129342 (I wanted to make sure of all the places that
std::env::varis called).Specifically, my goal was to minimize the amount of target-specific configuration, so to that end I renamed the
optsfunction that generates theTargetOptionstobase, and made it return the LLVM target andtarget_archtoo. In the future, I would like to move even more out of the target files and intospec::apple, as it makes it easier for me to maintain.For example, this fixed a bug in
aarch64-apple-watchos, which wasn't passing the deployment target as part of the LLVM triple. This (probably) fixes #123582 and fixes #107630.We also now parse the patch version of deployment targets, allowing the user to specify e.g.
MACOSX_DEPLOYMENT_TARGET=10.12.6.Finally, this fixes the LLVM target name for visionOS, it should be
*-apple-xrosand not*-apple-visionos.Since I have changed all the Apple targets here, I smoke-tested my changes by running the following:
I couldn't build for the
arm64e-apple-darwintarget, thearmv7k-apple-watchosandarm64e-apple-iostargets failed to link, and I know that thei686-apple-darwintarget requires a bit of setup, but all of this is as it was before this PR.r? thomcc
CC @BlackHoleFox
I would recommend using
rollup=neverwhen merging this, in case we need to bisect this later.