Use the new Android overlay in the tests and update some Bionic declarations #3009
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rations Motivation: Get this repo building again for Android with NDK 27 Modifications: - Update some networking declarations for newly added nullability annotations - Import the new Android overlay instead in some tests - Add two force-unwraps on all platforms, that are needed for Android Result: This repo and its tests build for Android again
finagolfin
commented
Dec 1, 2024
finagolfin
commented
Dec 1, 2024
| @@ -124,12 +124,20 @@ private let sysWritev = sysWritev_wrapper | |||
| #elseif !os(Windows) | |||
| private let sysWritev: @convention(c) (Int32, UnsafePointer<iovec>?, CInt) -> CLong = writev | |||
| #endif | |||
| #if !os(Windows) | |||
| #if canImport(Android) | |||
| private let sysRecvMsg: @convention(c) (CInt, UnsafeMutablePointer<msghdr>, CInt) -> ssize_t = recvmsg | |||
There was a problem hiding this comment.
These nullability annotations were added with NDK 27 this summer, so adapt these networking APIs for that.
finagolfin
commented
Dec 1, 2024
| #elseif canImport(Bionic) | ||
| import Bionic | ||
| #elseif canImport(Android) | ||
| import Android |
There was a problem hiding this comment.
As discussed a couple months ago, the compiler started requiring different imports after the Foundation rewrite this summer.
Lukasa
approved these changes
Dec 1, 2024
|
The two failing linux CI jobs with nightly-main appear unrelated. |
|
Thanks for the quick review. 😃 |
Lukasa
pushed a commit
that referenced
this pull request
Dec 15, 2024
I added this changed function call for Android in #2660 earlier this year, because of [an incorrect Bionic function signature as explained recently](#3009 (comment)), but now that it has been worked around through a new API note as mentioned there, this change is no longer needed. I simply did not notice this earlier because it still compiles and merely gives a warning, which removing this call now silences.
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.
Motivation:
Get this repo building again for Android with NDK 27
Modifications:
Result:
This repo and its tests build for Android again
I've been using these patches on my Android CI and natively on Android for a couple months now. I didn't bother keeping this patch building for Android with Swift 5 anymore, as my Android CI no longer tests Swift 5.
I built this pull and ran the tests on linux x86_64 to make sure there was no regression.