Skip to content
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

Test with android NDK r27c #40339

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Test with android NDK r27c #40339

wants to merge 11 commits into from

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Aug 7, 2024

New LTS Version

Problems:

See also https://github.com/orgs/android/projects/19

@Neumann-A
Copy link
Contributor

breaking compatibility with CMake projects which don't ask for CMake 3.5.

Why? Just set the required policy in the toolchain?

@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 7, 2024

Why?

Sounds aggressive. Ask Google.

Just set the required policy in the toolchain?

My plan is to do just that, in scripts/toolchains/android.cmake.

@bwrsandman
Copy link
Contributor

CMake projects which don't ask for CMake 3.5.

Looked into the versions. It looks like 3.3 (not 3.5) is the minimum necessary

@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 7, 2024

https://github.com/android/ndk/wiki/Changelog-r27:

Android V will allow OEMs to ship arm64-v8a devices with 16KiB page sizes. Devices that use this configuration will not be able to run existing apps that use native code. To be compatible with these devices, applications will need to rebuild all their native code to be 16KiB aligned, and rewrite any code which assumes a specific page size.

Also https://android.googlesource.com/platform/ndk/+/master/docs/BuildSystemMaintainers.md#Page-sizes

Q: Shall 16KiB be the default for NDK r27 arm64-android?
Q: Shall there be triplet variable?

OTOH it isn't more urgent than raising the API level.

@Neumann-A
Copy link
Contributor

Why?

Sounds aggressive. Ask Google.

Reads like: Words, has no sound.... ¯\(ツ)/¯ Don't mind me asking stupid questions.
But you already expressed my opinion upstream before I even had one :) .

Q: Shall there be a special arm64-v8a triplet?

@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 7, 2024

Initial success:

Success Fail Cascade
arm-neon-android 1120 252 230
arm64-android 1139 255 (due to IN_LIST: 251) 257
x64-android 1135 263 278

@MonicaLiu0311 MonicaLiu0311 added the requires:testing Needs tests added before merging label Aug 8, 2024
@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 8, 2024

I have difficulties to get CMP0057 from the toolchain scope (android.cmake, called via CMakeDetermineSystem.cmake) to the failing scope (flags.cmake, called via CMakeSystem.cmake). Even when I put it directly into vcpkg.cmake.

Testing the proposed fix to the NDK instead.

@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 8, 2024

This works:

diff --git a/scripts/toolchains/android.cmake b/scripts/toolchains/android.cmake
index d675d06a73..4564f29e7e 100644
--- a/scripts/toolchains/android.cmake
+++ b/scripts/toolchains/android.cmake
@@ -31,6 +31,11 @@ endif()
 
 include("${ANDROID_NDK_HOME}/build/cmake/android.toolchain.cmake")
 
+if(ANDROID_NDK_MAJOR STREQUAL "27")
+    # https://github.com/android/ndk/issues/2032#issuecomment-2274923977
+    string(APPEND CMAKE_SYSTEM_CUSTOM_CODE "cmake_policy(SET CMP0057 NEW)\n")
+endif()
+
 if(NOT _VCPKG_ANDROID_TOOLCHAIN)
     set(_VCPKG_ANDROID_TOOLCHAIN 1)
 

But it might be a somewhat tight binding to both CMake and NDK script code.

@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 8, 2024

Success with patch to NDK's flags.cmake:

Success Fail Cascade
arm-neon-android n/a* 6 20
arm64-android n/a* 8 (due to IN_LIST: 0 28
x64-android n/a* 9 77

*) successfuild builds from the first run were cached and not tested again

@dg0yt dg0yt changed the title Test with android NDK r27 Test with android NDK r27b Sep 24, 2024
@dg0yt
Copy link
Contributor Author

dg0yt commented Sep 24, 2024

Initial success with r27b:

Success Fail Cascade
arm-neon-android 1615 7 3
arm64-android 1658 9 7
x64-android 1633 9 59

@dg0yt dg0yt mentioned this pull request Oct 2, 2024
1 task
@dg0yt dg0yt mentioned this pull request Oct 12, 2024
@dg0yt dg0yt changed the title Test with android NDK r27b Test with android NDK r27c Oct 17, 2024
@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 18, 2024

Initial success with r27c:

Success Fail Cascade
arm-neon-android 1629 5 2
arm64-android 1673 7 6
x64-android 1646 7 61

x64-android errors:

  • cppcoro: clang++: error: unknown argument: '-fcoroutines-ts'
  • fbbgemmerror: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
  • freeglut: error: 'ALooper_pollAll' is unavailable: obsoleted in Android 1
  • libgnutls: error: call to undeclared function 'mktime_z' etc.
  • raylib: error: 'ALooper_pollAll' is unavailable: obsoleted in Android 1
  • vulkan-tools: error: 'ALooper_pollAll' is unavailable: obsoleted in Android 1
  • yajl: at reformatter/CMakeLists.txt:38 (GET_TARGET_PROPERTY): The LOCATION property may not be read from target "json_reformat".

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 19, 2024

Okay, waiting for #41647 arriving as an image.

@MonicaLiu0311 MonicaLiu0311 added the category:infrastructure Pertaining to the CI/Testing infrastrucutre label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:infrastructure Pertaining to the CI/Testing infrastrucutre requires:testing Needs tests added before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants