-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[6.2][build] Make it possible to build a cross-compilation toolchain for Android, including Testing #83503
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
base: release/6.2
Are you sure you want to change the base?
Conversation
@swift-ci test |
@swift-ci test |
@swift-ci test |
@swift-ci smoke test macos |
@swift-ci test macos |
@bnbarham, completely safe pull that only affects Android, please review. |
Android 15+ requires that native libraries be compiled with a linker flag to support 16 KB page sizes. See: https://developer.android.com/guide/practices/page-sizes#compile-r26-lower
…` from fcc0389, specifically the change that makes the following possible on linux: "* under Linux, do not cross compile LLVM when building for the host architecture -- that will ensure that the compiler-rt build will use the just built compiler and not the system one (which may not be new enough for this purpose);"
…wiftlang#83260) Also, disable a recently failing test for Android armv7.
This new flag makes it easy to build Swift cross-compilation toolchains, by disabling cross-compilation of all host tools, like the Swift compiler and various macros, building on prior pulls swiftlang#38441 and swiftlang#82163. Also, add two class methods to the Testing macros product so it works with swiftlang#83260.
I missed that `build-script` passes back `false` instead, so use the `true_false` function for broader checking.
@swift-ci test |
@bnbarham, not sure if you saw my ping this weekend, but can we get this in before the new 6.2.0 branch diverges? |
Ah sorry, I did not. This isn't really my area though - CC @shahmishal |
Looks good to me, though I'd appreciate if @etcwilde could sign off on the CMake changes here. @shahmishal Could this be pulled into 6.2.0? |
Ping @etcwilde, please take a look, would like to get this into 6.2.0/6.2.1 next. |
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.
One comment on enabling the cross-compiling test on iOS, tvOS, and watchOS.
Other than that, this should be fine.
@@ -1,11 +1,15 @@ | |||
# REQUIRES: standalone_build | |||
# UNSUPPORTED: OS=macosx | |||
# UNSUPPORTED: OS=ios |
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 not sure what it means to cross-compile for Android from an iPhone, Apple TV, or an Apple Watch.
These should likely remain unsupported.
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.
Heh, nobody is suggesting that: 😄 these are merely CI exclusions that determine whether this Android build-script
-related test will be run as part of a CI run targeting those Darwin platforms. Mishal disabled this test for those Darwin platforms four years ago in de148ad and 0ebceda because of some weird CI errors at that time, but I figured that these BuildSystem tests had matured since then and tried removing these exclusions in trunk a couple weeks ago, with macOS/iOS/watchOS now passing and no complaints from whatever internal CI runs any more of those platform tests.
The idea here is simply to clean up unnecessary platform exclusions that are no longer needed, not to pave the way to cross-compile from an Apple TV for Android. 😃 If you think that we should keep some of these exclusions because they may break some internal Apple CI, just let me know which and I will modify this pull to keep them. However, since nobody has reported any breakage in trunk for the last week and a half since these Darwin exclusions were removed, I think it's safe to clean up these legacy exclusions.
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.
Yeah, let's keep this disabled for iOS, tvOS, and watchOS. Thanks.
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.
@etcwilde, what are you worried might go wrong exactly? As I noted in one of the original trunk pulls that I added with this 6.2 pull, these BuildSystem tests are already disabled on remote test executors. I believe they are still run when simply targeting iOS/tvOS/watchOS from macOS because they are unaware of that target OS for the CI, ie they are only aware that the host is macOS, so now that this test works on macOS, it works for all Darwin platforms.
If you think these BuildSystem tests don't make sense to run again for each of the non-macOS Darwin platforms, I suggest you modify that lit.local.cfg separately to disable them. My removal of these lines here is purely procedural: they all work now so I remove unnecessary logic, just like they were disabled four years ago by Mishal because they were failing then.
Explanation: Add a linker flag for the upcoming 16 KB page support in Android, generate an Android CMake toolchain file that can be used to cross-compile repos like cmark and Testing, and add a build flag that makes it easy to build cross-compilation toolchains, by disabling the cross-compilation of all host tools and macros for listed
--cross-compile-hosts
, leaving only the Swift runtime libraries in a cross-compilation SDK.Scope: Build changes that either only affect Android or have to be explicitly enabled with a new
build-script
flagIssue: #80788
Original PR: #81596, single supporting change from #81386, #83260, #83422, #83629
Risk: Very low, only affects Android
Testing: Passed CI and we're using these patches in preliminary builds of an official Android CI job, which uses these changes to pass the full compiler validation suite and produce an Android SDK bundle
Reviewer: @edymtt
The Swift core team tasked the Swift on Android workgroup with putting together an official 6.2 Android CI job, so @marcprux and I have been piecing together a pull for that, swiftlang/swift-docker#467, and these are some of the last trunk patches we had to backport to 6.2.