swift
swift copied to clipboard
swiftlang
[android] add a module map for Android NDK
This module map covers the Bionic C standard library and other Posix headers used in the Android NDK
Oh nice, I had a similar pull I had been trying for both Glibc and Bionic, #66665, but this pull appears more specific to Bionic, which is good. Have you tried this pull with the C++ Interop tests on Android? Because they don't work with the current SwiftGlibc.h approach with NDK 26, hopefully this helps get them working.
but this pull appears more specific to Bionic, which is good. Have you tried this pull with the C++ Interop tests on Android? Because they don't work with the current
SwiftGlibc.happroach with NDK 26, hopefully this helps get them working.
Yeah, this allows me to compile Swift code with C++ interop enabled, and use libc++ from the NDK as well in Swift :)
Yeah, this allows me to compile Swift code with C++ interop enabled, and use libc++ from the NDK as well in Swift
OK, they were mostly working before the latest NDK 26, but NDK 26 broke many of them. You were using NDK 26? How were you running those tests, in an emulator? Because the community Android CI doesn't run those executable tests, so I run them natively in the Termux app.
OK, they were mostly working before the latest NDK 26, but NDK 26 broke many of them. You were using NDK 26? How were you running those tests, in an emulator?
I didn't run the C++ interop tests on device yet unfortunately. What kind of issues did you see with NDK 26? That's something we will need for sure, but we don't have a good test setup yet.
what is the plan for selecting a given API level to build against?
good question, we didn't discuss that yet. let me figure that out.
what is the plan for selecting a given API level to build against?
good question, we didn't discuss that yet. let me figure that out.
This seems to be currently behaving as desired - the module triple strips the API level. This allows us to build the SDK against an API level that we want to support (I am currently leaning towards 28). When the client code is built, it can build against a higher API level and still work with the single SDK build. The API level encoded version of the triple would be used by the driver for the linker driver.
This seems to be currently behaving as desired - the module triple strips the API level. This allows us to build the SDK against an API level that we want to support (I am currently leaning towards 28). When the client code is built, it can build against a higher API level and still work with the single SDK build. The API level encoded version of the triple would be used by the driver for the linker driver.
API level in the triple seems reasonable. That is in alignment with clang, IIRC.
What kind of issues did you see with NDK 26?
The same error I mentioned to you more than a year ago, except dozens of C++ Interop executable tests that passed with NDK 25 now fail to compile with that mbstate_t error with NDK 26. I know the issue is something related to NDK 26 because I natively built a late November trunk source snapshot tag of the Swift toolchain twice in the Termux app: the exact same source built once with NDK 25c then with NDK 26b.
What kind of issues did you see with NDK 26?
The same error I mentioned to you more than a year ago, except dozens of C++ Interop executable tests that passed with NDK 25 now fail to compile with that
mbstate_terror with NDK 26. I know the issue is something related to NDK 26 because I natively built a late November trunk source snapshot tag of the Swift toolchain twice in the Termux app: the exact same source built once with NDK 25c then with NDK 26b.
I see. That error should go away with the updated module map, since I am able to successfully build and import the uchar module from the NDK now.
@ian-twilightcoder I updated the module map to prefer split modules, how does it look now.
The sysroot flag is being worked on here - https://github.com/apple/swift/pull/72352 . I removed the sysroot detection from this PR .
@ian-twilightcoder I updated the module map, let me know what you think. I switched the majority of modules to be underscored (e.g. _bionic_stdlib), to prevent users from importing them directly from Swift.
On the topic of if we should have a large monolithic module or not, I think so far it would be more consistent with Swift's overall cross platform story to have a single umbrella Android module for the NDK. This is similar to the WinSDK approach for Windows, and also more similiar to the Glibc approach for linux. I think it would be great to have more narrow & topic specific umbrella modules , but I think that they should be more standardized accross all platforms in Swift, and as such they're outside of the scope of this PR.
On the topic of if we should have a large monolithic module or not, I think so far it would be more consistent with Swift's overall cross platform story to have a single umbrella Android module for the NDK. This is similar to the WinSDK approach for Windows, and also more similiar to the Glibc approach for linux.
Agreed that it's out of scope for this PR, but how wonder it would be to banish the libc-import towers
#if canImport(Glibc)
@_exported import Glibc
#elseif canImport(Musl)
@_exported import Musl
#elseif os(Windows)
@_exported import CRT
@_exported import WinSDK
#elseif os(WASI)
@_exported import WASILibc
#else
@_exported import Darwin.C
#endif
Like if you could import the actual parts defined by the C spec with a simple import libc and then if you needed platform-specifics, you could then go import them separately.
Like if you could import the actual parts defined by the C spec with a simple import libc and then if you needed platform-specifics, you could then go import them separately.
Yep, that should be done. I thought there was a recent discussion thread about Libc module on the forums, but I couldn't find (there's also this one from 2016: https://forums.swift.org/t/proposal-for-a-standard-darwin-glibc-module/4434). I can create a new Bionic submodule here that just imports the libc headers, and we can earmark it for future general Libc module use.
@etcwilde - I think that we should strive to avoid WinSDK in that switch. That is the Windows SDK which is a really wide set of APIs and libraries outside of the C standard. Just CRT would be the right thing.
I thought there was a recent discussion thread about Libc module on the forums, but I couldn't find (there's also this one from 2016
You may be thinking of this more recent pitch thread from a couple years ago, that didn't end up going anywhere.
Like if you could import the actual parts defined by the C spec with a simple import libc and then if you needed platform-specifics, you could then go import them separately.
Yep, that should be done. I thought there was a recent discussion thread about
Libcmodule on the forums, but I couldn't find (there's also this one from 2016: https://forums.swift.org/t/proposal-for-a-standard-darwin-glibc-module/4434). I can create a newBionicsubmodule here that just imports the libc headers, and we can earmark it for future generalLibcmodule use.
I was talking to @ian-twilightcoder the other day about an ambition I have to do something in that area. The ideal, I think, would be to have import Libc just import things defined by the C standard(s), and nothing else, the point being that it's there for portability purposes. It's slightly tricky to do that in practice because the system headers tend to define all kinds of extra stuff and as @ian-twilightcoder commented they may also include lots of other headers.
- Added 'Bionic' module that includes just the Libc headers.
- Added 'Android' Swift overlay module, that is required to build
FoundationusingAndroidinstead ofGlibc
I'm going to rebase, squash and split this patch, first to cover adding the module map only, and the second patch will add the actual overlay and build it with the stdlib. This will let us land the module map without any disruptions, while the follow-up patch will land with additional changes to Foundation.
I will address the review comments for additional module map changes after that.