android / ndk

The Android Native Development Kit
1.97k stars 255 forks source link

[BUG] NDK 27 no longer supports passing in `CMAKE_SYSTEM_PROCESSOR` to CMake to set the arch #2049

Closed finagolfin closed 1 month ago

finagolfin commented 2 months ago

Description

This simple command always worked in the past for most CMake repos:

> cmake -GNinja /home/finagolfin/swift-corelibs-libdispatch/ -DCMAKE_SYSTEM_NAME=Android -DCMAKE_SYSTEM_VERSION=21 -DCMAKE_SYSTEM_PROCESSOR=aarch64 -DCMAKE_ANDROID_NDK=/home/finagolfin/android-ndk-r27/
CMake Error at /usr/share/cmake/Modules/Platform/Android-Determine.cmake:378 (message):
  Android: Unknown processor CMAKE_SYSTEM_PROCESSOR='aarch64'.
Call Stack (most recent call first):
  /usr/share/cmake/Modules/CMakeDetermineSystem.cmake:212 (include)
  CMakeLists.txt:12 (project)

CMake Error: CMake was unable to find a build program corresponding to "Ninja".  CMAKE_MAKE_PROGRAM is not set.  You probably need to select a different build tool.
CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
-- Configuring incomplete, errors occurred!

For example, with NDK 26d:

