[android] Add support for natively building on Android

[finagolfin]

Check if building on Android with uname -o through the ANDROID_DATA environment variable, then set SWIFT_HOST_VARIANT to android and use it for all further configuration. ANDROID_NATIVE_SYSROOT is also set to the default layout for the Termux app, and all the include, lib, and other SDK paths keyed off of that. Finally, the system libc and a few other libraries are linked against from /system/lib[64] and RPATH is configured.

To begin with, I installed the Termux app and its clang, cmake, ninja, git, libicu-dev, binutils-gold, python2, libuud-dev, perl, and pkg-config packages. A couple C++ headers had to be tweaked after the latest NDK 19b update and the default python symlink had to be changed to version 2, so it didn't use version 3.

I haven't tried using build-script natively yet, but simply configured and built the same targets on a x86_64 linux VPS, then extracted the commands that it ran and used them on Android. That meant running the following CMake and ninja commands to get Swift built and tested on Android.

CMark:

cmake -G Ninja -DCMAKE_BUILD_TYPE:STRING=Release /data/data/com.termux/files/home/src/swift/cmark
ninja

LLVM:

cmake -G Ninja -DLLVM_VERSION_MAJOR:STRING=7 -DLLVM_VERSION_MINOR:STRING=0 -DLLVM_VERSION_PATCH:STRING=0 -DCLANG_VERSION_MAJOR:STRING=7 -DCLANG_VERSION_MINOR:STRING=0 -DCLANG_VERSION_PATCH:STRING=0 -DLLVM_ENABLE_ASSERTIONS=FALSE '-DLLVM_TARGETS_TO_BUILD=ARM;AArch64' '-DCMAKE_C_FLAGS= -Wno-unknown-warning-option -Werror=unguarded-availability-new -fno-stack-protector' '-DCMAKE_CXX_FLAGS= -Wno-unknown-warning-option -Werror=unguarded-availability-new -fno-stack-protector' '-DCMAKE_C_FLAGS_RELWITHDEBINFO=-O2 -DNDEBUG' '-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=-O2 -DNDEBUG' -DCMAKE_BUILD_TYPE:STRING=Release -DLLVM_INCLUDE_DOCS:BOOL=TRUE -DLLVM_ENABLE_LTO:STRING= -DLLVM_BUILD_EXTERNAL_COMPILER_RT:BOOL=TRUE -DLLVM_LIT_ARGS=-sv -DLLVM_DEFAULT_TARGET_TRIPLE=aarch64-unknown-linux-android /data/data/com.termux/files/home/src/swift/llvm
ninja

Two additional flags were set for Android: only building ARM targets and setting the default triple. A couple compiler-rt sanitizers didn't compile. I had to apply these two Termux patches to get the local clang to work too.

Swift:

cmake -G Ninja -DLLVM_VERSION_MAJOR:STRING=7 -DLLVM_VERSION_MINOR:STRING=0 -DLLVM_VERSION_PATCH:STRING=0 -DSWIFT_STDLIB_ENABLE_SIL_OWNERSHIP=FALSE -DCMAKE_EXPORT_COMPILE_COMMANDS=TRUE -DSWIFT_FORCE_OPTIMIZED_TYPECHECKER=FALSE -DSWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING=FALSE '-DCMAKE_C_FLAGS= -Wno-unknown-warning-option -Werror=unguarded-availability-new -fno-stack-protector' '-DCMAKE_CXX_FLAGS= -Wno-unknown-warning-option -Werror=unguarded-availability-new -fno-stack-protector' '-DCMAKE_C_FLAGS_RELWITHDEBINFO=-O2 -DNDEBUG' '-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=-O2 -DNDEBUG' -DCMAKE_BUILD_TYPE:STRING=Release -DLLVM_ENABLE_ASSERTIONS:BOOL=FALSE -DSWIFT_ANALYZE_CODE_COVERAGE:STRING=FALSE -DSWIFT_STDLIB_BUILD_TYPE:STRING=Release -DSWIFT_STDLIB_ASSERTIONS:BOOL=FALSE -DSWIFT_STDLIB_USE_NONATOMIC_RC:BOOL=FALSE -DSWIFT_ENABLE_RUNTIME_FUNCTION_COUNTERS:BOOL=FALSE -DSWIFT_NATIVE_LLVM_TOOLS_PATH:STRING= -DSWIFT_NATIVE_CLANG_TOOLS_PATH:STRING= -DSWIFT_NATIVE_SWIFT_TOOLS_PATH:STRING= -DSWIFT_INCLUDE_TOOLS:BOOL=TRUE -DSWIFT_BUILD_REMOTE_MIRROR:BOOL=TRUE -DSWIFT_STDLIB_SIL_DEBUGGING:BOOL=FALSE -DSWIFT_CHECK_INCREMENTAL_COMPILATION:BOOL=FALSE -DSWIFT_REPORT_STATISTICS:BOOL=FALSE -DSWIFT_BUILD_DYNAMIC_STDLIB:BOOL=TRUE -DSWIFT_BUILD_STATIC_STDLIB:BOOL=FALSE -DSWIFT_BUILD_DYNAMIC_SDK_OVERLAY:BOOL=TRUE -DSWIFT_BUILD_STATIC_SDK_OVERLAY:BOOL=FALSE -DSWIFT_INCLUDE_TESTS:BOOL=TRUE -DSWIFT_EMBED_BITCODE_SECTION:BOOL=FALSE -DSWIFT_TOOLS_ENABLE_LTO:STRING= -DSWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER:BOOL=FALSE -DLLVM_LIT_ARGS=-sv -DCOVERAGE_DB= -DSWIFT_DARWIN_XCRUN_TOOLCHAIN:STRING=default -DSWIFT_AST_VERIFIER:BOOL=TRUE -DSWIFT_SIL_VERIFY_ALL:BOOL=FALSE -DSWIFT_RUNTIME_ENABLE_LEAK_CHECKER:BOOL=FALSE -DSWIFT_PATH_TO_CLANG_SOURCE:PATH=/data/data/com.termux/files/home/src/swift/llvm/tools/clang -DSWIFT_PATH_TO_CLANG_BUILD:PATH=/data/data/com.termux/files/home/src/swift/build/llvm -DSWIFT_PATH_TO_LLVM_SOURCE:PATH=/data/data/com.termux/files/home/src/swift/llvm -DSWIFT_PATH_TO_LLVM_BUILD:PATH=/data/data/com.termux/files/home/src/swift/build/llvm -DSWIFT_PATH_TO_CMARK_SOURCE:PATH=/data/data/com.termux/files/home/src/swift/cmark -DSWIFT_PATH_TO_CMARK_BUILD:PATH=/data/data/com.termux/files/home/src/swift/build/cmark -DSWIFT_PATH_TO_LIBDISPATCH_SOURCE:PATH=/data/data/com.termux/files/home/src/swift/swift-corelibs-libdispatch -DSWIFT_CMARK_LIBRARY_DIR:PATH=/data/data/com.termux/files/home/src/swift/build/cmark/src -DSWIFT_SDKS:STRING=ANDROID -DLLVM_DIR:PATH=/data/data/com.termux/files/home/src/swift/build/llvm/lib/cmake/llvm -DClang_DIR:PATH=/data/data/com.termux/files/home/src/swift/build/llvm/lib/cmake/clang -DSWIFT_ANDROID_API_LEVEL:STRING=21 /data/data/com.termux/files/home/src/swift/swift
ninja all swift-stdlib-android-aarch64

I ran the validation test suite after building the Swift/LLVM/CMark/compiler-rt 3/6/2019 source snapshots on my 64-bit Android device, and only about 30-40 tests failed:

ninja SwiftUnitTests check-swift-validation-android-aarch64
grep  "testsuite " swift-test-results/aarch64-unknown-linux-android/lit-tests.xml
<testsuite name="Swift-Unit" tests="480" failures="0" skipped="0">
<testsuite name="Swift(android-aarch64)" tests="11053" failures="41" skipped="1608">

I'll look into those next, some might simply be configuration issues with Termux. To get lit working on Android, I had to back out apple/swift-llvm@919d78ce8 and apple/swift-llvm@a898e97f, because Android doesn't implement sem_open.

Nice work by @modocache, @zhuowei, @compnerd, @amraboelela, @drodriguez, @futurejones, and others that this pull is so small and still gets most of Swift built and seemingly working on 64-bit Android. I added some support for building natively on 32-bit Android too, but haven't tested there and likely never will.

Once this is in, I'll look into adding a Termux package for Swift, so that people can write Swift code on their Android phone or tablet.

[finagolfin]

Why use CMAKE_SYSTEM_NAME and not SWIFT_HOST_VARIANT here? The two seem to be inconsistently used in some places.

[compnerd]

CMAKE_SYSTEM_NAME is the name of the system we are targeting. That should be preferred to enable cross-compilation. The SWIFT_HOST_VARIANT is the target for the runtime components IIRC, but, we should be aiming to reduce that usage (as it prevents cross-compilation).

[finagolfin]

No, the docs say SWIFT_HOST_VARIANT is the "Deployment OS for Swift host tools (the compiler)" and it is used that way in a few places. However, counting it up now, I see CMAKE_SYSTEM_NAME used much more in this repo.

A decision probably needs to be made on whether SWIFT_HOST_VARIANT should be defined at all, as right now its basically just an alias for CMAKE_SYSTEM_NAME. This pull actually tries to use SWIFT_HOST_VARIANT though, since CMAKE_HOST_SYSTEM_NAME/CMAKE_SYSTEM_NAME are set to "Linux" by default on Android.

[compnerd]

CMAKE_SYSTEM_NAME should/must be Android: https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html#cross-compiling-for-android. Switching to CMAKE_SYSTEM_NAME seems like an improvement.

[finagolfin]

First off, I'm still not sure if you understand exactly what I'm doing here, take a look at my latest long comment answering @drodriguez's questions. That CMake manual link you give is irrelevant because it's about cross-compiling with the NDK, which I'm not using here.

As for using CMAKE_SYSTEM_NAME instead to cross-compile the host tools, it is a possibility. However, someone, presumably @gribozavr in 6670bb7, chose to use SWIFT_HOST_VARIANT_SDK/SWIFT_HOST_VARIANT, presumably to give this project more flexibility in how it names and deals with the host OS in CMake.

I'm simply following that precedent by using SWIFT_HOST_VARIANT more. For example, build-script always sets the SWIFT_HOST_VARIANT* variables and never CMAKE_SYSTEM_NAME.

However, both are being used all over this project's CMake config now, so it will break if you actually try to cross-compile the host tools for another OS someday, unless you set both CMAKE_SYSTEM_NAME and SWIFT_HOST_VARIANT*. 😄

[drodriguez]

@compnerd: I looked a little to setting CMAKE_SYSTEM_NAME to Android, and it will work for cross-compiling with the NDK, but it might not work with the Termux environment. It supposes that you are using the NDK, so the search paths might not work for what they want to do.

This is why I’m asking to try to use the NDK itself inside Termux. It will make the changes minimal.

[finagolfin]

@drodriguez,

I looked a little to setting CMAKE_SYSTEM_NAME to Android, and it will work for cross-compiling with the NDK, but it might not work with the Termux environment.

Can you elaborate a bit more on what you did? What did you set the SWIFT_HOST_VARIANT_SDK and SWIFT_HOST_VARIANT to when you set the CMAKE_SYSTEM_NAME to Android? Given that this project doesn't use the CMake Android toolchain that @compnerd linked and rolls its own NDK support, I suspect the two will clash and not actually work. Maybe CMake didn't error out, but I suspect it would at least fail when building.

