Skip to content

Add support for iOS triples in swift build #6572

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Add support for iOS triples in swift build #6572

wants to merge 5 commits into from

Conversation

aleksproger
Copy link
Contributor

@aleksproger aleksproger commented May 17, 2023 •
edited by MaxDesiatov
Loading

Add support for iOS/iOS Simulator triples in swift build

Motivation:

Fixes the issue - #6571
Details provided there

Modifications:

  • Add parsing of ios/ios-simulator triples
  • Add parsing of SupportedPlatformVariant to XCFrameworkMetadata
  • Add ios to Triple.OS and simulator to Triple.ABI

Result:

  • swift build accepts the following triples arm64/x86_64-apple-ios, arm64/x86_64-apple-ios-simulator
  • SwiftPM able to resolve the necessary XCFramework slice for provided triple

@aleksproger
Copy link
Contributor Author

aleksproger commented May 17, 2023 •
edited
Loading

Will add tests later after settling things down and if the change would be accepted:)

@aleksproger aleksproger mentioned this pull request May 17, 2023
Closed
@tomerd
Copy link
Contributor

tomerd commented May 17, 2023

cc @neonichu

@aleksproger aleksproger marked this pull request as draft May 18, 2023 10:06
Aleksei added 3 commits May 18, 2023 21:30
Link platform SDKs when building for iOS/iOS simulator
5b94d39 
**Content**
- Added logic to resolve SDK paths whe building for Appli platforms
- Added specific structure PlatformSDKPaths insted of simple tuple to represetn paths for platform headers/sdk
- Destination.defaultDestination now instead of silent fallback to host emits error when triple is not supported
- Added changes from swiftlang#6380 to pass --target fla to clang/swiftc with provided triple
- Added validation of supported triples in applePlatformDestination() to ensure correct combination and enforce specifying the os version
Cache sdkPlatformPaths result
185a899
Add OS and ABI information to PlatformSDK to perform caching
d90e437
@aleksproger
Copy link
Contributor Author

@neonichu @MaxDesiatov any thoughts on this?
By the way, previously it was non-obvious that one cannot build triples for non-Mac Apple platforms, while you may chose .iOS and others in platforms section. And lack of descriptive errors also forced to dig into the details of what's going on. So, at least more descriptive error messages could be added.

@aleksproger aleksproger marked this pull request as ready for review May 24, 2023 13:21
@kabiroberai
Copy link
Contributor

This is promising! A couple of things to note:

  • You probably want to update BuildParameters.targetTripleArgs(for:) to handle iOS as well
  • It'd be even better if this supported other Darwin triples: the Swift Driver has a more comprehensive representation of LLVM triples here

@aleksproger
Copy link
Contributor Author

aleksproger commented Jul 18, 2023 •
edited
Loading

@kabiroberai Yeah, regarding to the more comprehensive triples representation. In parallel to my pull-request there were a change to how SPM interacts with LLVM triple here:
#6627
That's why I didn't make any changes to this PR. For now it's assigned to @neonichu so seems like guys from Apple picked this up, but if not I may continue to work on this and make updates

@kabiroberai
Copy link
Contributor

Ah that's great, it looks like the Triple changes just haven't been cherry-picked into 5.9. Interestingly the targetTripleArgs bug (assuming darwin == macOS) exists on main as well so I might throw up a PR to fix that.

@neonichu
Copy link
Contributor

Sorry, this PR is just assigned to me to review it, but I haven't had a chance to do so.

@kabiroberai
Copy link
Contributor

kabiroberai commented Jul 20, 2023 •
edited
Loading

Decided to take a crack at this with the new Triple type + UserToolchain changes: #6732. @aleksproger I'd be happy to push my changes to your branch instead if you wanna give me write access.

@kabiroberai kabiroberai mentioned this pull request Jul 20, 2023
Merged
@neonichu
Copy link
Contributor

neonichu commented Sep 5, 2023

@kabiroberai I think we can close this in favor of #6732, right?

@kabiroberai
Copy link
Contributor

@neonichu yep!

@MaxDesiatov MaxDesiatov closed this Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abertelrud abertelrud Awaiting requested review from abertelrud

@neonichu neonichu Awaiting requested review from neonichu

@tomerd tomerd Awaiting requested review from tomerd

@MaxDesiatov MaxDesiatov Awaiting requested review from MaxDesiatov

Assignees

@neonichu neonichu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aleksproger @tomerd @kabiroberai @neonichu @MaxDesiatov