Android API 23

[marcprux]

This PR builds the Swift SDK for Android to support Android API levels back to 23 (down from 28).

It also unified the source patches in the build scripts so they are just in a single place (in the fetch-source.sh script).

[grynspan]

This patch isn't necessary anymore. Swift Testing uses dlsym() to look up backtrace() before calling it.

[marcprux]

Thanks! I'll remove it.

[finagolfin]

Unfortunately, it is still needed for the 6.2 branch though.

[marcprux]

Derp. OK, I'll put it back.

[grynspan]

Yeah but remove it for main! 😭

[finagolfin]

Meaning?

[grynspan]

I'd like to get this patch out of the way now, then revisit if needed when we add #available to the mix. I don't like having an official toolchain that isn't building from actual source. Makes for spooky bugs I can't diagnose or fix.

[finagolfin]

I mean, we have much bigger problems with Testing, as any Swift package with macros sees strange build errors when cross-compiling, swiftlang/swift-package-manager#8094, so I wouldn't worry about many using Testing until that is fixed. 😉 We will probably have to add doc telling people to only use XCTest on Android until then.

As for this change, rather than putting in logic to check both scheme and tag names that use 6.2 and only applying this change then, let's decide after the next trunk tag, when people can try the new #available(Android feature on their code finally, if we want to submit that new feature for 6.2. If not, I guarantee you that I will elide this change for 6.3 alone before the branch in a couple weeks. 👍

[grynspan]

We definitely don't want to tell people to only use XCTest! Given that Swift Testing is part of the toolchain, there should be no reason a developer who can use XCTest can't use Swift Testing.

But okay, I'll stop complaining until after the next tag.

[finagolfin]

Given that Swift Testing is part of the toolchain, there should be no reason a developer who can use XCTest can't use Swift Testing.

Given that long-standing issue with cross-compiling macros for tests, I don't see any way around warning Android users off Testing for now.

[finagolfin]

What do you want to do with this pull, actually merge all these changes, that simply stub out newer API calls, to the official CI and SDK bundle generator or properly #available check them in their repos once you try this stubbed-out bundle here first?

My bigger concern is who is going to maintain the SDK bundle for these older APIs in the coming years. If @etcwilde and @madsodgaard and whoever else wants us to ship an official bundle built against API 23, they should be willing to handle all issues stemming from APIs 23-27, since that primarily affects them.

[marcprux]

What do you want to do with this pull

Well, I mostly started this PR to see how hard it would be to get an SDK build that worked for API 23. I was surprised how little patching was needed. I hadn't developed any concrete plans beyond that.

Ideally, we'd hear from more people from the forum post on the topic, and especially whether this build might be suitable for @madsodgaard's work. If supporting back to API 23 would prevent the need for people to use their own custom builds of the SDK, then I think it would be worth it to get this in.