It supposes that you are using the NDK, so the search paths might not work for what they want to do.

Depending on what you set the SWIFT_HOST_VARIANT* variables to and the aforementioned potential conflicts with CMake's built-in Android toolchain config, I suspect it wouldn't work for you with the NDK either.

This is why I’m asking to try to use the NDK itself inside Termux. It will make the changes minimal.

I think they're already minimal enough. Take a look at my latest commit, which makes it easy to tell when the NDK is being used. Since you understandably care much more about the NDK paths being easy to follow, I think the latest iteration of this pull does that.

[finagolfin]

__linux__ is defined on Android, so the above block is used.

[finagolfin]

These lit.cfg changes were a hack simply to get lit running natively on Android. I left this in simply to show what was needed: this will need to be changed and done right, after discussion.

I also had to comment out several tests in test/stdlib/InputStream.swift.gyb as they would hang.

[compnerd]

This seems correct actually.

[drodriguez]

It is both correct and incorrect. The checks for run_os being linux-androideabi happens below (L929). For some reason AArch64 uses linux-android instead. In #19503, I modified this file to accept both ARMv7 and AArch64.

I wonder if the replacements from the other branch (or my modified version from that PR) works for your case, but they are intended to be used with a remote Android device, not the host one.

[compnerd]

Please leave a comment explaining that this won't break the Windows -> Linux cross-compilation.

[finagolfin]

Are we sure that it won't? I had not considered that cross-compilation combo, but I guess it won't work if this code path is hit, ie does Windows have uname?

If you try to cross-compile by setting the SWIFT_HOST_VARIANT, then this isn't hit, no problem. However, if you try to cross-compile by setting CMAKE_SYSTEM_NAME to "Linux" on Windows instead, I don't think this will work.

I'm not sure any of this matters since there doesn't appear to be any support for cross-compiling the host tools from Windows to linux right now.

[compnerd]

Windows does not have uname and we are building the android components on Windows too now :-).

[finagolfin]

Windows does not have uname and we are building the android components on Windows too now :-).

Not the host tools like the Swift compiler though, so that situation's different.

In fact, given the inconsistent usage of CMAKE_SYSTEM_NAME and SWIFT_HOST_VARIANT in this repo, I bet nobody has ever actually tried cross-compiling the Swift host tools to any OS.

[drodriguez]

In fact, given the inconsistent usage of CMAKE_SYSTEM_NAME and SWIFT_HOST_VARIANT in this repo, I bet nobody has ever actually tried cross-compiling the Swift host tools to any OS.

I think you just lost a bet :D

But I think that changing this to check for CMAKE_HOST_SYSTEM_NAME, which should be Linux even when cross-compiling from Windows into Linux or any other combination. Unless Saleem has a better proposal.

[compnerd]

They are for both the host tools AND the runtime, not just the runtime. The patches for supporting just the cross-compile of the runtime aren't posted yet. It's not in a single place, you need to read through the entirety of the contents of the toolchain-infrastructure to understand how the build is setup. It would probably be valuable to your goals as well since it would allow you to build what you are after as well.

[finagolfin]

I don't need to read the whole thing: I just spent a couple minutes looking at the top-level Makefile and running github searches for the relevant variables, CMAKE_SYSTEM_NAME and SWIFT_HOST_VARIANT_SDK. It appears that you simply set CMAKE_SYSTEM_NAME to the host OS, in which case you're not cross-compiling the host tools, just like I said.

I suppose it's possible you're overriding the Makefile variables when you run it on the command-line, in which case it's pointless to tell me to look at any of those files, as what matters is what command-line variables you're passing in, not what's in those files.

It looks to me that you don't really understand these matters, with how you kept talking about the NDK when I kept saying I wasn't using it and now point to docs and scripts that don't reference building the host tools at all, all the while claiming they do.

[compnerd]

I think you may want to actually look at my contributions before assuming that it is I who doesn't understand this matter ;-).

The toolchain-infrastructure does cross-compile the host tools to whatever host you want. Look at the codepath that code path that it goes through on BuildHost=Windows-arm64 (yes, I can cross-compile the entire toolchain AND standard library to Windows ARM64 already).

[drodriguez]

Let's don't take that road. This can not end well for anyone. I will leave a longer answer in the main thread.

@buttaface: please believe us, we have cross-compiled the compiler to other platforms.

[finagolfin]

@compnerd, there's no BuildHost variable used in that repo. 😄 Another sign that you're not cross-compiling the Swift compiler itself is that you'd have to also cross-compile LLVM itself, ie have a Windows version of the LLVM libraries built or downloaded on linux to link the Swift build for Windows against, but there's no mention of using a different LLVM in that repo.

Perhaps you have done what you claim, ie cross-compiling the Swift compiler itself for Windows on a linux host, but the links you gave don't support that claim. Maybe you have additional patches or CMake config locally that you're using for cross-compiling the Swift compiler itself and haven't pushed them to github.

Anyway, getting back to the original point of this thread, the Swift CMake config for building the host tools currently uses two variables, CMAKE_SYSTEM_NAME and SWIFT_HOST_VARIANT, to signify pretty much the same thing. That will work as long as they're both set to the same OS, but is error-prone because someone might accidentally set them to different OS's and this Swift host build will then go haywire.

[compnerd]

Please don't set this variable, we want this to be specified by the user since there is no standard location for it.

[finagolfin]

I believe it can still be overridden by the user, if passed in by CMake? Given that all subsequent uses of this variable in this pull are keyed to this Termux path and it's likely the only popular build environment on Android, this seems a reasonable default.

[compnerd]

Yes, it can be overridden by making it a cached variable

[finagolfin]

It's already a cached variable now, so I guess nothing needs to be changed.

[compnerd]

We can initialize ANDROID_NATIVE_SYSROOT to the NDK path as we used to if you like, but, I think that computing it is better as it allows for cross-compiling to different target architectures.

[finagolfin]

I'm not sure what you mean, but I'm guessing this is because you think SWIFT_ANDROID_NDK_PATH is set and being used by this pull. I pointed out in a separate comment above that I'm not using it.

[compnerd]

Right, I am suggesting that we figure out how to get that that setup properly. This will enable us to share the codepath for the cross-compile and native compile paths.

[finagolfin]

Even if I got rid of ANDROID_NATIVE_SYSROOT and set SWIFT_ANDROID_NDK_PATH to the same Termux path on Android, instead of leaving the latter uninitialized like I am now, the paths used for headers and libraries are different in the Termux app than the NDK, so the different CMake config here would still be needed.

[compnerd]

This is wrong; you MUST use the swift variant libraries as the versions on Android are not guaranteed to meet the requirements.

[drodriguez]

Agree. IIRC those libraries are distributed with Android, but they are not part of the NDK, so they should not be considered public. The problem with ICU is that the version number is baked into the symbols, so if you compile in your machine, it might be version 59, but someone tries to use it one year from now, and the package will not work, because their ICU libraries provide version 61 symbols. That's why Swift decides to ship their own ICU (it needs the different name to avoid loading the system libraries).

[finagolfin]

That's why Swift decides to ship their own ICU

Yes, I ran into this early on, maybe before I installed the libicu-dev Termux package mentioned above, but CMake now automatically finds and links libswiftCore.so against that local ICU 63.1 package, so this should be fine.

[compnerd]

There is no guarantee about the version being shipped, so, it should always use the locally built version.

[finagolfin]

It is, the ICU version that ships with Termux's libicu package. Since Termux comes with its own ICU with a known version, that's what it's using. It isn't using the Android system ICU in /system/lib64, which can vary based on which Android version is installed.

[drodriguez]

That explains things. If you plan to distribute this, however, you are forced to fix onto a ICU version package. If a 3rd party users upgrades ICU, but not the Swift package, things will break anyway. I also don’t know how Termux sets things up, but IIRC this line will not link to the full path of the libraries, so they will be searched in the runtime search path, and the system ones might be the first one to be loaded (because their name is compatible with Termux name). You had a phrase about some test failing, I wonder if those are the ones that tries to use ICU symbols (I think there are some in the stdlib, and many more in Foundation).

[finagolfin]

That explains things. If you plan to distribute this, however, you are forced to fix onto a ICU version package.

Yep.

If a 3rd party users upgrades ICU, but not the Swift package, things will break anyway.

Sure, but Termux can always bump the Swift package if that happens, just as they recently bumped a bunch of packages when they upgraded all the libraries they pull from the NDK to the latest.

This is a known and easily solved problem, and no different than how the official build of Swift for Ubuntu 18.04 links against Ubuntu's system ICU 60 package, rather than shipping its own ICU.

I also don’t know how Termux sets things up, but IIRC this line will not link to the full path of the libraries, so they will be searched in the runtime search path, and the system ones might be the first one to be loaded (because their name is compatible with Termux name).

Termux adds the its lib path to LD_LIBRARY_PATH, so it will always find the Termux ICU package first:

termux$ env | grep LD_LIBRARY_PATH
LD_LIBRARY_PATH=/data/data/com.termux/files/usr/lib

Even if it didn't, the FindICU CMake config finds the local Termux ICU libraries and links against those, which are symbolic links to the versioned libraries like libicuuc.so.63, so the linker adds those versioned library names to the list too.

Searching the test results now, one does fail when it resets LD_LIBRARY_PATH, because it doesn't find the versioned ICU library:

: 'RUN: at line 65';   env LD_LIBRARY_PATH=/abc/ '/data/data/com.termux/files/home/src/swift/build/swift/bin/swift' -### -target x86_64-unknown-linux-gnu -L/foo/ -L/bar/ /data/data/com.termux/files/home/src/swift/swift/test/Driver/options-interpreter.swift | /data/data/com.termux/files/home/src/swift/swift/utils/PathSanitizingFileCheck --sanitize 'BUILD_DIR=/data/data/com.termux/files/home/src/swift/build/swift' --sanitize 'SOURCE_DIR=/data/data/com.termux/files/home/src/swift/swift' --use-filecheck '/data/data/com.termux/files/home/src/swift/build/llvm/bin/FileCheck' -check-prefix=CHECK-LINUX-COMPLEX${LD_LIBRARY_PATH+_LAX} /data/data/com.termux/files/home/src/swift/swift/test/Driver/options-interpreter.swift
--
Exit Code: 2

Command Output (stderr):
--
CANNOT LINK EXECUTABLE "/data/data/com.termux/files/home/src/swift/build/swift/bin/swift": library "libicudata.so.63" not found

Not a big deal, you just can't reset the LD_LIBRARY_PATH like that in Termux.

You had a phrase about some test failing, I wonder if those are the ones that tries to use ICU symbols (I think there are some in the stdlib, and many more in Foundation).

I would imagine many more tests fail with informative errors like the one above if that were the case, but a grep for icu only turned up that one. I suppose it's possible that the validation test suite doesn't really check calling ICU functions anywhere, but that's not possible, is it?

Again, you're putting a lot of thought into a problem that the Termux devs already deal with regularly, and is no different than how Swift already packages its linux release.

[compnerd]

I think that the existing paths should work or the android native compilation as well.

[finagolfin]

Not sure what you mean, the NDK uses a different path layout than the Termux app.

[compnerd]

We should figure out how to map the paths appropriately and use that to have a specialised build. Alternatively, we can refactor this out into a toolchain.

[finagolfin]

We should figure out how to map the paths appropriately and use that to have a specialised build.

Isn't that what I'm doing here? I'm specifying different paths based on whether it's the NDK being used or the Termux app.

Alternatively, we can refactor this out into a toolchain.

It's easier to just build off this existing work, but that may make sense at some point.

[compnerd]

Same as above

[compnerd]

This seems wrong, since this is always android specific?

