-
Notifications
You must be signed in to change notification settings - Fork 257
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
[BUG] NDK 27 no longer supports passing in CMAKE_SYSTEM_PROCESSOR
to CMake to set the arch
#2049
Comments
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.
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...
Yes, it's generated from meta/abis.json as part of the NDK's build process. |
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. |
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. |
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.
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.
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. |
I went to write a test for the fix, and I noticed that the premise of this report (using 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. |
Broke on Windows. I think it's just a test issue, but reverting until I can debug. |
Github runner seems to bump a vendored version of NDK to version 27 (which seems to have an issue with CMAKE_SYSTEM_PROCESSOR android/ndk#2049 and `cmake-rs` crate does define that https://github.com/rust-lang/cmake-rs/blob/c4a60dd154dd90e469dffc41a1faa717704f90b3/src/lib.rs#L458) The problem is fixed by explicitly installing an NDK version which we're currently use in build.gradle file ("25.2.9519653" or r25c)
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). |
Maybe the following information can be interesting: Is there any idea when NDK 27b will be published? |
As soon as we're able. Infrastructure's broken. |
(in the mean time, keep using r26d?) |
Yes, we keep using r26d on our stable branch but we also started to use r27 in the dev branch.
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. |
You could also delete your hack and switch to using |
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 When
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). |
`NDK_PROC_*_ABI` and `NDK_ARCH_*_ABI` were removed in the Android NDK 27 but will be reintroduced in the Android NDK 27b: * android/ndk#2049 * https://android-review.googlesource.com/c/platform/ndk/+/3196345 Both are only used when `CMAKE_ANDROID_ARCH_ABI` is NOT given. But currently the existence is also checked when `CMAKE_ANDROID_ARCH_ABI` is given. So we move the checks to the position they are required.
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. |
Description
This simple command always worked in the past for most CMake repos:
For example, with NDK 26d:
I looked into that error and the problem is that CMake expects
NDK_PROC_aarch64_ABI
and so on to be set, but the newabis.cmake
no longer does: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 settingCMAKE_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
The text was updated successfully, but these errors were encountered: