-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[android] Add support for natively building on Android #23208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,7 +140,8 @@ function(_add_variant_c_compile_link_flags) | |
# lld can handle targeting the android build. However, if lld is not | ||
# enabled, then fallback to the linker included in the android NDK. | ||
if(NOT SWIFT_ENABLE_LLD_LINKER) | ||
list(APPEND result "-B" "${SWIFT_SDK_ANDROID_ARCH_${CFLAGS_ARCH}_NDK_PREBUILT_PATH}/${SWIFT_SDK_ANDROID_ARCH_${CFLAGS_ARCH}_NDK_TRIPLE}/bin") | ||
swift_android_tools_path(${CFLAGS_ARCH} tools_path) | ||
list(APPEND result "-B" "${tools_path}") | ||
endif() | ||
endif() | ||
|
||
|
@@ -482,22 +483,9 @@ function(_add_variant_link_flags) | |
# We need to add the math library, which is linked implicitly by libc++ | ||
list(APPEND result "-lm") | ||
|
||
if("${LFLAGS_ARCH}" MATCHES armv7) | ||
set(android_libcxx_path "${SWIFT_ANDROID_NDK_PATH}/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a") | ||
elseif("${LFLAGS_ARCH}" MATCHES aarch64) | ||
set(android_libcxx_path "${SWIFT_ANDROID_NDK_PATH}/sources/cxx-stl/llvm-libc++/libs/arm64-v8a") | ||
elseif("${LFLAGS_ARCH}" MATCHES i686) | ||
set(android_libcxx_path "${SWIFT_ANDROID_NDK_PATH}/sources/cxx-stl/llvm-libc++/libs/x86") | ||
elseif("${LFLAGS_ARCH}" MATCHES x86_64) | ||
set(android_libcxx_path "${SWIFT_ANDROID_NDK_PATH}/sources/cxx-stl/llvm-libc++/libs/x86_64") | ||
else() | ||
message(SEND_ERROR "unknown architecture (${LFLAGS_ARCH}) for android") | ||
endif() | ||
|
||
# link against the custom C++ library | ||
list(APPEND link_libraries | ||
${android_libcxx_path}/libc++abi.a | ||
${android_libcxx_path}/libc++_shared.so) | ||
swift_android_cxx_libraries_for_arch(${LFLAGS_ARCH} cxx_link_libraries) | ||
list(APPEND link_libraries ${cxx_link_libraries}) | ||
|
||
# link against the ICU libraries | ||
list(APPEND link_libraries | ||
|
@@ -1104,7 +1092,7 @@ function(_add_swift_library_single target name) | |
set_target_properties("${target}" | ||
PROPERTIES | ||
INSTALL_NAME_DIR "${install_name_dir}") | ||
elseif("${SWIFTLIB_SINGLE_SDK}" STREQUAL "LINUX" AND NOT "${SWIFTLIB_SINGLE_SDK}" STREQUAL "ANDROID") | ||
elseif("${SWIFTLIB_SINGLE_SDK}" STREQUAL "LINUX") | ||
finagolfin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
set_target_properties("${target}" | ||
PROPERTIES | ||
INSTALL_RPATH "$ORIGIN:/usr/lib/swift/linux") | ||
|
@@ -1116,6 +1104,14 @@ function(_add_swift_library_single target name) | |
# CMake generates incorrect rule `$SONAME_FLAG $INSTALLNAME_DIR$SONAME` for Android build on macOS cross-compile host. | ||
# Proper linker flags constructed manually. See below variable `swiftlib_link_flags_all`. | ||
set_target_properties("${target}" PROPERTIES NO_SONAME TRUE) | ||
# Only set the install RPATH if cross-compiling the host tools, in which | ||
# case both the NDK and Sysroot paths must be set. | ||
if(NOT "${SWIFT_ANDROID_NDK_PATH}" STREQUAL "" AND | ||
NOT "${SWIFT_ANDROID_NATIVE_SYSROOT}" STREQUAL "") | ||
set_target_properties("${target}" | ||
PROPERTIES | ||
INSTALL_RPATH "$ORIGIN") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
endif() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two things:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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: 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
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. |
||
endif() | ||
|
||
set_target_properties("${target}" PROPERTIES BUILD_WITH_INSTALL_RPATH YES) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,9 +42,14 @@ function(add_swift_unittest test_dirname) | |
if("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin") | ||
set_property(TARGET "${test_dirname}" APPEND_STRING PROPERTY | ||
LINK_FLAGS " -Xlinker -rpath -Xlinker ${SWIFT_LIBRARY_OUTPUT_INTDIR}/swift/macosx") | ||
elseif("${SWIFT_HOST_VARIANT}" STREQUAL "android") | ||
swift_android_lib_for_arch(${SWIFT_HOST_VARIANT_ARCH} android_system_libs) | ||
set_property(TARGET "${test_dirname}" APPEND PROPERTY LINK_DIRECTORIES | ||
"${android_system_libs}") | ||
set_property(TARGET "${test_dirname}" APPEND PROPERTY LINK_LIBRARIES "log") | ||
elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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*. 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @compnerd: I looked a little to setting This is why I’m asking to try to use the NDK itself inside Termux. It will make the changes minimal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
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.
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. |
||
set_property(TARGET "${test_dirname}" APPEND_STRING PROPERTY | ||
LINK_FLAGS " -latomic") | ||
set_property(TARGET "${test_dirname}" APPEND PROPERTY LINK_LIBRARIES | ||
"atomic") | ||
endif() | ||
|
||
find_program(LDLLD_PATH "ld.lld") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -206,7 +206,7 @@ char ** _swift_stdlib_getUnsafeArgvArgc(int *outArgLen) { | |
|
||
return outBuf; | ||
} | ||
#else // __ANDROID__; Add your favorite arch's command line arg grabber here. | ||
#else // Add your favorite OS's command line arg grabber here. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
SWIFT_RUNTIME_STDLIB_API | ||
char ** _swift_stdlib_getUnsafeArgvArgc(int *outArgLen) { | ||
if (_swift_stdlib_ProcessOverrideUnsafeArgv) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.