[android] Fix package manifest and one Bionic function's API version

[finagolfin]

@jakepetroules, looks like you added that new C function, please take a look.

[finagolfin]

@swift-ci please test

[finagolfin]

This was in the CMake config for Swift 5, but was not brought over in the Foundation rewrite in Swift 6.

I still haven't needed it for the new CMake config, maybe because it was set up as a framework in Swift 5 and now it's just a CMake static library?

[jmschonfeld]

For my context, could you elaborate on why this removal is necessary? Not sure if Android has all of the dispatch pieces compiled out, but otherwise Foundation will fail to build without dispatch which isn't in the default header search paths

[finagolfin]

I only build these libraries for Android using CMake through the official build-script, so this SwiftPM config is unused normally. This manifest only appears to be used by build-script when running the tests, which I do natively on Android, with DISPATCH_INCLUDE_PATH set so this block goes unused.

For the general case of simply compiling this repo with SwiftPM, I'm not sure why you'd use these headers from the host platform for linux either, as they could be any random version or not be there. When natively compiling on Android, these system directories don't exist, so I simply removed Android.

[jmschonfeld]

Ah gotcha, so just to summarize to make sure I have it right: Dispatch headers don't actually exist at /usr/lib/swift because the headers aren't installed onto an Android OS, and when cross-compiling from linux where they do exist they're likely wrong because they don't match the "Android SDK" that you're compiling for (akin to using something in the system's /usr/lib on a Mac when compiling for iOS)

If so, this sounds similar to the Windows situation, where instead we do:

    if let sdkRoot = Context.environment["SDKROOT"] {
        dispatchIncludeFlags.append(.unsafeFlags([
            "-I\(sdkRoot)usr\\include",
            "-I\(sdkRoot)usr\\include\\Block",
        ], .when(platforms: [.windows])))
    }

in order to use headers from the SDK instead of the host system or on the destination system since Windows also doesn't have a \usr\lib\swift. Is there an equivalent environment variable that we can use to get the SDK path when building for Android to find these headers? If not, removing this to effectively require the manual inclusion of DISPATCH_INCLUDE_PATH which is what the tests do when building against a local copy of dispatch seems best (which is what you have here).

[finagolfin]

Is there an equivalent environment variable that we can use to get the SDK path when building for Android to find these headers?

Sort of, there is a $PREFIX variable set in the most popular Android environment, the Termux app, where you can build Swift, but again, that simply has the latest release version, which could cause problems if you're trying to build the trunk version of this repo.

I realize you're just trying to keep this repo building for this niche use case, but I think you'd be better off erroring out on all non-Windows platforms with a message telling people to get the matching version of libdispatch and specify it with DISPATCH_INCLUDE_PATH, at least until there's a proper Swift package manifest for libdispatch.

Simply grabbing the system libdispatch headers from /usr/lib/swift/ or $PREFIX will likely cause subtle versioning errors at some point, better to avoid that.

[jmschonfeld]

Gotcha ok if this convenience isn't easy / isn't necessary for the Android build then that's fine to leave for now.

I realize you're just trying to keep this repo building for this niche use case

In reality this is actually far from a niche use case - most contributors to this project (nowadays after the re-core) do not work on this project with a full toolchain checkout/build so it's really important to us that (when possible) this project can be built as a standalone project via SwiftPM. We've had a lot of recent success with triaging issues and fixing bugs that before would have required a much larger effort to fix due to the simplification of our build processes.

In this case, yes it's entirely possible that swift-corelibs-libdispatch could update their C headers and this project could be updated to depend on those new headers which would break this convenience build which picks up headers from the most recent release on the system. However, the likelihood that the public swift-corelibs-libdispatch headers are updated and Foundation decides to depend on that update is quite low at this point and even so this build would still work fine using the nightly Linux docker containers. Based on that we've found that on Linux the value of the convenience is far greater than the risk of a mismatch resulting in a build failure. But if you don't feel that trade-off of reading from $PREFIX is as valuable/applicable on the Android side then leaving then leaving this off sounds like the right move as long as Android development has the expectation that dispatch will need to be manually built via CMake first (or that this project is built as part of a full toolchain build)

[jmschonfeld]

@swift-ci please test Windows

[finagolfin]

@jmschonfeld, mind reviewing? Jake was only pinged for the one C function he wrote, and that was only modified for the Android version to match his comment, so you'd be more qualified for this pull anyway.

[finagolfin]

Thanks, merged, once #5024 is in, we should get these into 6.1.