Android API minimum for the Swift SDK for Android

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.

Simple, the posix_spawn polyfills were only added for the APIs that were used in Foundation in July 2019, but as I noted last week, these three posix_spawnattr_* functions were newly used from the pull I linked in Dec. 2020, after which nobody bothered adding polyfills for them.

1 Like

I see. I'd be in favor of just #if !os(Android)-ing out the calls to posix_spawnattr_*, since on non-Darwin platforms they are only used to set the POSIX_SPAWN_SETPGROUP (for [SR-13861] macOS vs. Linux: exec()'d subprocess runs with different process group · Issue #3964 · swiftlang/swift-corelibs-foundation · GitHub). No one is ever even invoking posix_spawnattr_setpgroup, so I don't even see how it is expected to be doing anything useful in any case.

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)

Just my two cents :smiley:

3 Likes

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).

@madsodgaard, if you want to pick an option and file a PR for it against https://github.com/swiftlang/swift-corelibs-foundation/blob/main/Sources/Foundation/Process.swift, I'd support whichever option you pick :slight_smile:

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.

2 Likes

I’ll take a stab at it soon:)

2 Likes

I have opened a PR to shim out the remaining spawn APIs. Are there any other blockers for API 24?

Great!

I have opened a PR to shim out the remaining spawn APIs. Are there any other blockers for API 24?

I believe that should be the only issue, based on what I needed to patch out to get a POC building.

If you want to go down to API 23, you'll additionally need stubs for getgrgid_r and getgrnam_r.

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?

4 Likes

I think that if we can land swift-corelibs-foundation/pull/5301 (for posix_spawnattr_r) and swift-foundation/pull/1663 (for getgrgid_r and getgrnam_r), then that will be all we will need to get it building by setting android_api=23 in ‎swift-ci/sdks/android/scripts/build.sh (like I do in the experimental swift-docker PR).

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.

3 Likes

All should be good to start testing API 24 SDKs soon. It has been merged into main and a cherrypick PR for 6.3 is waiting for review [6.3] Add Android shims for `spawnattr` by madsodgaard · Pull Request #5398 · swiftlang/swift-corelibs-foundation · GitHub

For API 23 the only "hard" blocker is the getgrnam_r and getgrgid_r used in Foundation. I have a open PR for adding shims for these two as well: [Android] Add shims for `getgrnam_r` and `getgrgid_r` for API 23 by madsodgaard · Pull Request #1663 · swiftlang/swift-foundation · GitHub

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.

cc @Finagolfin @marcprux @jrose @grynspan

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. :smiley:

Thanks to @madsodgaard for getting the Foundation fixes in to make this work, and the Foundation reviewers who checked his pulls. We'll see if we can get API 23 in next, and CI work is underway for the next LTS NDK 30, currently in beta.

10 Likes

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.

10 Likes