Use the new Android overlay and Bionic module from Swift 6 #2784
Conversation
| let IFF_BROADCAST: CUnsignedInt = numericCast(Android.IFF_BROADCAST.rawValue) | ||
| let IFF_POINTOPOINT: CUnsignedInt = numericCast(Android.IFF_POINTOPOINT.rawValue) | ||
| let IFF_MULTICAST: CUnsignedInt = numericCast(Android.IFF_MULTICAST.rawValue) | ||
| #else | ||
| let IFF_BROADCAST: CUnsignedInt = numericCast(SwiftGlibc.IFF_BROADCAST.rawValue) |
There was a problem hiding this comment.
Swift 5 has canImport(Glibc) and SwiftGlibc on Android, while Swift 6 no longer uses those but has canImport(Bionic) and canImport(Android) instead. Both have os(Android).
To keep this code compiling for both Swift 5 and 6, I duplicated these C declarations. Whenever NIO stops supporting Swift 5, I will submit a pull to rip these out.
Sources/NIOPosix/Linux.swift
Outdated
| internal static let EPOLLIN: CUnsignedInt = numericCast(CNIOLinux.EPOLLIN) | ||
| internal static let EPOLLOUT: CUnsignedInt = numericCast(CNIOLinux.EPOLLOUT) | ||
| internal static let EPOLLERR: CUnsignedInt = numericCast(CNIOLinux.EPOLLERR) | ||
| internal static let EPOLLRDHUP: CUnsignedInt = numericCast(CNIOLinux.EPOLLRDHUP) | ||
| internal static let EPOLLHUP: CUnsignedInt = numericCast(CNIOLinux.EPOLLHUP) | ||
| #if canImport(Android) | ||
| internal static let EPOLLET: CUnsignedInt = 2147483648 |
There was a problem hiding this comment.
Previously, all these constants couldn't be automatically set by the ClangImporter from the C headers, but now only this one constant couldn't, because it's the only one that's a C macro.
| #if os(Android) | ||
| #if canImport(Android) |
There was a problem hiding this comment.
Shouldn't we just use the same conditionals as above? Instead of checking for the os we just check for canImport(Glibc) and canImport(Android)?
There was a problem hiding this comment.
The OS check works when building for Android on both Swift 5 and 6 and also includes the Android armv7 definitions on line 100 below. It cannot be replaced because then canImport(Glibc) would be true and entered into on linux too, and the #elseif os(Linux) on line 103 would be skipped on most linux distros that use glibc.
There was a problem hiding this comment.
@finagolfin Thanks for the PR. Relatedly I have been wondering if we could setup GH action based Android testing in this repo in the long term?
I would interested to hear what your thoughts are and how we can make developing for Android as easy as installing a cross compilation SDK.
|
Thanks for the quick review, a github action CI workflow for Android on this repo would be great. 😄 I plan to start distributing an Android SDK bundle, which would obsolete the additional local configuration required for my current Android SDK, and then look into putting together a custom github build action that any Swift package can use with that new SDK bundle for their own CI, finagolfin/swift-android-sdk#127. If you would like, I can submit an Android CI workflow for this repo when that SDK bundle is ready. However, my daily Android CI already tests this repo and I quickly upstream most changes needed, so I'm fine with that cadence too. It really just depends if you want to know about Android breaks with every pull, and given that Android is not an officially supported platform, I'm fine with not imposing that additional requirement on NIO pull authors for now. |
In the fullness of time I would like to have Android support in all the repos as well. This would require a few things:
Once we tackle the latter we might be able to distribute this cross compilation bundle on swift.org which would unlock us testing this in our GitHub Actions. |
Already done and I think provisionally agreed to by the core team members I talked to. I will know for sure once I submit it as part of the official CI, which has only been delayed by me.
Yes, once I'm distributing my own Android SDK bundle and then if and when Android becomes officially supported, distributing an official Android SDK bundle like the current official linux static SDK bundle would be the next step. We can try setting up an Android CI action for this and other Swift repos then, let's see. |
Motivation: Get this repo building again for Android with the new overlay Modifications: - Import the new module or overlay wherever `Glibc` is used - Keep this repo building with Swift 5 by duplicating some declarations Result: All the same tests keep passing on my Android CI, finagolfin/swift-android-sdk#158
|
Rebased and fixed merge conflicts, anything holding up review here? |
Motivation:
Get this repo building again for Android with the new overlay
Modifications:
Glibcis usedResult:
All the same tests keep passing on my Android CI, finagolfin/swift-android-sdk#158
I imported the smaller Bionic module where possible, and only used
Androidwhen files invoked C functions not in the new Bionic module, swiftlang/swift#72161.