[compnerd]

This is wrong, the driver should behave the same irrespective of being hosted or cross.

[finagolfin]

The reason it doesn't set an RPATH now is because it assumes cross-compilation, and you don't know what path an app and its Swift shared libraries will be installed to. However, if you're natively compiling on Android, you know the path to libswiftCore.so and so on, so the RPATH should be set for Swift executables and the like.

[compnerd]

The path of the target environment should be known because we know the layout of android. So it should be the same everywhere.

[finagolfin]

I'm not getting through, let me go into more detail. There are two possibilities on Android, since it doesn't ship with Swift.

1. You cross-compile the Swift shared libraries, libswiftSwiftOnoneSupport.so and libswiftCore.so, and bundle them with your Android app in its local directory. This is the current default because the Swift compiler cannot know the RPATH, as every app has a different local directory based on its name. That's why it currently doesn't set it.

2. You're natively compiling with Swift in the Termux app. In that case you know exactly where the needed shared libraries are in Termux and would like the Swift compiler to simply add the RPATH for any Swift executables or libraries you build on Android. That's what this patch does. You may not want it to do so if you're natively building Swift shared libraries for an Android app that you then want to install in its own non-Termux path, but you can presumably override the compiler then.

In either case, "the layout of android" is irrelevant, since Android doesn't ship with Swift.

[drodriguez]

I don’t know what's the final intention of having the compiler running in Android. If it is just to “play” with it inside Termux, it might make sense to have rpaths, but if you intent to distribute anything built with that compiler, it should not have rpath.

In my opinion, this should be kept as false, since it is the only value that can be overridden from the command line. If you set this to true, and then you want to distribute the generated binaries, the embedded rpaths will have to be changed.

[finagolfin]

I would really prefer not to have this in the code for the reasons we try to explain above. Baking the rpath to the build-local products is plain wrong.

I don't understand those reasons, as every other Unix build of the Swift compiler bakes the rpath in non-static executables and shared libraries. This would merely do the same for the Swift compiler running in Termux. As I explained above, the only reason the Android target currently leaves off the rpath is because Swift doesn't ship with Android in /system/lib64, so you'll usually bundle the Swift stdlib with an Android app and place it in an arbitrary app path when cross-compiling. That's not the case when compiling Swift code on Android itself, hence this patch which only affects the Swift compiler when it runs on an Android host.

I have found a new reason. The Android linker (Bionic) doesn’t support RPATH… so I really don’t think this can work as you want. Are you sure this is working?

Apparently, DT_RUNPATH is supported by the Bionic linker since Android 7, which is what the gold linker generates when passed an rpath in the Android 8 device I'm using for testing. That is one of the reasons the Termux devs are looking at dropping support for Android 5 and 6 (well, really putting it into maintenance mode with no new packages).

Given the low share of Android 5/6, it's better to support this for Android 7+.

[drodriguez]

Nice that they started supporting RPATH. And if you are fine only supporting Android 7, I suppose you can live with that.

About the topic of RPATH.