> cmake -GNinja /home/finagolfin/swift-corelibs-libdispatch/ -DCMAKE_SYSTEM_NAME=Android -DCMAKE_SYSTEM_VERSION=21 -DCMAKE_SYSTEM_PROCESSOR=aarch64 -DCMAKE_ANDROID_NDK=/home/finagolfin/android-ndk-r26d/
-- Android: Targeting API '21' with architecture 'arm64', ABI 'arm64-v8a', and processor 'aarch64'
-- Android: Selected unified Clang toolchain
-- The C compiler identification is Clang 17.0.2
-- The CXX compiler identification is Clang 17.0.2
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /home/finagolfin/android-ndk-r26d//toolchains/llvm/prebuilt/linux-x86_64/bin/clang - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /home/finagolfin/android-ndk-r26d//toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Threads: TRUE
-- Looking for __GNU_LIBRARY__
-- Looking for __GNU_LIBRARY__ - not found
-- Performing Test __BUILTIN_TRAP
-- Performing Test __BUILTIN_TRAP - Success
-- Looking for _pthread_workqueue_init
-- Looking for _pthread_workqueue_init - not found
-- Looking for getprogname
-- Looking for getprogname - found
-- Looking for mach_absolute_time
-- Looking for mach_absolute_time - not found
-- Looking for mach_approximate_time
-- Looking for mach_approximate_time - not found
-- Looking for mach_port_construct
-- Looking for mach_port_construct - not found
-- Looking for malloc_create_zone
-- Looking for malloc_create_zone - not found
-- Looking for posix_fadvise
-- Looking for posix_fadvise - found
-- Looking for posix_spawnp
-- Looking for posix_spawnp - not found
-- Looking for pthread_key_init_np
-- Looking for pthread_key_init_np - not found
-- Looking for pthread_attr_setcpupercent_np
-- Looking for pthread_attr_setcpupercent_np - not found
-- Looking for pthread_yield_np
-- Looking for pthread_yield_np - not found
-- Looking for pthread_main_np
-- Looking for pthread_main_np - not found
-- Looking for pthread_workqueue_setdispatch_np
-- Looking for pthread_workqueue_setdispatch_np - not found
-- Looking for strlcpy
-- Looking for strlcpy - found
-- Looking for sysconf
-- Looking for sysconf - found
-- Looking for arc4random
-- Looking for arc4random - found
-- Looking for include file TargetConditionals.h
-- Looking for include file TargetConditionals.h - not found
-- Looking for include file dlfcn.h
-- Looking for include file dlfcn.h - found
-- Looking for include file fcntl.h
-- Looking for include file fcntl.h - found
-- Looking for include file inttypes.h
-- Looking for include file inttypes.h - found
-- Looking for include file libkern/OSAtomic.h
-- Looking for include file libkern/OSAtomic.h - not found
-- Looking for include file libkern/OSCrossEndian.h
-- Looking for include file libkern/OSCrossEndian.h - not found
-- Looking for include file libproc_internal.h
-- Looking for include file libproc_internal.h - not found
-- Looking for include file mach/mach.h
-- Looking for include file mach/mach.h - not found
-- Looking for include file malloc/malloc.h
-- Looking for include file malloc/malloc.h - not found
-- Looking for include file memory.h
-- Looking for include file memory.h - found
-- Looking for include file pthread/qos.h
-- Looking for include file pthread/qos.h - not found
-- Looking for include file pthread/workqueue_private.h
-- Looking for include file pthread/workqueue_private.h - not found
-- Looking for include file pthread_machdep.h
-- Looking for include file pthread_machdep.h - not found
-- Looking for include file pthread_np.h
-- Looking for include file pthread_np.h - not found
-- Looking for include file pthread_workqueue.h
-- Looking for include file pthread_workqueue.h - not found
-- Looking for include file stdint.h
-- Looking for include file stdint.h - found
-- Looking for include file stdlib.h
-- Looking for include file stdlib.h - found
-- Looking for include file string.h
-- Looking for include file string.h - found
-- Looking for include file strings.h
-- Looking for include file strings.h - found
-- Looking for include file sys/guarded.h
-- Looking for include file sys/guarded.h - not found
-- Looking for include file sys/stat.h
-- Looking for include file sys/stat.h - found
-- Looking for include file sys/types.h
-- Looking for include file sys/types.h - found
-- Looking for include file objc/objc-internal.h
-- Looking for include file objc/objc-internal.h - not found
-- Looking for sem_init in pthread
-- Looking for sem_init in pthread - not found
-- Looking for CLOCK_UPTIME
-- Looking for CLOCK_UPTIME - not found
-- Looking for CLOCK_UPTIME_FAST
-- Looking for CLOCK_UPTIME_FAST - not found
-- Looking for CLOCK_MONOTONIC
-- Looking for CLOCK_MONOTONIC - found
-- Looking for CLOCK_REALTIME
-- Looking for CLOCK_REALTIME - found
-- Looking for CLOCK_MONOTONIC_COARSE
-- Looking for CLOCK_MONOTONIC_COARSE - found
-- Looking for FD_COPY
-- Looking for FD_COPY - not found
-- Looking for NOTE_LOWAT
-- Looking for NOTE_LOWAT - not found
-- Looking for NOTE_NONE
-- Looking for NOTE_NONE - not found
-- Looking for NOTE_REAP
-- Looking for NOTE_REAP - not found
-- Looking for NOTE_REVOKE
-- Looking for NOTE_REVOKE - not found
-- Looking for NOTE_SIGNAL
-- Looking for NOTE_SIGNAL - not found
-- Looking for POSIX_SPAWN_START_SUSPENDED
-- Looking for POSIX_SPAWN_START_SUSPENDED - not found
-- Looking for SIGEMT
-- Looking for SIGEMT - not found
-- Looking for VQ_DESIRED_DISK
-- Looking for VQ_DESIRED_DISK - not found
-- Looking for VQ_NEARLOWDISK
-- Looking for VQ_NEARLOWDISK - not found
-- Looking for VQ_QUOTA
-- Looking for VQ_QUOTA - not found
-- Looking for VQ_UPDATE
-- Looking for VQ_UPDATE - not found
-- Looking for VQ_VERYLOWDISK
-- Looking for VQ_VERYLOWDISK - not found
-- Looking for VQ_FREE_SPACE_CHANGE
-- Looking for VQ_FREE_SPACE_CHANGE - not found
-- Looking for program_invocation_name
-- Looking for program_invocation_name - not found
-- Looking for __printflike
-- Looking for __printflike - not found
-- Performing Test C_SUPPORTS_OMIT_LEAF_FRAME_POINTER
-- Performing Test C_SUPPORTS_OMIT_LEAF_FRAME_POINTER - Success
-- Configuring done (3.1s)
-- Generating done (0.0s)
-- Build files have been written to: /home/finagolfin/swift-corelibs-libdispatch/build

I looked into that error and the problem is that CMake expects NDK_PROC_aarch64_ABI and so on to be set, but the new abis.cmake no longer does:

