To clarify, the thing I had to patch out was posix_spawnattr_init and the like. I wasn't aware until today that we were already dlsym-ing posix_spawn (although I suppose I should have wondered why I didn't also need to patch that out).
But if we were already polyfilling posix_spawn, why weren't we also pollyfilling posix_spawnattr_init? I would assume that the availability/unavailability of those two functions would always be co-incidental.
Even if it were, I don't think that attribute even makes any sense on Android, even in the rare case that posix_spawn is actually used. swift-subprocess only bothers with this behavior on Darwin, correct @jakepetroules?
This is the original forum thread where that was first reported and why the pull was submitted later. Saleem made a case that we should try to match all Foundation APIs across platforms as much as we can, which I agree with in principle, though there appeared to be some skepticism from others about how much these Process APIs would even be used on Android.
While I have little idea how useful these three methods would be on Android myself, they do seem easy to implement, so not sure why anyone would complain about finally filling this small gap.
Yes and no -- the PlatformOptions.processGroupID API is available on all Unix-like platforms currently supported in SwiftSubprocess (macOS, Linux, Android, FreeBSD, and OpenBSD).
On Darwin, that currently uses posix_spawn and POSIX_SPAWN_SETPGROUP, while on all other platforms, fork + exec + setpgid is used instead. So it's not an issue.
Note on future direction: there is an open issue that proposes preferring posix_spawn where possible on all platforms (except OpenBSD). The general architecture of SwiftSubprocess allows for it to choose an appropriate strategy (posix_spawn vs fork+exec) based on the parameters of the process spawning request. The Android API level could be just another variable in computing which particular path to use at runtime.
I agree that we should at least drop the minimum API level to 24. As @Finagolfin mentioned, the previous Android SDK has been compiling against that for years. I think the rationale that some limited APIs are not available on Android should not be the deciding factor. Even since the APIs in question are probably not even used by Android developers that much. I understand that previously it might be confusing for users to know what APIs work and do not on different API levels, but I really think now that we have @available(Android) support, that should not be an issue for us.
If people really need to use legacy API versions such as 24 or even 23, I think we should give them the option to, especially now since we can explicitly at compile-time tell them what Swift APIs they can use on those legacy platforms. I don’t see how that is different from not even letting them use the Android SDK, as it is now. And API availability is quite a common thing in Swift anyways.
For me personally, I’d definitely help out with making sure this works out, help patching out things and marking any APIs with availability guards.
If the deciding factor between 23 and 24 is the FILE pointer stuff, we can definitely work that out somehow. It’s not that the actual stdlib or Swift that fails to compile on API 23, it is any libraries like `swift-log` that explicitly assume it to be annotated as OpaquePointer on Android.
On top of that the other obstacle for API 23 I see is I don’t think `URLSession` works, because of `libcurl`, currently due to missing ifaddrs(I have a working stub that I use in my API 23 repo). Again, if we cannot get it working, I don’t think its that unfair to mark `URLSession` as Android 24+ only, just like it has iOS 7.0 requirements as well. And we have other options such as GitHub - swift-server/async-http-client: HTTP client library built on SwiftNIO - which works fine on API 23 (with the stub I use)
I agree. The big question here is whether we should gate the posix_spawnattr_* API using the @available(Android) (which requires bumping the NDK to 28, which might have bigger consequences), or by implementing it with a dlsym dance (which is a hassle), or by just #if !os(Android)-ing out those three calls (which I don't think do anything on Android anyway, so this would be my preferred choice).
Once that's merged, I believe we'll build out-of-the-box for API 24, and then the only major blocker for building an SDK that supports API 23 would be what to do with getgrnam_r and getgrgid_r. We would of course still have the FILE issue, but we might be able to defer solving that and working around it.
I fixed the old polyfills and built an API 23 SDK (based on finagolfins), and tested it on both a API 23 and 28 device, and both work fine!
I also opened a PR for stubbing out getgrgid_r and getgrnam_r, which was the only thing we needed to get Swift compiling on API 23, as well. So, any support there would be appreciated as well. Let's see how it lands.
Is there anything else we are missing? If we get the posix spawn APIs merged, what would be next steps? I.e, can we just start trying distributing API 24 SDKs instead of 28?
Ideally we'd also cherry-pick this to the 6.3 branch so we can get it into the next release. Otherwise, we would need to add some logic to the build scripts to know whether it is building against 6.3 or later, but that wouldn't be too hard.
Personally, I think we should go ahead with it. It is much better to have a single authoritative Swift SDK for Android than having a bunch of different custom builds floating out in the world. And it could also make for a simple support policy: we support back to whatever the minimum suported API level is for the NDK (which will soon be 23). We can talk about it at the next Android workgroup meeting.
Now, the FILE* issues does not block us from compiling a API 23 Swift Android SDK. The issue only arises in projects that assume the imported Swift type of FILE*, such as swift-log. It would be nice for the compiler to assure it is always imported as the OpaquePointer even on API 23. I could not find a way to use API notes to change the imported type of the struct. The only way I see that we could use API notes is to change the signature of all stdio.h functions to force OpaquePointer. That is doable, but it is around 70 functions.
An second option is to modify the ClangImporter in the compiler, to specifically extract FILE* as OpaquePointer on Android.
A third option is to add support for a new compiler directive #if _androidApiLevel(23), such that swift-log (and other libraries) could define FilePointer depending on Android version compiled against? However, from the Android API availability discussion, there did seem to be a bit pushback on adding new compiler directives.
Does anyone else have any other ideas? Or what do you think about the above options? I am not sure if adding anything to the Android module would help, since we need the actual C function signatures to see the same imported type.
In Swift Testing we use a typedef on the C side to rename FILE * to something consistent across platforms. Maybe we need Swift to introduce a CFILEPointer typealias in the stdlib to paper over this problem?
Thanks to the CI team, we have just switched the trunk Android CI to build with Android API 24, which means that starting with the next trunk snapshot of the Swift SDK for Android, you will be able to build against APIs 24-27 also, if you choose (that CI run also has a link to the SDK bundle produced, which you can try using with a recent trunk build of the Swift toolchain). We already switched the trunk Windows CI which builds Android SDKs over to API 24 last month, so both trunk CI are using the same Android NDK 28c and API 24 again now.
Beginning with the latest May 27 trunk snapshot bundle and the first 6.4 snapshot bundle, we now support down to API 23. To use those, note that the setup-android-sdk.sh script is no longer needed with the swift-build build system, as the NDK location is now set by the ANDROID_NDK_HOME environment variable instead.
Thanks to @madsodgaard, the Android workgroup member who drove this effort to support older Android APIs, and the reviewers and CI team who helped put this into place. With the next NDK after the upcoming LTS NDK 30 release planned to drop support for API 21, we shouldn't have to go any further with older APIs.