CMake builds in one directory, and then installs in another directory. For the full build of Swift, some pieces are build against the Swift in the build directory, and some other later pieces are build against the installation directory (I think products that use swiftpm). The build directory is tied to the machine that builds the code, while the install directory is sometimes for every machine in the platform (but doesn't need to be).

What this option do is adding a -rpath <swift shared runtime library path> (this is one path that will probably end in lib/swift/android/). When Swift is building, the Swift compiler is being used to compile some parts of the standard library, so those libraries will end up in the build directory with an rpath pointing to the build directory (which is for your machine).

When CMake installs the results into the install directory, it will simply copy the Swift runtime libraries, which will still point to the build directory. If the build directory is still there, the libraries will work. However, if you start using the compiler in the install directory, the rpath of those binaries will point to the install directory. This means that in the case of Swift, libswiftCore.so or libFoundation.so points to the build directory, but things that build with SwiftPM will point to the install directory. If you remove the build directory, or move the install directory, things start to fail.

Then there's the CMake script, which also adds its own RPATH here and there (using a hardcoded $ORIGIN:/usr/lib/swift/linux for Linux, for example, in which $ORIGIN is great because is relocatable, but the second part is 🤦‍♂️), and the test script which adds even more.

At some point is not even funny all the patches on top of patches that exist for rpath.

My recommendation: do not switch this. It will only make things worse. If you want to apply a local patch, go for it, I cannot stop you, but please don’t try to submit this here.

[finagolfin]

There are some misconceptions about how rpath is currently used. I just spent some time investigating, here are my results:

What this option do is adding a -rpath (this is one path that will probably end in lib/swift/android/). When Swift is building, the Swift compiler is being used to compile some parts of the standard library, so those libraries will end up in the build directory with an rpath pointing to the build directory (which is for your machine).

No, this doesn't happen, the current CMake config is smarter than this. I haven't dug through all the stdlib CMake config to see where this is set, but the Swift runtime/stdlib doesn't have any DT_RUNPATH set for me on Android. Deleting lib/swift/android/aarch64/libswiftGlibc.so and building it again with ninja -v shows the reason is that CMake calls the local C++ compiler to link that and other Swift libraries, not the Swift compiler itself, so it avoids that potential rpath problem you're worried about.

If you remove the build directory, or move the install directory, things start to fail.

The former doesn't happen, as the rpath's not set to the build directory.

What you may be thinking of is that it does seem to add the INSTALL_RPATH later in this pull to the Swift libraries' rpath- which I gleaned by checking the shared libraries in the official Swift release for linux, I don't set that install rpath for native builds on Android itself- but since that is separate and set explicitly by the developer to the preferred install lib path, it is a different matter from this rpath invocation by the Swift compiler.

Then there's the CMake script, which also adds its own RPATH here and there (using a hardcoded $ORIGIN:/usr/lib/swift/linux for Linux, for example, in which $ORIGIN is great because is relocatable, but the second part is man_facepalming), and the test script which adds even more.

I actually don't mind the INSTALL_RPATH being set to /usr/lib/swift/linux on linux, as that could be easily changed by a packager and serves as an example. Perhaps it should be removed for the official Swift release for linux, since that's a standalone build normally run from the user's directory, but not a big deal.

At some point is not even funny all the patches on top of patches that exist for rpath.

It is true that some extraneous paths have snuck into a few shared libraries' rpaths, here's one from the official Swift 4.2.2 release for Ubuntu 18.04:

> readelf -d swift-4.2.2-RELEASE-ubuntu18.04/usr/lib/swift/linux/libXCTest.so | grep RUNPATH
0x000000000000001d (RUNPATH)            Library runpath: [/home/buildnode/jenkins/workspace/oss-swift-4.2-package-linux-ubuntu-18_04/build/buildbot_linux/swift-linux-x86_64/lib/swift/linux:$ORIGIN]

While this default rpath added by the Swift compiler may have a role to play in that, it can be done right, as we see most of the main shared libraries shipped with the official Swift release doing.

My recommendation: do not switch this. It will only make things worse. If you want to apply a local patch, go for it, I cannot stop you, but please don’t try to submit this here.

I understand your concern but I think it's overblown. I'd rather have a Swift compiler that produces working executables as soon as it's built, rather than worry about a few extraneous rpaths here and there. This is how this project configures the Swift compiler for every other non-Windows native toolchain, I see no reason the native Android toolchain should differ. I completely understand why you don't want the rpath for cross-compilation to Android though, which is why this patch keeps rpath disabled there.

I still think this patch should go in for the native Android build of the Swift compiler.

[drodriguez]

In the case of LLVM/Clang, since they also don’t add rpaths to Android, the problem is handled in a local patch to Termux (https://github.com/termux/termux-packages/blob/master/packages/libllvm/tools-clang-lib-Driver-ToolChains-Linux.cpp.patch). Wouldn’t it be better for the Termux community that every compiler is dealt the same? From an outsider point of view, I really think the decisions of a distro/packaging system should not be imposed into the packaged software.

[finagolfin]

Except this patch has nothing to do with the "distro/packaging system"- note that this default rpath patch has nothing to do with Termux, it doesn't explicitly set the Termux sysroot like the clang patch you link- which is why every other Unix toolchain for Swift sets a default rpath too, as I keep pointing out. Really, it's about "those who want to try compiling the Swift project on Android itself and running its tests there," as I pointed out earlier in this thread, where no Termux packaging patch is going to help you. Now, as I already pointed out, it will also help with a mooted Termux package of the Swift compiler, but I can always add this patch to the Termux package repo for that.

This patch is really about getting the native Android toolchain for Swift in line with every other Unix toolchain for Swift, which will help those building the Swift compiler itself on Android, as I have.

[compnerd]

This seems correct actually.

[drodriguez]

Can you explain what you are trying to do? From my understanding, you want to build a package for the Swift compiler in Termux, right? And you want to compile inside Termux? Can it not be compiled against the NDK (even inside Termux)? It might make things easier to modify.

One thing that I see all across the changes is the checks for the SWIFT_HOST_VARIANT everywhere, and then duplicated branches. This make the scripts harder to read (they are already impossible to read, not your fault). It would be nice to find out the reasons for the duplication, and hopefully remove some or all of them. I’m actually amazed that the Android NDK provided sysroot differs that much.

What is the plan for the patches that you talk about in the description of the PR? The first patch can be avoided by using --nostdlib++ and linking against the right name manually (and probably add -lm). You can see the same in AddSwift.cmake around L440 to cross-compile the runtime libraries against Android. The second one can be also avoided by providing those paths to the build script. That should avoid bringing environment specific patches to the Apple clang sources.

[drodriguez]

It is both correct and incorrect. The checks for run_os being linux-androideabi happens below (L929). For some reason AArch64 uses linux-android instead. In #19503, I modified this file to accept both ARMv7 and AArch64.

I wonder if the replacements from the other branch (or my modified version from that PR) works for your case, but they are intended to be used with a remote Android device, not the host one.

[drodriguez]

Agree. IIRC those libraries are distributed with Android, but they are not part of the NDK, so they should not be considered public. The problem with ICU is that the version number is baked into the symbols, so if you compile in your machine, it might be version 59, but someone tries to use it one year from now, and the package will not work, because their ICU libraries provide version 61 symbols. That's why Swift decides to ship their own ICU (it needs the different name to avoid loading the system libraries).

[finagolfin]

@compnerd, to be clear, SWIFT_ANDROID_NDK_PATH is unset when natively compiling on Android (see my CMake command above, it's not passed in), that's why ANDROID_NATIVE_SYSROOT is needed. What the Termux app does is bundle together all the needed headers and libraries from the latest NDK and install them in the local path /data/data/com.termux/files/usr/{bin,include,lib} when you install its clang package. That's why this pull replaces the NDK paths with sysroot paths in several places.

[finagolfin]

@drodriguez,

Can you explain what you are trying to do?

For right now, just making it possible to build Swift and run its tests on Android devices by using the Termux app, which also includes ARM Chromebooks that have Android support.

From my understanding, you want to build a package for the Swift compiler in Termux, right? And you want to compile inside Termux? Can it not be compiled against the NDK (even inside Termux)? It might make things easier to modify.

Not exactly. The Android NDK is only distributed for popular desktop OSs using x86 chips. The Termux app takes many of those files from the NDK and combines them with a mostly stock clang built to run on Android itself and makes them available as installable packages that you can use to build large C/C++ projects on your Android device, which is how I'm building LLVM, clang, and Swift by using the commands pasted above.

This pull is only about making that possible, ie building the Swift project on Android. Eventually, I will look into making a Swift package for Termux, which entails cross-compiling both the Swift compiler and stdlib for Android from x86_64 linux by using the NDK and may require some more changes to this CMake config then.

One thing that I see all across the changes is the checks for the SWIFT_HOST_VARIANT everywhere, and then duplicated branches. This make the scripts harder to read (they are already impossible to read, not your fault). It would be nice to find out the reasons for the duplication, and hopefully remove some or all of them. I’m actually amazed that the Android NDK provided sysroot differs that much.

I see no way around adding them somewhere (which I go into more below), as the paths that the Termux app installs the Android headers, libraries, etc. into is different than the NDK. The Termux devs try to mimic a standard linux layout in the app's local directory at /data/data/com.termux/files, as opposed to the NDK putting some headers/libraries in android-ndk-r19b/sysroot and others elsewhere.

What is the plan for the patches that you talk about in the description of the PR? The first patch can be avoided by using --nostdlib++ and linking against the right name manually (and probably add -lm). You can see the same in AddSwift.cmake around L440 to cross-compile the runtime libraries against Android. The second one can be also avoided by providing those paths to the build script. That should avoid bringing environment specific patches to the Apple clang sources.

You're right that the clang patches linked above should be redundant to some of the CMake changes in this pull, that are enclosed by SWIFT_HOST_VARIANT and that you referenced just above. However, I believe I had to add those local clang patches to build compiler-rt and for when CMake tries to check that C/C++ compilation is working by building small executables with the locally-built clang, ie the one from the apple/swift-clang repo, before running this CMake config.

But once those include and library paths are hard-coded in clang, they shouldn't be needed here, if they're not used at runtime or elsewhere. I wasn't sure if they were, so I just went ahead and modified them here too.

I wasn't sure what to do with the clang patches, but I wasn't planning on submitting them anywhere. Regarding the redundancy, I can minimize it by either

1. keeping the clang patches and removing those directories from this config. It would be good to document these paths here though, say for some other future Android build environment other than Termux, which is what this pull does.

2. removing the clang patches and figuring out how to patch compiler-rt and configure CMake so that the above problems with an unpatched clang go away.

I did neither, as the path of least resistance, 😉 but I'm open to trying one of those. Let me know what you prefer.

[finagolfin]

Reflecting on the use of SWIFT_HOST_VARIANT in this pull some more, I think I can see why it's caused some confusion. There are three Android scenarios that should eventually work when building the Swift compiler and stdlib:

1. Natively compiling the compiler on a non-Android host and then cross-compiling the stdlib for Android from that non-Android host, ie SWIFT_HOST_VARIANT_SDK=LINUX and SWIFT_SDKS includes ANDROID. This already works as long as the host is a linux distro, ie not Android.

2. Natively compiling the compiler and stdlib on an Android host, ie SWIFT_HOST_VARIANT_SDK=ANDROID and SWIFT_SDKS includes ANDROID. This pull adds support for such native compilation on Android.

3. Cross-compiling both the compiler and stdlib for Android from a non-Android host, ie SWIFT_HOST_VARIANT_SDK=ANDROID and SWIFT_SDKS includes Android. This pull doesn't support this.

I had not thought about 3. much and was simply adding support for 2. that will keep 1. working, but given that both 2. and 3. will set the same two SDK variables above, this pull will not work for 3. I will add another commit with some preliminary support for 3. also, at least in terms of reworking how SWIFT_HOST_VARIANT* are used.

Speaking of which, what is the point of having a separate SWIFT_HOST_VARIANT_SDK and SWIFT_HOST_VARIANT, ie a separate variable for the "Deployment SDK" and "Deployment OS" for the host? Is the idea that there might eventually be many host SDKs supported for a single host OS, even though there's only one SDK for each OS now?

[drodriguez]

About building with the NDK:

I know that the NDK is not distributed for Android itself, but the NDK is made up of many pieces, and not all of them are used to compile Swift. IIRC the compiler is not used at all, but the linker might be (that's what the -B argument is for).

My proposal is that you forget about the Termux version of the NDK files, grab the NDK for Linux, which is just a zip file, and decompress it in some point inside Termux, but you keep the layout. The headers and the binary libraries will be in the same places.

The only problem at that point might be the prebuilt binaries. IIRC the build system only needs a linker that can link for Android. One option is installing lld, and forcing Swift to use it (check SWIFT_ENABLE_LLD_LINKER). Another option will be creating symbolink links inside that NDK layout in the right place for your platform. If you look in SwiftConfigureSDK.cmake, depending on CMAKE_HOST_SYSTEM_NAME, one or other subdirectory from prebuilt/ is used. If you create a android-aarch64 and point SWIFT_SDK_ANDROID_ARCH_${arch}_NDK_PREBUILT_PATH there, it will be automatically found. The last option is modifying the build system as you have already done (but only for the linker).

I don’t remember if other pieces of the NDK are used, but I think they are not.

Another possibility might be re-creating the layout of the NDK and point to the right places inside Termux. Again, what I’m trying here is to avoid big changes to the build system that will be difficult to automate for testing.

An improvement to make it easier to future maintainers is be more specific during the checks. I think what you have done with SWIFT_HOST_VARIANT is conceptually right, but it makes everything very weird. I will not mind it that much if the checks were against something very explicit like BUILDING_ANDROID_WITH_TERMUX. That way future maintainers will have an easy way to figure out what the different branches are doing.

[drodriguez]

In fact, given the inconsistent usage of CMAKE_SYSTEM_NAME and SWIFT_HOST_VARIANT in this repo, I bet nobody has ever actually tried cross-compiling the Swift host tools to any OS.

I think you just lost a bet :D

But I think that changing this to check for CMAKE_HOST_SYSTEM_NAME, which should be Linux even when cross-compiling from Windows into Linux or any other combination. Unless Saleem has a better proposal.

[drodriguez]

That explains things. If you plan to distribute this, however, you are forced to fix onto a ICU version package. If a 3rd party users upgrades ICU, but not the Swift package, things will break anyway. I also don’t know how Termux sets things up, but IIRC this line will not link to the full path of the libraries, so they will be searched in the runtime search path, and the system ones might be the first one to be loaded (because their name is compatible with Termux name). You had a phrase about some test failing, I wonder if those are the ones that tries to use ICU symbols (I think there are some in the stdlib, and many more in Foundation).

[drodriguez]

@compnerd: I looked a little to setting CMAKE_SYSTEM_NAME to Android, and it will work for cross-compiling with the NDK, but it might not work with the Termux environment. It supposes that you are using the NDK, so the search paths might not work for what they want to do.

This is why I’m asking to try to use the NDK itself inside Termux. It will make the changes minimal.

[drodriguez]

I don’t know what's the final intention of having the compiler running in Android. If it is just to “play” with it inside Termux, it might make sense to have rpaths, but if you intent to distribute anything built with that compiler, it should not have rpath.

In my opinion, this should be kept as false, since it is the only value that can be overridden from the command line. If you set this to true, and then you want to distribute the generated binaries, the embedded rpaths will have to be changed.

[finagolfin]

Made some changes and rerunning the tests now, will address the comments later today.

[finagolfin]

My proposal is that you forget about the Termux version of the NDK files, grab the NDK for Linux, which is just a zip file, and decompress it in some point inside Termux, but you keep the layout. The headers and the binary libraries will be in the same places.

Why would I do that? I appreciate all the thought you put into this proposal, but it makes no sense to ask Termux users to download the gigantic NDK itself when the few needed NDK files you reference are already available to them in easily installable Termux packages.

Another possibility might be re-creating the layout of the NDK and point to the right places inside Termux. Again, what I’m trying here is to avoid big changes to the build system that will be difficult to automate for testing.

I'm not sure what you consider "big changes" here. There's only about 10 fairly easy changes to the CMake config: most are simply rote addition of header and library paths, as you've noted. I'm also not sure what you're going to "automate for testing:" I expect that these changes will remain untested by a CI, just as for example the FreeBSD CMake config. And these changes should in no way interfere with the regular Android cross-compilation from linux that you're testing automatically now, as they're external to that.

An improvement to make it easier to future maintainers is be more specific during the checks. I think what you have done with SWIFT_HOST_VARIANT is conceptually right, but it makes everything very weird.

See my latest commit, where I got rid of all but one use of SWIFT_HOST_VARIANT.

I will not mind it that much if the checks were against something very explicit like BUILDING_ANDROID_WITH_TERMUX. That way future maintainers will have an easy way to figure out what the different branches are doing.

I thought of adding another build variable like that, but chose to instead check if the needed path variables SWIFT_ANDROID_NDK_PATH or SWIFT_ANDROID_NATIVE_SYSROOT are defined instead, in my latest commit. Let me know what you think.

You guys are putting way too much thought into what is a minor pull, adding 112 net lines to support building the Swift compiler and stdlib on Android itself. Very few people will likely use that, but the pull is small enough that I thought it might be worth upstreaming.

[finagolfin]

Alright, hack gone and put in a check so that lit can differentiate between running on Android or sending the tests to Android using adb.

[drodriguez]

Let's go back to the start and try to keep this conversation well grounded in civility.

@buttaface we are trying to understand what you are trying to do with this, and we are also trying to make the work that we will have to do in the future easier to maintain. Up to this point, compiling the Android SDK involved the Android NDK, and that's why they paths for the Android NDK are present in the build system. This is the way Google want you to build things for Android, and that's why we were using it.

My repeated proposals for you to use the NDK is make it simpler for the build system. If you and us use the same system, there should be no major changes needed to the build system. It is clear to me that you don't want to use the NDK, so I will leave that discussion aside.

The modifications that you propose are then making our lives a little bit more complicated. They live side by side the paths of the build script that we are working on every day. If we need to modify the parts of the script that we are invested in, how do we modify the parts of the script that you provided. As you have stated above, there will be no automated testing, so those parts will rot and get broken over time, and we will break them without even knowing. It is already quite brittle to modify the build script. Our goal is to not make it more brittle (and I know there's a lot of paths that are probably not maintained in that build script, but let's keep the discussion focused).

We only want to make the solution better, and at the same time, we are trying to make it easier to maintain for us in the future. That's why keeping the changes to a minimum will make this PR easier to review.

I will try to review the PR again with open eyes, but please understand that we are trying to work with you.

[finagolfin]

Let's go back to the start and try to keep this conversation well grounded in civility.

I don't see anwhere it wasn't, disagreements aren't uncivil.

The modifications that you propose are then making our lives a little bit more complicated. They live side by side the paths of the build script that we are working on every day. If we need to modify the parts of the script that we are invested in, how do we modify the parts of the script that you provided.

Simple, most of this pull now clearly demarcates what affects building the Swift compiler natively for Android and what affects using the NDK, which is all you care about. There are only about 2-3 places in this pull where both share common code.

Obviously, if you change the former, ie NDK-only CMake config, it's not going to affect natively building on Android at all, nor should you care that the native build won't get a similar update. Same for the latter, it's not your concern that if you change CMake config shared by NDK cross-compilation and my native compilation, that the native Android build might break. That's up to me or whoever is building Swift natively on Android to monitor and fix.

As you have stated above, there will be no automated testing, so those parts will rot and get broken over time, and we will break them without even knowing. It is already quite brittle to modify the build script. Our goal is to not make it more brittle (and I know there's a lot of paths that are probably not maintained in that build script, but let's keep the discussion focused).

Don't worry about breaking the native Android build. Obviously, cross-compiling for Android is much more popular, so just do what you need there. It is up to those building natively on Android to keep that mostly separate CMake codepath working.

We only want to make the solution better, and at the same time, we are trying to make it easier to maintain for us in the future. That's why keeping the changes to a minimum will make this PR easier to review.

Because most of this pull clearly separates the NDK CMake path you're using from the native Android sysroot path I'm using, I don't think it will take any maintenance from you.

If you're worried about changing the few places the CMake config is shared breaking the native Android build, don't. That would be like worrying that changing the overall CMake config would break some obscure platform like Haiku or Fuchsia, 😉 I don't think you're worried about that either.

I will try to review the PR again with open eyes, but please understand that we are trying to work with you.

Sure, I appreciate the review, which has already led to my submitting a second commit, addressing some of your concerns. I have also suggested some ways this pull could be partially redone to further address your concerns (see my last response in that comment), but I never got a response to that.

[finagolfin]

Updated with some last few tweaks. This is ready to go on my end: it works to natively build the Swift compiler and stdlib on Android and has been cleaned up in lit.cfg and a few other places.

[drodriguez]

All the proposals try to move all the added branches into SwiftAndroidSupport.cmake. The intention of this changes are to make all the modifications reside close to each other. If someone has to change something, they might also see the other pieces, and be aware that more changes might be necessary. There are some parts in other files that I'm not sure can reach SwiftAndroidSupport.cmake, but it might be good to explore extracting them there too.

[drodriguez]

Proposal: let's try to move some of the pieces that diverge out of the AddSwift.cmake and into SwiftAndroidSupport.cmake. This will make all the diverging pieces be hidden most of the time.

This can be as simple as:

swift_android_tools_path(${CFLAGS_ARCH} TOOLS_PATH
list(APPEND result "-B" ${TOOLS_PATH})

And then in SwiftAndroidSupport.cmake:

function(swift_android_tools_path arch path_var_name)
if(NOT SWIFT_ANDROID_NDK_PATH STREQUAL "")
  set(${path_var_name} "${SWIFT_SDK_ANDROID_ARCH_${arch}_NDK_PREBUILT_PATH}/${SWIFT_SDK_ANDROID_ARCH_${arch}_NDK_TRIPLE}/bin" PARENT_SCOPE)
elseif(NOT SWIFT_ANDROID_NATIVE_SYSROOT STREQUAL "")
  set(${path_var_name} "${SWIFT_ANDROID_NATIVE_SYSROOT}/usr/bin" PARENT_SCOPE)
else()
  message(SEND_ERROR "Some error string")
endfunction ()

[finagolfin]

Sounds good, same to next two comments in this vein. I'll make the changes and push.

[drodriguez]

Same idea.

In AddSwift.cmake:

swift_android_icu_libraries(icu_link_libraries)
list(APPEND link_libraries "dl" "log" "atomic" ${icu_link_libraries})

In SwiftAndroidSupport.cmake:

function(swift_android_icu_libraries libraries_var_name)
  if(NOT SWIFT_ANDROID_NDK_PATH STREQUAL "")
    set(${libraries_var_name} "icudataswift" "icui18nswift" "icuucswift" PARENT_SCOPE)
  elseif(NOT SWIFT_ANDROID_NATIVE_SYSROOT STREQUAL "")
    set(${libraries_var_name} "icudata" "icui18n" "icuuc" PARENT_SCOPE)
  else()
    message(SEND_ERROR "Some error string")
  endif()
endfunction ()

[drodriguez]

In AddSwift.cmake:

swift_android_cxx_link_libraries_for_arch(${LFLAGS_ARCH} cxx_link_libraries)
list(APPEND link_libraries ${cxx_link_libraries})
swift_android_lib_for_arch(${LFLAGS_ARCH} ${LFLAGS_ARCH}_LIB)
foreach(path IN LISTS ${LFLAGS_ARCH}_LIB)
  list(APPEND library_search_directories ${path})
endforeach()

In SwiftAndroidSupport.cmake:

function(swift_android_cxx_libraries_for_arch arch libraries_var_name)
  set(link_libraries)
  if(NOT SWIFT_ANDROID_NDK_PATH STREQUAL "")
    if("${arch}" MATCHES armv7)
      set(cxx_arch armeabi-v7a)
    elseif("${arch}" MATCHES aarch64)
      set(cxx_arch arm64-v8a)
    else()
      message(SEND_ERROR "unknown architecture (${LFLAGS_ARCH}) when cross-compiling for android")
    endif()

    set(android_libcxx_path "${SWIFT_ANDROID_NDK_PATH}/sources/cxx-stl/llvm-libc++/libs/${cxx_arch}")
    list(APPEND link_libraries "${android_libcxx_path}/libc++abi.a")
    list(APPEND link_libraries "${android_libcxx_path}/libc++_shared.so")
  elseif(NOT "${SWIFT_ANDROID_NATIVE_SYSROOT}" STREQUAL "")
      list(APPEND link_libraries "${SWIFT_ANDROID_NATIVE_SYSROOT}/usr/lib/libc++_shared.so")
  else()
    message(SEND_ERROR "Some error string")
  endif()

  set(${libraries_var_name} ${link_libraries})
endfunction()

function(swift_android_lib_for_arch arch var)
  set(paths)
  if(NOT SWIFT_ANDROID_NDK_PATH STREQUAL "")
    # previous contents of the function, except the lines outside of this if/elseif/else
  elseif()
      if("${arch}" MATCHES armv7)
        list(APPEND paths "/system/lib")
      elseif("${arch}" MATCHES aarch64)
        list(APPEND paths "/system/lib64")
      else()
        message(SEND_ERROR "unknown architecture (${arch}) when natively compiling on android")
      endif()
  else()
    message(SEND_ERROR "Some error string")
  endif()

  set(${var} ${paths} PARENT_SCOPE)
endfunction()

[drodriguez]

LINK_FLAGS is deprecated, and for this case, it would be better to use LINK_DIRECTORIES instead:

set_property(TARGET "${test_dirname}" APPEND PROPERTY LINK_DIRECTORIES "${android_system_libs}")

[finagolfin]

Certainly no CMake expert, will try it.

[drodriguez]

I will add a comment about why "${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" is dealing with pieces from Android.

Either that, or I will separate into a different branch:

elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" AND NOT "${SWIFT_ANDROID_NATIVE_SYSROOT}" STREQUAL "")
  # Natively building in Android reports CMAKE_SYSTEM_NAME as Linux.
  # the code for Android
elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux")
  # the code for Linux, but keep the change to use LINK_LIBRARIES

[finagolfin]

I was trying to keep the surrounding logic the same as much as possible, to avoid raising the SWIFT_HOST_VARIANT/CMAKE_SYSTEM_NAME issue again. Otherwise, I have a third possibility:

elseif("${SWIFT_HOST_VARIANT}" STREQUAL "linux" OR "${SWIFT_HOST_VARIANT}" STREQUAL "android")

Or alternately, separating these two SWIFT_HOST_VARIANT checks like you do in your example.

[drodriguez]

My idea was keeping the Linux part of the code separate. That way the people dealing with Linux can do their modifications without worrying about that piece of Android code in there. I choose to keep comparing with CMAKE_SYSTEM_NAME because all the other checks in this part of the code uses that, so I didn’t want to break the pattern with something like SWIFT_HOST_VARIANT by itself, if you prefer to use that instead of checking for SWIFT_ANDROID_NATIVE_SYSROOT it is fine by me. The branch that has to be introduced has to be before the Linux branch, sadly, but there's little we can do about that.

[finagolfin]

Done, and uses swift_android_lib_for_arch instead now.

[drodriguez]

I will ask you once again to remove this from here. As we have explain above consistency in the resulting products between different platforms is better. Besides that, adding an rpath is something that can be done from the invocation line of the compiler, while removing this rpath means modifying the result ELF in a later step.

[drodriguez]

Can you check if Python’s platform.dist() might give you this information and we avoid the “random” environment variable?

[finagolfin]

platform.dist is deprecated: this SO answer showed people checking env variables instead, so I did the same. It's understandable given the Python devs don't expect it to run on Android.

[drodriguez]

Yes, I read that it was deprecated in 3.5 and going to be removed in 3.8, but right now Swift needs 2.7 to build, so it is not going to affect us (I'm trying to be compatible with 3.5, but we are still far away from that goal, or being able to stop using deprecated methods).

[finagolfin]

Nope, read my two links: deprecated since 2.6 also and didn't give the needed info anyway.

I was happy to see your pull to add support for Python 3 as that was a PITA when configuring the Swift build on linux and Android and currently undocumented, which is why I explicitly called it out above.

[drodriguez]

Sorry, the documentation for the 2.x series and the 3.x series differ, and I was looking at the later.

Use linux_distribution instead. dist was deprecated by renaming to linux_distribution. They are the same function. It is still deprecated in 3.5, but I think we can live with it for a little while.

[finagolfin]

linux_distribution doesn't work on Android either, as it looks for an /etc release file, which Android doesn't have.

ANDROID_DATA in the environment appears to be the best bet, as it's set by the OS itself.

[drodriguez]

I don’t know why I expected something different from Android.

OK, let's try to make this a little bit more clear: all the run_* variables are related to the target (they all come from the variant triple), so I will love to avoid that naming. Around L166 in my copy there’s a variable named kIsWindows which we can use as a pattern for the name.

# The global environment in Android sets ANDROID_DATA,
# so if that variable is set, we are probably running in Android.
kIsAndroid = 'ANDROID_DATA' in os.environ

Hopefully the name and the comment will explain better the intention of the variable.

I would also recommend adding an extra comment between L871 and L872 explaining that when running in Android, and targeting Android, all the NDK and the remote host are not needed, since Android behaves closer to running on Linux for that case.

[finagolfin]

Done.

[finagolfin]

Refactored along the lines @drodriguez suggested and pushed, didn't bother adding another commit as no need to see all the previous reshufflings anymore, just squashed it all into one new commit.

I didn't bother extracting the last two places NDK/Sysroot paths are set outside SwiftAndroidSupport because they're one-liners, but can extract that too if wanted.

[drodriguez]

Two things:

• Please don’t apply RPATH when building with the NDK. Let's try not to change the behaviour when cross-compiling. Also, I don't think the RPATH for cross-compiling can refer to ${SWIFT_ANDROID_NATIVE_SYSROOT}, because that variable will be empty, and /usr/lib doesn't exist in Android (also, the system library paths should not be added to the RPATH, because they are already searched anyway).
• I'm very bad with logic, but I don’t think this branch is going to be taken at all, writing it in a more common language, you are doing if (swift_android_ndk_path != "" and swift_android_native_sysroot != ""), that is, when both SWIFT_ANDROID_NDK_PATH and SWIFT_ANDROID_NATIVE_SYSROOT are set to something. I would recommend changing the check to only check for the native sysroot.

[finagolfin]

See the comment I had written above in this file: this is only going to kick in when the Android NDK is being used to cross-compile the host tools, ie the Swift compiler itself, to run on an Android host, ie Termux. As such, it shouldn't affect you when cross-compiling the Swift runtime/stdlib nor will it add the Android system library paths you mention.

If it wasn't clear already, here's the full Boolean truth table for how SWIFT_ANDROID_NDK_PATH and SWIFT_ANDROID_NATIVE_SYSROOT are used throughout this pull:
Both unset - Android is not being used
only NDK path set - cross-compiling the runtime/stdlib alone
only Sysroot path set - natively compiling the compiler and runtime/stdlib on an Android host, ie in the Termux app like I've been doing
both NDK and Sysroot paths set - cross-compiling both the host tools and runtime/stdlib for an Android host

I only want this install rpath set for that last case, which is how the official Termux packages are built, which is why I check that both are set. Since you only care about the second case, where you don't set the new Android Sysroot path I added for Android host tools, this shouldn't affect you at all.

[drodriguez]

Can you provide a link to some document about how Termux packages are supposed to be built? I find some minimal documentation in the wiki, but nothing related to rpath. If I understand correctly, they are trying to move away from LD_LIBRARY_PATH to use RUNPATH, which (IMO) is not the right thing to do, but if you can point me to any documentation, I will try to understand.

[finagolfin]

The move to rpath is pretty new, I think the best doc is the README link I gave you above, along with the github issue it links to:

It is highly recommended to try to execute programs without
LD_LIBRARY_PATH set. Most of them should continue to run since
DT_RUNPATH field compiled-in ELF binary is used. Though, some
programs are not working due to missing DT_RUNPATH, please
check list in the related issue

That rpath github issue was just closed yesterday, saying that it's all working now.

That said, I'm mainly adding this rpath because that's what this project does for other OS's, as Saleem noted above, though of course Termux is moving to rpath also. As such, this INSTALL_RPATH patch is done speculatively, as I have not cross-compiled the Swift host tools yet.

[drodriguez]

elif kIsAndroid:

[finagolfin]

Done, see latest commit. This pull was tested against the latest 3/10 Swift 5.0 snapshot, cherry-picked to master and then I changed the variable name, so I missed this. However, it was tested with the old variable name, so it should work fine.

[drodriguez]

Nice that they started supporting RPATH. And if you are fine only supporting Android 7, I suppose you can live with that.

About the topic of RPATH.

CMake builds in one directory, and then installs in another directory. For the full build of Swift, some pieces are build against the Swift in the build directory, and some other later pieces are build against the installation directory (I think products that use swiftpm). The build directory is tied to the machine that builds the code, while the install directory is sometimes for every machine in the platform (but doesn't need to be).

What this option do is adding a -rpath <swift shared runtime library path> (this is one path that will probably end in lib/swift/android/). When Swift is building, the Swift compiler is being used to compile some parts of the standard library, so those libraries will end up in the build directory with an rpath pointing to the build directory (which is for your machine).

When CMake installs the results into the install directory, it will simply copy the Swift runtime libraries, which will still point to the build directory. If the build directory is still there, the libraries will work. However, if you start using the compiler in the install directory, the rpath of those binaries will point to the install directory. This means that in the case of Swift, libswiftCore.so or libFoundation.so points to the build directory, but things that build with SwiftPM will point to the install directory. If you remove the build directory, or move the install directory, things start to fail.

Then there's the CMake script, which also adds its own RPATH here and there (using a hardcoded $ORIGIN:/usr/lib/swift/linux for Linux, for example, in which $ORIGIN is great because is relocatable, but the second part is 🤦‍♂️), and the test script which adds even more.

At some point is not even funny all the patches on top of patches that exist for rpath.

My recommendation: do not switch this. It will only make things worse. If you want to apply a local patch, go for it, I cannot stop you, but please don’t try to submit this here.

[drodriguez]

There's another configure_sdk_unix("Android"… below around L774.

If you are cross-compiling the host tools, wouldn’t this branch and the branch below be hit? Wouldn’t that configure twice the same SDK? That probably will create duplicated targets, right?

[finagolfin]

I wasn't sure if I needed to disable that second configure_sdk_unix when cross-compiling and SWIFT_HOST_VARIANT is "android," as I haven't cross-compiled the host tools for Android yet, so I punted on it for now. I will submit a separate pull when I actually get the Swift host tools cross-compiled for Android.

This pull is primarily about natively compiling this project on an Android host and though it tries to anticipate that cross-compilation pull and lay some groundwork, because they have some CMake config in common, this CMake config will probably require further patches then.

[drodriguez]

Can you provide a link to some document about how Termux packages are supposed to be built? I find some minimal documentation in the wiki, but nothing related to rpath. If I understand correctly, they are trying to move away from LD_LIBRARY_PATH to use RUNPATH, which (IMO) is not the right thing to do, but if you can point me to any documentation, I will try to understand.

[drodriguez]

In the case of LLVM/Clang, since they also don’t add rpaths to Android, the problem is handled in a local patch to Termux (https://github.com/termux/termux-packages/blob/master/packages/libllvm/tools-clang-lib-Driver-ToolChains-Linux.cpp.patch). Wouldn’t it be better for the Termux community that every compiler is dealt the same? From an outsider point of view, I really think the decisions of a distro/packaging system should not be imposed into the packaged software.

[drodriguez]

The reasons for this not being accepted and 23742 are several:

• There were two reviewers asking you to remove a piece of code in the compiler code. We are not able to understand your reasons, and we cannot explain our reasons better. We are discussing in circles and going nowhere.
• The other PR was way simpler and easier to check, and it almost accomplished the same results as this one without a lot of changes. Simple is good.
• It uses the NDK to cross compile some tools, as two reviewers asked you to consider for this PR, and the result improves the cross compilation of the runtime in a couple of points too. Wins for everyone.

[drodriguez]

I have checked this PR against the standard way of compiling for Android and it doesn’t seem to break the compilation in its current state.

I have no permission to start the tests myself, and I cannot accept in good faith these changes with the modification to the compiler. If you want to find someone else in reviewing, testing and accepting these changes, I will not oppose.

[finagolfin]

I understand, I knew you "have no permission to start the tests" from looking at other pulls of yours.

I'll see if I can find another reviewer.

[finagolfin]

Since @jrose-apple wrote the original code to add an rpath for the Swift shared library, I emailed him to see if he'll resolve this rpath issue.

I've also been trying to build and run the tests for the Swift package manager and other projects it needs on Arch linux x86_64 with the April 7th source snapshot. I had to tweak three files in Foundation and libdispatch, disabling the statx patch, swiftlang/swift-corelibs-foundation@692e91457, and adding the LIST_FOREACH_SAFE macro to libdispatch.

Everything built fine and all tests passed for CMark, swift, libdispatch, and XCTest. One command buffering test fails for llbuild, a couple pkgconfig tests fail for swiftpm, and three tests were disabled or failed for Foundation.

Similarly, I built and tested the same April 7th snapshot for the same projects natively on Android AArch64, which required several more tweaks to the source. All tests pass for CMark, llbuild, and XCTest. 19 tests fail from the Swift unit and validation test suite, and another 30-40 fail or had to be disabled from the Foundation test suite, many related to handling time zones since that isn't supported yet.

I had to revert libdispatch to the March 17 snapshot, as there have been regressions since then that cause tests to fail in both libdispatch and Foundation, but all libdispatch tests pass after tweaking and using that earlier snapshot.

Finally, I got the first stage of swiftpm bootstrapped, but the second stage of building swiftpm with itself segfaults. I'll look into that next.

[finagolfin]

Alright, got back to this again, after not doing much with it in the spring, other than occasionally rebasing, building, and rerunning the tests.

I finally got SwiftPM built and bootstrapped natively on Android last week, and ran its tests for the first time. It reports 575 tests run and 566 results logged, with 493 passing and 73 failing. Most of the failing tests are related to some Android-specific organizational issues in the SwiftPM build directory, as those tests can't find the source for PackageDescription4, will look into those further.

Here are the test results from building all the Swift repos needed by SwiftPM from the July 1 snapshot, against this pull:

• CMark, llbuild, and XCTest pass all their tests as before, with only llbuild requiring several small patches. libdispatch as of the 7/1 snapshot passes all of its tests, after a couple tweaks to its CMake config.
• Foundation is in the best shape it's been on Android since I started testing it: only 64 failures from 1559 tests run, as compared to 59 failures from 1566 tests run with a stock 6/23 snapshot on Arch linux x86_64. Good to see Android reaching parity with linux, and it only required applying [android] Implement Process and modify TestProcess to pass. swift-corelibs-foundation#2228 and [android] Improve CFKnownLocations. swift-corelibs-foundation#2391, both since merged, and two other tiny tweaks to the 7/1 snapshot: commenting out a system header that messed with ClangImporter and disabling a segfaulting testcase.
• The Swift unit and validation testsuite reports 27 failures out of about 10.5k run, many of which are executable_tests which I don't think are run on the Android CI. Eight of those failing are new tests added in swift_build_sdk_interfaces.py: builds compiled modules from an SDK #25432, because they depend on Python multiprocessing, which doesn't work on Android without sem_open as mentioned initially. Also, I switched over to a new Termux environment and package repository for Android 7+ last month, and linking swift-syntax-parser-test appears to have since broken, along with three tests that run it. Note that one of the clang patches linked in the OP has been updated to add an rpath in this new Termux environment, replacing the prior LD_LIBRARY_PATH, so I had to apply that updated patch to the locally-built clang that Swift uses too.

Of the remaining 16 failing tests, several exit because of linking issues, such as multifile/multiconformanceimpls. With that particular test, I think the problem is that it assumes that the dynamic linker will apply any rpath given to the executable main.out both to any shared libraries its linked against and transitively to any libraries they invoke.

So simply having main.out have the rpath for test-android-aarch64/multifile/multiconformanceimpls/Output/main.swift.tmp from the build directory is enough to find both the shared libraries libC.so and libB.so that it depends on, and those two libraries can find the libA.so that they depend on when run on linux x86_64 employing the glibc dynamic linker. However, the Bionic dynamic linker seems to require that each library lists the rpaths it has dependencies from itself, and will not apply the rpath from main.out when finding the dependencies for libB.so and libC.so, so it won't find libA.so for them.

This is probably why the SwiftPM bootstrap was segfaulting before, when it tried to generate a JSON manifest by interpreting the SwiftPM source, as some of its required shared libraries likely don't have the needed rpaths. I worked around it by not interpreting that source and building an executable instead, then invoking it with the necessary LD_LIBRARY_PATH, which the Bionic dynamic linker does apply transitively, to generate the JSON manifest, then read that instead.

Five more tests fail because they can't find some sanitizers, such as Sanitizers/asan, will have to check those.

In addition, I had to apply the following hacky patch to get Swift built and to use build-script natively on Android:

diff --git a/stdlib/public/Platform/bionic.modulemap.gyb b/stdlib/public/Platform/bionic.modulemap.gyb
index 6119bbc7c3..5302069197 100644
--- a/stdlib/public/Platform/bionic.modulemap.gyb
+++ b/stdlib/public/Platform/bionic.modulemap.gyb
@@ -159,10 +159,10 @@ module SwiftGlibc [system] {
       header "${GLIBC_INCLUDE_PATH}/ftw.h"
       export *
     }
-    module glob {
-      header "${GLIBC_INCLUDE_PATH}/glob.h"
-      export *
-    }
+//    module glob {
+//      header "${GLIBC_INCLUDE_PATH}/glob.h"
+//      export *
+//    }
     module iconv {
       header "${GLIBC_INCLUDE_PATH}/iconv.h"
       export *
@@ -215,6 +215,14 @@ module SwiftGlibc [system] {
       header "${GLIBC_INCLUDE_PATH}/fcntl.h"
       export *
     }
+    module bits {
+      export *
+
+    module fcntl {
+      header "${GLIBC_INCLUDE_PATH}/bits/fcntl.h"
+      export *
+    }
+    }
     module fnmatch {
       header "${GLIBC_INCLUDE_PATH}/fnmatch.h"
       export *
@@ -285,14 +293,14 @@ module SwiftGlibc [system] {
         header "${GLIBC_ARCH_INCLUDE_PATH}/sys/file.h"
         export *
       }
-      module sem {
-        header "${GLIBC_ARCH_INCLUDE_PATH}/sys/sem.h"
-        export *
-      }
-      module shm {
-        header "${GLIBC_ARCH_INCLUDE_PATH}/sys/shm.h"
-        export *
-      }
+//      module sem {
+//        header "${GLIBC_ARCH_INCLUDE_PATH}/sys/sem.h"
+//        export *
+//      }
+//      module shm {
+//        header "${GLIBC_ARCH_INCLUDE_PATH}/sys/shm.h"
+//        export *
+//      }
       module inotify {
         header "${GLIBC_ARCH_INCLUDE_PATH}/sys/inotify.h"
         export *
diff --git a/test/Driver/multi-threaded.swift b/test/Driver/multi-threaded.swift
index 6afa7da998..9f461fe775 100644
--- a/test/Driver/multi-threaded.swift
+++ b/test/Driver/multi-threaded.swift
@@ -5,7 +5,7 @@
 // RUN: %target-swiftc_driver -driver-print-jobs -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -c | %FileCheck -check-prefix=OBJECT %s
 // RUN: cd %t && %target-swiftc_driver -parseable-output -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -c 2> %t/parseable-output
 // RUN: cat %t/parseable-output | %FileCheck -check-prefix=PARSEABLE %s
-// RUN: cd %t && env TMPDIR=/tmp %swiftc_driver -driver-print-jobs -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -o a.out | %FileCheck -check-prefix=EXEC %s
+// RUN: cd %t && env %swiftc_driver -driver-print-jobs -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -o a.out | %FileCheck -check-prefix=EXEC %s
 // RUN: echo "{\"%/s\": {\"llvm-bc\": \"%/t/multi-threaded.bc\", \"object\": \"%/t/multi-threaded.o\"}, \"%/S/Inputs/main.swift\": {\"llvm-bc\": \"%/t/main.bc\", \"object\": \"%/t/main.o\"}}" > %t/ofmo.json
 // RUN: %target-swiftc_driver -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s  -emit-dependencies -output-file-map %t/ofmo.json -c
 // RUN: cat %t/*.d | %FileCheck -check-prefix=DEPENDENCIES %s
diff --git a/tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake b/tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake
index 87af05b99e..1725171c52 100644
--- a/tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake
+++ b/tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake
@@ -177,7 +177,7 @@ macro(add_sourcekit_library name)
   if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux")
     if(SOURCEKITLIB_SHARED)
       set_target_properties(${name} PROPERTIES BUILD_WITH_INSTALL_RPATH TRUE)
-      set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/../lib/swift/linux:/usr/lib/swift/linux")
+      set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/../lib/swift/android")#:/usr/lib/swift/linux")
     endif()
   endif()
 
diff --git a/utils/build-script b/utils/build-script
index 44a31ed9ac..757e8f5515 100755
--- a/utils/build-script
+++ b/utils/build-script
@@ -186,7 +186,7 @@ class BuildScriptInvocation(object):
         android_tgts = [tgt for tgt in args.stdlib_deployment_targets
                         if StdlibDeploymentTarget.Android.contains(tgt)]
         if not args.android and len(android_tgts) > 0:
-            args.android = True
+            #args.android = True
             args.build_android = False
 
         # Include the Darwin supported architectures in the CMake options.
@@ -550,6 +550,7 @@ class BuildScriptInvocation(object):
                 "--android-icu-i18n-include", args.android_icu_i18n_include,
                 "--android-icu-data", args.android_icu_data,
             ]
+        impl_args += ["--android-api-level", args.android_api_level]
         if args.android_deploy_device_path:
             impl_args += [
                 "--android-deploy-device-path",
diff --git a/utils/build-script-impl b/utils/build-script-impl
index 54702c1768..38f8e60e8f 100755
--- a/utils/build-script-impl
+++ b/utils/build-script-impl
@@ -451,6 +451,11 @@ function set_build_options_for_host() {
             SWIFT_HOST_VARIANT_SDK="HAIKU"
             SWIFT_HOST_VARIANT_ARCH="x86_64"
             ;;
+        android-aarch64)
+            SWIFT_HOST_VARIANT="android"
+            SWIFT_HOST_VARIANT_SDK="ANDROID"
+            SWIFT_HOST_VARIANT_ARCH="aarch64"
+            ;;
         linux-*)
             SWIFT_HOST_VARIANT="linux"
             SWIFT_HOST_VARIANT_SDK="LINUX"
@@ -718,6 +723,7 @@ function set_build_options_for_host() {
     llvm_cmake_options+=(
         -DLLVM_TOOL_COMPILER_RT_BUILD:BOOL="$(false_true ${SKIP_BUILD_COMPILER_RT})"
         -DLLVM_BUILD_EXTERNAL_COMPILER_RT:BOOL="$(false_true ${SKIP_BUILD_COMPILER_RT})"
+        -DLLVM_DEFAULT_TARGET_TRIPLE=aarch64-unknown-linux-android
     )
 
     # If we are asked to not generate test targets for LLVM and or Swift,
@@ -2372,6 +2378,7 @@ for host in "${ALL_HOSTS[@]}"; do
                     -DSWIFT_TOOLS_ENABLE_LTO:STRING="${SWIFT_TOOLS_ENABLE_LTO}"
                     -DSWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER:BOOL=$(true_false "${BUILD_RUNTIME_WITH_HOST_COMPILER}")
                     -DLIBDISPATCH_CMAKE_BUILD_TYPE:STRING="${LIBDISPATCH_BUILD_TYPE}"
+                    -DSWIFT_ANDROID_API_LEVEL:STRING="${ANDROID_API_LEVEL}"
                     "${swift_cmake_options[@]}"
                 )
 
diff --git a/utils/build_swift/driver_arguments.py b/utils/build_swift/driver_arguments.py
index ff85dff606..d6e660a33b 100644
--- a/utils/build_swift/driver_arguments.py
+++ b/utils/build_swift/driver_arguments.py
@@ -238,7 +238,7 @@ def _apply_default_arguments(args):
         args.test_watchos_simulator = False
 
     if not args.build_android:
-        args.test_android = False
+        args.test_android = True
         args.test_android_host = False
 
     if not args.test_android:
diff --git a/utils/swift_build_support/swift_build_support/targets.py b/utils/swift_build_support/swift_build_support/targets.py
index 5ec02dfcc7..975de5b90c 100644
--- a/utils/swift_build_support/swift_build_support/targets.py
+++ b/utils/swift_build_support/swift_build_support/targets.py
@@ -186,6 +186,14 @@ class StdlibDeploymentTarget(object):
         machine = platform.machine()
 
         if system == 'Linux':
+            if 'ANDROID_DATA' in os.environ:
+                if machine.startswith('armv7'):
+                    return StdlibDeploymentTarget.Android.armv7
+                elif machine == 'aarch64':
+                    return StdlibDeploymentTarget.Android.aarch64
+                raise NotImplementedError('Android System with architecture "%s" is not '
+                                          'supported' % machine)
+
             if machine == 'x86_64':
                 return StdlibDeploymentTarget.Linux.x86_64
             elif machine == 'i686':

The bionic modulemap changes were made because Termux doesn't supply three of those headers, and a header added because of a long-standing ClangImporter bug, to which I added this info and a linux testcase a couple months ago. A test was fixed by using TMPDIR from the Termux environment instead, and the right rpath hacked into SourceKit for Android. The remaining hacks got build-script working on Android, so that I just had to run the following command to build all these repos and run their tests, by avoiding the standard Android cross-compilation codepath:

./swift/utils/build-script -R --no-assertions --llvm-targets-to-build="X86;ARM;AArch64" -j9 --xctest -b -p -T

Of course, that means one or two of the build-script tests fail. I'll get this build-script portion and some of the above patches cleaned up and submitted later.

Regarding the default rpath argued about earlier, I stumbled upon an issue where it was discussed a couple years ago, when looking for where to report the above ClangImporter bug. I didn't know the default rpath was such a controversial matter, with it being discussed there how to disable it when needed. I think having a default rpath is the right choice, but I agree that there should also be a flag to disable or configure it, so I've simply removed the default rpath from this pull.

I've also updated this pull in a few places, as some of it wasn't needed after recent changes, such as #25491. I've put the last version of this pull up at finagolfin/swift@a93550c for easy comparison.

This pull should now be ready to merge, after which I'll look into submitting the llbuild, libdispatch, and other patches mentioned above and cross-compiling everything using the Termux build scripts and submitting a Termux package for Swift.

[finagolfin]

Just updated to remove the native sysroot's library directory from this rpath list, as the clang in the new Termux environment adds it already. Once there's a Swift Termux package and it's decided where it will store libraries, I'll probably add that path here.

[drodriguez]

@buttaface: let me check this applies and doesn't break anything in the CI setup of Android. I will read your comment later.

[drodriguez]

libdispatch as of the 7/1 snapshot passes all of its tests, after a couple tweaks to its CMake config.

I normally cross-compile libdispatch for Android without any changes. I imagine that Termux might be the reason for the changes, since I remember that it reports itself as a Linux distribution. Do you have this changes published somewhere? If you send a PR to s-c-libdispatch, would you mind mentioning me? Thanks.

Foundation [...] only 64 failures from 1559 tests run […] two other tiny tweaks to the 7/1 snapshot: commenting out a system header that messed with ClangImporter and disabling a segfaulting testcase.

My failure count for Foundation is just around 10. The failing tests are normally related to the file system like for example the ones fixed in swiftlang/swift-corelibs-foundation#2145. There are another couple of failures in some XML tests, but those are because the libxml2 that I build doesn't link against libiconv, which might not be your case.

Can you create a gist of the failing tests and the output of running TestFoundation? It will be interesting to know which test actually fail for other Androids. It is going to be hard, but I normally test in one or two devices, so it will be nice to know what other devices do not like. I would also like to know about those changes: what system header? what segfaulting test case?

The Swift unit and validation testsuite reports 27 failures out of about 10.5k run, many of which are executable_tests which I don't think are run on the Android CI.

I will try to run the executable tests again. In case I don't get that many failures, it would be nice to create another gist with those failing tests and their outputs.

I think the problem is that it assumes that the dynamic linker will apply any rpath given to the executable main.out both to any shared libraries its linked against and transitively to any libraries they invoke.

It might be that Android is setting and using RUNPATH, which is a very close sibling to RPATH, but not the same. Sadly they use the same set of switches in the compilers, and depending on the linkers you might have one, the other, or both in your binaries. One difference is that, for security reasons, RUNPATH doesn't apply to finding indirect dependencies of the binary that has the value. I don’t know the test in particular, but something that should work across platforms should be setting explicit RUNPATH in all the binaries of the test.

I had to apply the following hacky patch to get Swift built and to use build-script natively on Android

There are some parts of that patch that are surprising: why Termux doesn’t ship with glob.h, sys/sem.h, or sys/shm.h? Those are probably omissions on the Terms side, but I cannot be sure (I'm looking at NDKr16, r17, r18, and r20 btw). Is Termux still using an older NDK?

I will wait in case someone says other thing, but the part about bits/fcntl.h should not be in the module map. The bits/ directory is supposed to be an internal detail, so importing it in the module map doesn't seem to be the better solution. I tried to read the bug report and it seems to me the problem is both the module for fcntl and unistd do an export * (which is normally recommended for C libraries), but since they both include bits/fcntl.h, the contents of it are considered to be in both modules, which cannot be imported in the same translation unit. Remove the export * from one of those modules might remove the problem, but will probably create others. IMO, unistd.h should actually import fcntl.h instead of bits/fcntl.h, but I'm not sure.

The part in test/Driver/multi-threaded.swift is something that can be fixed better, I think. I don’t know why TMPDIR is reset there, but it would be better to use a subdirectory inside %t as destination. That should work in any system just fine.

The other parts seems very tied to Termux reporting itself as Linux, instead of a more helpful Android. It would be nice to find cleaner alternatives to those changes (some of them are going to break Linux builds, some other break the Android builds).

@swift-ci please smoke test

[finagolfin]

The linux smoke test failure appears unrelated to this pull. I just updated to the July 28 snapshot of these repos and reran everything with this pull, so all results below refer to that updated run.

I normally cross-compile libdispatch for Android without any changes. I imagine that Termux might be the reason for the changes, since I remember that it reports itself as a Linux distribution.

Not Termux, but CMake when run natively on Android reports the host OS as Linux, as noted early in this pull.

Do you have this changes published somewhere?

No, but I've put it up as a gist now, pretty much what you'd expect since CMAKE_SYSTEM_NAME is set to Linux, with only a couple more log linker flags added.

The main issue is that build-script doesn't support cross-compiling anything other than this repo yet, ie libdispatch, Foundation, and so on: has any decision been made on how to handle that? I mentioned early in this pull discussion that CMAKE_SYSTEM_NAME and SWIFT_HOST_VARIANT are used interchangeably in the Swift repos, and build-script only sets the latter.

If one way is decided, I can modify that patch to use it before submitting.

My failure count for Foundation is just around 10.

I'm down to 49 failures from 1628 tests run on Android, as opposed to 51 failures from 1632 tests run on linux x86_64, both with the 7/28 snapshot.

There are another couple of failures in some XML tests, but those are because the libxml2 that I build doesn't link against libiconv, which might not be your case.

The Termux-supplied libxml2 links against libiconv, yes.

Can you create a gist of the failing tests and the output of running TestFoundation? It will be interesting to know which test actually fail for other Androids.

I've posted the full log output, along with the linux x86_64 output for comparison, both for the 7/28 snapshot though the latter is unpatched.

I would also like to know about those changes: what system header? what segfaulting test case?

Here's my full patch for the 7/28 snapshot, which produced the above linked test output:

diff --git a/CoreFoundation/Base.subproj/SwiftRuntime/CoreFoundation.h b/CoreFoundation/Base.subproj/SwiftRuntime/CoreFoundation.h
index a3ebe169..dc6c1318 100644
--- a/CoreFoundation/Base.subproj/SwiftRuntime/CoreFoundation.h
+++ b/CoreFoundation/Base.subproj/SwiftRuntime/CoreFoundation.h
@@ -24,7 +24,7 @@
 
 #include <sys/types.h>
 #include <stdarg.h>
-#include <assert.h>
+//#include <assert.h>
 #include <ctype.h>
 #include <errno.h>
 #include <float.h>
diff --git a/TestFoundation/TestDecimal.swift b/TestFoundation/TestDecimal.swift
index 87e7ad5d..78899d16 100644
--- a/TestFoundation/TestDecimal.swift
+++ b/TestFoundation/TestDecimal.swift
@@ -421,9 +421,9 @@ class TestDecimal: XCTestCase {
                 var actual = Decimal(i)
                 var power = actual
                 XCTAssertEqual(.noError, NSDecimalPower(&actual, &power, j, .plain))
-                let expected = Decimal(pow(Double(i), Double(j)))
+                /*let expected = Decimal(pow(Double(i), Double(j)))
                 XCTAssertEqual(expected, actual, "\(actual) == \(i)^\(j)")
-                XCTAssertEqual(expected, pow(power, j))
+                XCTAssertEqual(expected, pow(power, j))*/
             }
         }
     }
diff --git a/TestFoundation/TestFileHandle.swift b/TestFoundation/TestFileHandle.swift
index 9ac998e5..04f5fe88 100644
--- a/TestFoundation/TestFileHandle.swift
+++ b/TestFoundation/TestFileHandle.swift
@@ -539,7 +539,7 @@ class TestFileHandle : XCTestCase {
             ("test_availableData", test_availableData),
             ("test_readToEndOfFileInBackgroundAndNotify", test_readToEndOfFileInBackgroundAndNotify),
             ("test_readToEndOfFileAndNotify", test_readToEndOfFileAndNotify),
-            ("test_readToEndOfFileAndNotify_readError", test_readToEndOfFileAndNotify_readError),
+            //("test_readToEndOfFileAndNotify_readError", test_readToEndOfFileAndNotify_readError),
             ("test_waitForDataInBackgroundAndNotify", test_waitForDataInBackgroundAndNotify),
             /* ⚠️ */ ("test_readWriteHandlers", testExpectedToFail(test_readWriteHandlers,
             /* ⚠️ */     "<rdar://problem/50860781> sporadically times out")),
diff --git a/TestFoundation/TestURLSession.swift b/TestFoundation/TestURLSession.swift
index 22de3d25..d21619ed 100644
--- a/TestFoundation/TestURLSession.swift
+++ b/TestFoundation/TestURLSession.swift
@@ -43,7 +43,7 @@ class TestURLSession : LoopbackServerTest {
             ("test_concurrentRequests", test_concurrentRequests),
             ("test_disableCookiesStorage", test_disableCookiesStorage),
             ("test_cookiesStorage", test_cookiesStorage),
-            ("test_cookieStorageForEphmeralConfiguration", test_cookieStorageForEphmeralConfiguration),
+            //("test_cookieStorageForEphmeralConfiguration", test_cookieStorageForEphmeralConfiguration),
             ("test_setCookies", test_setCookies),
             ("test_dontSetCookies", test_dontSetCookies),
             ("test_initURLSessionConfiguration", test_initURLSessionConfiguration),

If I don't comment out that system header, I get this strange compile error when building Foundation, from ClangImporter:

<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "Headers/CoreFoundation.h"
         ^
/data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:27:10: note: in file included from /data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:27:
#include <assert.h>
         ^
/data/data/com.termux/files/usr/include/assert.h:72:1: error: unknown type name '__BEGIN_DECLS'
__BEGIN_DECLS
^
<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "Headers/CoreFoundation.h"
         ^
/data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:27:10: note: in file included from /data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:27:
#include <assert.h>
         ^
/data/data/com.termux/files/usr/include/assert.h:78:1: error: expected identifier or '('
void __assert(const char* __file, int __line, const char* __msg) __noreturn;
^
<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "Headers/CoreFoundation.h"
         ^
/data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:27:10: note: in file included from /data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:27:
#include <assert.h>
         ^
/data/data/com.termux/files/usr/include/assert.h:84:91: error: expected function body after function declarator
void __assert2(const char* __file, int __line, const char* __function, const char* __msg) __noreturn;
                                                                                          ^
<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "Headers/CoreFoundation.h"
         ^
/data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:27:10: note: in file included from /data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:27:
#include <assert.h>
         ^
/data/data/com.termux/files/usr/include/assert.h:86:1: error: unknown type name '__END_DECLS'
__END_DECLS
^
<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "Headers/CoreFoundation.h"
         ^
/data/data/com.termux/files/home/src/swift/build/Ninja-Release/foundation-android-aarch64/CoreFoundation.framework/Headers/CoreFoundation.h:28:1: error: expected identifier or '('
#include <ctype.h>
^
/data/data/com.termux/files/home/src/swift/swift-corelibs-foundation/Foundation/Bridging.swift:13:8: error: could not build C module 'CoreFoundation'
import CoreFoundation
       ^
ninja: build stopped: subcommand failed.

This is a recent regression, wasn't getting it last month, haven't looked into it much.

The segfaulting test case I mentioned is the one in TestFileHandle, that hasn't worked since the beginning. The other two are regressions when I just updated to the 7/28 snapshot, wasn't hitting them with 7/1, with the former a linker error when looking for pow and the latter failing and stopping the test run.

I will try to run the executable tests again. In case I don't get that many failures, it would be nice to create another gist with those failing tests and their outputs.

I see about 1k executable_tests when searching the test suite, so only 27 failures is very good, particularly when half of those are likely because of Termux differences. I've posted the 7/28 failures now, with two tests from 7/1 now passing and tgmath now failing with 7/28 for the first time.

It might be that Android is setting and using RUNPATH, which is a very close sibling to RPATH, but not the same.

Yes, I mentioned earlier that that's what Android is using, but I was unaware that DT_RPATH vs DT_RUNPATH was the reason for the difference.

Sadly they use the same set of switches in the compilers, and depending on the linkers you might have one, the other, or both in your binaries. One difference is that, for security reasons, RUNPATH doesn't apply to finding indirect dependencies of the binary that has the value. I don’t know the test in particular, but something that should work across platforms should be setting explicit RUNPATH in all the binaries of the test.

What's strange is that readelf reports RUNPATH being used on Arch Linux x86_64 also, but that test passes there:

> readelf -d ../swift-linux-x86_64/test-linux-x86_64/multifile/multiconformanceimpls/Output/main.swift.tmp/{main.out,libA.so,libB.so,libC.so} | grep -E "File:|PATH"
File: ../swift-linux-x86_64/test-linux-x86_64/multifile/multiconformanceimpls/Output/main.swift.tmp/main.out
 0x000000000000001d (RUNPATH)            Library runpath: [/home/butta/swift/build/Ninja-Release/swift-linux-x86_64/lib/swift/linux:/home/butta/swift/build/Ninja-Release/swift-linux-x86_64/test-linux-x86_64/multifile/multiconformanceimpls/Output/main.swift.tmp]
File: ../swift-linux-x86_64/test-linux-x86_64/multifile/multiconformanceimpls/Output/main.swift.tmp/libA.so
 0x000000000000001d (RUNPATH)            Library runpath: [/home/butta/swift/build/Ninja-Release/swift-linux-x86_64/lib/swift/linux]
File: ../swift-linux-x86_64/test-linux-x86_64/multifile/multiconformanceimpls/Output/main.swift.tmp/libB.so
 0x000000000000001d (RUNPATH)            Library runpath: [/home/butta/swift/build/Ninja-Release/swift-linux-x86_64/lib/swift/linux]
File: ../swift-linux-x86_64/test-linux-x86_64/multifile/multiconformanceimpls/Output/main.swift.tmp/libC.so
 0x000000000000001d (RUNPATH)            Library runpath: [/home/butta/swift/build/Ninja-Release/swift-linux-x86_64/lib/swift/linux]

There are some parts of that patch that are surprising: why Termux doesn’t ship with glob.h, sys/sem.h, or sys/shm.h?

Note that my pasted patch above won't be part of this pull. They list the reasons for removing those headers here. I now see that glob.h has been installed by another package, but it wasn't a couple months ago when I modified that modulemap.

Is Termux still using an older NDK?

They update to the latest NDK pretty quickly, switching to r20 in the middle of June, termux/termux-packages@d0f4522.

the part about bits/fcntl.h should not be in the module map.

Sure, just a hack to get it to compile.

The part in test/Driver/multi-threaded.swift is something that can be fixed better, I think. I don’t know why TMPDIR is reset there, but it would be better to use a subdirectory inside %t as destination. That should work in any system just fine.

Yep.

The other parts seems very tied to Termux reporting itself as Linux, instead of a more helpful Android.

The SourceKit change, yes, just like the libdispatch patch linked above, but not most of the build-script changes, which are more because it expects to cross-compile for Android and not natively compile.

It would be nice to find cleaner alternatives to those changes (some of them are going to break Linux builds, some other break the Android builds).

Yes, I mentioned that the pasted patch from my previous comment will need to be cleaned up and submitted in later pulls.

[drodriguez]

@swift-ci please test Linux platform

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 9547693b7ff67b2cb7d0cb432bf8eabd8cab9b1a

[finagolfin]

I don't know what to make of the build failure, some problem with git checkout on the build nodes?

[drodriguez]

When things are rebased the bot first sends those build failures. The real build is green: https://ci.swift.org/job/swift-PR-Linux/15349/. I will merge on Monday, so I can be on top if something happens.

[finagolfin]

Thanks for the review and for merging, I'll look at getting my other patches in.