The question then would be whether to just go ahead with this PR (which merely disables things that aren't really applicable on Android anyway), or wait on being able to use #available(Android, …) (which, from what I understand, would need to wait until we want to require NDK 28+ for Android development). Since we're already patching stuff, and since the patches are fairly harmless, I personally think it would be fine to merge this short-term. We could even say that we "officially" only support/recommend API 28+, and older targets like x86_64-unknown-linux-android23 would be left to the developer to experiment with.

WDYT, @finagolfin, @shahmishal, @madsodgaard, @grynspan, @andriydruk, @Joannis?

[madsodgaard]

It would be great for us, so that we don't have to fork our own SDK:) but as I mentioned on the forums, we would still need the shim for ifaddrs (to use networking like NIO) and probably also fix up the FILE pointer, for this to be usable for us. Other than that, it would be great.

[grynspan]

Not to be overly cantankerous, but an official Android toolchain really ought to be built from the official Swift sources. In other words, if there are changes needed in components of the Swift toolchain, we should get PRs up for as many of them as possible. If this is a stopgap until #available is, er, available, then can it wait a few weeks for that to happen?

I realize the situation is different for Swift 6.2, but Swift 6.2 has shipped to developers already and changes to the Swift toolchain should probably focus on the next release (6.3) instead.

With regards to the Swift Testing patch specifically, it's a problem for us because it is fragile and means a change to our code could break Android without us having any signal. The code in question is shaped like:

#if Apple
doThis()
#elseif os(Linux)
doThat()
#elseif os(Android)
if let doThat = dlsym("doThat") { doThat() }
#else
#warning("Didn't do that.")
#endif

But if we were to change the code such that Android came first, or such that we had #error instead of #warning, now the toolchain would fail to build. I'm not anticipating those changes will actually happen, just trying to illustrate why I'm opposed to having the patch here.

Anyway, that's my two cents.

[marcprux]

Not to be overly cantankerous, but an official Android toolchain really ought to be built from the official Swift sources

I understand — and I don't disagree with — your objection, but do note that the potential backtrace issue you mention is already in place with the current Swift SDK for Android release. This PR just shuffles the patch from the build-docker and build-local scripts into a single place in fetch-sources.sh (as well as implementing the additional patches).

Not to be too cavalier, but if the build starts failing (either in the official CI or in my watchdog CI), either @finagolfin or I will know to address it. And if/when we are able to make @swift-ci Please test Android requests, that will further enhance stability.

The broader picture is that this PR widens the potential users of this SDK, which is something @shahmishal asked us to look into. The patching is meant to be temporary, and will be progressively removed as we either start relying on #available(Android, …) or work around them with dlsym jiggery-pokery.

[grynspan]

Yup, I'm not trying to hold up the merge, just trying to get my thoughts out before I forget 'em all. :)

[finagolfin]

wait on being able to use #available(Android, …) (which, from what I understand, would need to wait until we want to require NDK 28+ for Android development).

Hmm, that is a good question: we may have to provide an SDK bundle ourselves that's built against the new stable NDK 29, as the next trunk tag may not work well with LTS NDK 27, now that Mads's #available pull was merged.

We may have to use different NDKs for 6.3 and the upcoming 6.2 CI, as long as one has #available and the other doesn't, then decide if we want to push #available into 6.2 or not.

@marcprux, since you're one of the few people building your own toolchain, want to try pushing the #available pull and NDK 29 into a release/6.2.1 bundle locally generated by the Docker script, then copy that modified swift-frontend it builds into your toolchain and test it? We're going to have to provide something like that in our quasi-official 6.2 releases next, so people can try out this new feature easily.

Not to be overly cantankerous, but an official Android toolchain really ought to be built from the official Swift sources. In other words, if there are changes needed in components of the Swift toolchain, we should get PRs up for as many of them as possible.

No problem, I think we all agree on the principle, but as they say, the cantanker is in the details. 😉

In this case, I tried to upstream all the patches I could, but left only three in the final Docker script: a compiler test that we were seeing strange file permission errors with git grep in our Docker runs alone, a modification of build-script to work around a swift-driver regression for which I submitted a fix many months ago, and disabling backtrace() for swift-testing. I think that is fine for a preview release that can't use Testing much anyway because of this long-standing SwiftPM bug, swiftlang/swift-package-manager#8094, which appears to break cross-compiling Testing with SwiftPM to all platforms, while the first two changes obviously only affect the build, not the produced SDK bundle.

As for this change, I'm fine with it, as these APIs basically do nothing on any Android device, because the OS assigns a random temporary user to every Android app installed.

I do think we should investigate and remove the git grep issue soon, and not simply stub out those later posix_spawn APIs if we want to get this in.

[Joannis]

Wow I missed a lot of discussion. I'm all for this PR, and supporting a wider range of devices. Even if we have to mark it as being less supported than more recent SDKs. I do agree with @grynspan 's sentiment that these should go in their respective repos. Also, I think our community should focus on making sure we're in the best shape for 6.3 before we focus on 6.2.x.

[marcprux]

I think we are all sufficiently aligned on this that this PR can be merged into https://github.com/swift-android-sdk/swift-docker/tree/main. I'll need to file another PR for upstream https://github.com/swiftlang/swift-docker/tree/main, so we can continue discussions there…