> diff android-ndk-r26d/build/cmake/abis.cmake android-ndk-r27/build/cmake/abis.cmake                                                            4,5c4,5
< set(NDK_KNOWN_DEVICE_ABI64S "arm64-v8a;x86_64")
< set(NDK_KNOWN_DEVICE_ABIS "arm64-v8a;armeabi-v7a;x86;x86_64")
---
> set(NDK_KNOWN_DEVICE_ABI64S "arm64-v8a;riscv64;x86_64")
> set(NDK_KNOWN_DEVICE_ABIS "arm64-v8a;armeabi-v7a;riscv64;x86;x86_64")
10,11c10
< set(NDK_PROC_armv7-a_ABI "armeabi-v7a")
< set(NDK_ARCH_arm_ABI "armeabi-v7a")
---
> set(NDK_ABI_armeabi-v7a_MIN_OS_VERSION "21")
16,17c15,20
< set(NDK_PROC_aarch64_ABI "arm64-v8a")
< set(NDK_ARCH_arm64_ABI "arm64-v8a")
---
> set(NDK_ABI_arm64-v8a_MIN_OS_VERSION "21")
> set(NDK_ABI_riscv64_PROC "riscv64")
> set(NDK_ABI_riscv64_ARCH "riscv64")
> set(NDK_ABI_riscv64_TRIPLE "riscv64-linux-android")
> set(NDK_ABI_riscv64_LLVM_TRIPLE "riscv64-none-linux-android")
> set(NDK_ABI_riscv64_MIN_OS_VERSION "35")
22,23c25
< set(NDK_PROC_i686_ABI "x86")
< set(NDK_ARCH_x86_ABI "x86")
---
> set(NDK_ABI_x86_MIN_OS_VERSION "21")
28,29c30,31
< set(NDK_PROC_x86_64_ABI "x86_64")
< set(NDK_ARCH_x86_64_ABI "x86_64")
\ No newline at end of file
---
> set(NDK_ABI_x86_64_MIN_OS_VERSION "21")
\ No newline at end of file

I looked in the source for this file, but it's not there and saw no mention of it in the scripts either. Is it generated externally somewhere?

Was dropping these a mistake or a choice? If I add back NDK_PROC_aarch64_ABI, the config works again.

Many build systems likely simply set that CMAKE_SYSTEM_NAME/CMAKE_SYSTEM_VERSION/CMAKE_SYSTEM_PROCESSOR triad to compile for all platforms, just setting CMAKE_ANDROID_NDK additionally for Android. It would be good to continue supporting that basic CMake config.

Affected versions

r27

Canary version

No response

Host OS

Linux

Host OS version

Fedora 40 x86_64

Affected ABIs

armeabi-v7a, arm64-v8a, x86, x86_64

Build system

CMake

Other build system

No response

minSdkVersion

21

Device API level

No response

DanAlbert commented 2 months ago

Many build systems likely simply set that CMAKE_SYSTEM_NAME/CMAKE_SYSTEM_VERSION/CMAKE_SYSTEM_PROCESSOR triad to compile for all platforms, just setting CMAKE_ANDROID_NDK additionally for Android. It would be good to continue supporting that basic CMake config.

I really can't recommend doing this. There's a reason we have a toolchain file. We've never gotten CMake to behave the way we want it to without one. More time invested would certainly be able to work that out, but we invested quite a lot already and have nothing to show for it. It isn't something we can afford to spend time on for the foreseeable future.

I looked into that error and the problem is that CMake expects NDK_PROC_aarch64_ABI and so on to be set, but the new abis.cmake no longer does:

Ugh. It looked unused so I removed it while doing some related work (I think we didn't know what the correct value was for rv64, and since it appeared unused it was simpler to just remove than it was to figure that out). Didn't realize that was for CMake interop. I'll put that back and add a comment...

Is [abis.json] generated externally somewhere?

Yes, it's generated from meta/abis.json as part of the NDK's build process.

DanAlbert commented 2 months ago

https://android-review.git.corp.google.com/c/platform/ndk/+/3196345 should fix it. Like I said above though, I really don't recommend using CMake this way for the NDK. We don't test that workflow, and that work was never finished. It was supposed to reduce maintenance costs for us and CMake, and make things easier for users, but the result was the opposite for everyone, so for now we have no plans to pick it back up.

finagolfin commented 2 months ago

I really don't recommend using CMake this way for the NDK. We don't test that workflow, and that work was never finished.

Understood, but realistically there are a lot of CMake projects that use that original set of config options, that predates toolchain files, who are never going to migrate over to toolchain files, until forced to. That's why I asked if this was a choice, to force people to stop using that original basic config.

In my case, I hit this when testing my Android SDK for Swift with the new NDK, but the Swift project is slowly moving to toolchain files anyway, swiftlang/swift#38124. I suspect most projects won't, until forced to.

DanAlbert commented 2 months ago

Understood, but realistically there are a lot of CMake projects that use that original set of config options, that predates toolchain files

FWIW, for Android, our CMake toolchain file actually does predate the built-in CMake support, and the 3p toolchain file ours is based on predates it by years. If CMake had built-in support for Android before we went that route, we definitely would not have bothered.

That's why I asked if this was a choice, to force people to stop using that original basic config.

Well, it's somewhere on the edge of supported and unsupported. Like I said, we don't test this configuration, and we know it doesn't work as well as the toolchain file (where "well" means that it uses all our recommended configuration options; if what you actually want is for your build config to most closely match the defaults for other CMake platforms, the built-in behavior works better for that). We'll fix issues as they're reported, but until we've got people to spare to make this work properly, it's going to have a lower standard of support (from us) than the toolchain file workflow.

I suspect most projects won't [move to toolchain files], until forced to.

IME, most projects are already on toolchain files for Android. I don't know the last time I ran into a project that wasn't using them. They're certainly out there, but Android's docs have always recommended our CMake toolchain file, and until CMake 3.7 the toolchain file (ours or the 3p one) was the only option.

DanAlbert commented 2 months ago

I went to write a test for the fix, and I noticed that the premise of this report (using CMAKE_SYSTEM_PROCESSOR) isn't actually the interface that CMake documents for Android. https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html#cross-compiling-for-android-with-the-ndk doesn't seem to mention CMAKE_SYSTEM_PROCESSOR and says to use CMAKE_ANDROID_ARCH_ABI. I'm not sure CMAKE_SYSTEM_PROCESSOR is future-proof, since one processor can support many ABIs. A much older version of the CMake toolchain file for Android in fact had several ABIs for CMAKE_SYSTEM_PROCESSOR=armeabi-v7a. That's gone now, but had 16k page sizes been a new ABI (in a lot of ways, that is a new ABI), CMAKE_SYSTEM_PROCESSOR also wouldn't have been enough to specify the target. It's a bit too early to say if riscv64 will reintroduce that problem, but there are a lot of hardware variations for that architecture.

I'll still put those values back because it's trivial, but the test I'm adding is only going to cover the documented interface.

Even if you don't want to migrate to the toolchain file, you probably should migrate to the documented CMake interface. OTOH, CMake's pretty good (very good, in fact) about keeping backwards compat even for accidental stuff, so maybe not something that'd ever be a problem.

DanAlbert commented 1 month ago

Broke on Windows. I think it's just a test issue, but reverting until I can debug.

DanAlbert commented 1 month ago

Round 2 stuck: https://r.android.com/3213881. r28 (currently trunk) is fixed. Will cherry-pick to r27b (I'm waiting for some compiler fixes before that goes to QA, but hopefully shouldn't be long).

SmallLars commented 1 month ago

Maybe the following information can be interesting: https://bugreports.qt.io/browse/QTBUG-127468 https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9766

Is there any idea when NDK 27b will be published?

DanAlbert commented 1 month ago

Is there any idea when NDK 27b will be published?

As soon as we're able. Infrastructure's broken.

DanAlbert commented 1 month ago

(in the mean time, keep using r26d?)

SmallLars commented 1 month ago

Yes, we keep using r26d on our stable branch but we also started to use r27 in the dev branch. Currently we use a "hack" in our toolchain file until CMake 3.30.3 is published to work with r27:

if(ANDROID_NDK_REVISION VERSION_EQUAL "27.0.12077973" AND CMAKE_VERSION VERSION_LESS "3.30.3")
    set(NDK_PROC_aarch64_ABI "DUMMY")
    set(NDK_PROC_armv7-a_ABI "DUMMY")
    set(NDK_PROC_i686_ABI    "DUMMY")
    set(NDK_PROC_x86_64_ABI  "DUMMY")
endif()

This is possible for us because we set CMAKE_ANDROID_ARCH_ABI. In this case CMake currently requires the values but do not use them.

DanAlbert commented 1 month ago

You could also delete your hack and switch to using CMAKE_ANDROID_ARCH_ABI like the CMake docs say? Or using the toolchain file?

SmallLars commented 1 month ago

As far as I know a toolchain file is a way to provide information for cross-compiling early in the CMake run. But it looks like it is also possible to provide all information in command line parameters like @finagolfin described. So I think this is not relevant here.

In general it is possible to set CMAKE_ANDROID_ARCH_ABI so NDK_PROC_*_ABI and NDK_ARCH_*_ABI are not used. But with CMake <= 3.30.2 they are expected anyways. That is, what https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9766 would change -> Only expect when use.

When CMAKE_ANDROID_ARCH_ABI is not set CMake try to determine it:

It looks like there was also a time when the NDK did not provide this information before because CMake has also a function to set it by its own: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9766/diffs#60c1329725f8668d3f59d64323770918b100780a_321_321 (Line 321++ is not directly visible at loading).

DanAlbert commented 1 month ago

As far as I know a toolchain file is a way to provide information for cross-compiling early in the CMake run. But it looks like it is also possible to provide all information in command line parameters like @finagolfin described. So I think this is not relevant here.

See https://developer.android.com/ndk/guides/cmake (specifically the "caution" section under "The CMake toolchain file"). It's very relevant. We only provide best-effort support for the workflow you're using.