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

Android major upgrades #7524

Merged
merged 3 commits into from
Nov 11, 2024
Merged

Conversation

tsunamistate
Copy link
Collaborator

@tsunamistate tsunamistate commented Nov 10, 2024

  • Use SDL2 2.30.9
  • Use Android Gradle plugin 8.7.2
  • Use latest NDK 28
  • Use latest CMake 3.31.0 for configuration

WARNING! Testing on real devices needed before merging, due to NDK upgrades. I have verified everything works on Pixel 8 Pro via emulator, but nothing beats real device testing

@tsunamistate
Copy link
Collaborator Author

Android build failed because licence for NDK was not accepted, I have no idea how to do it in CI

FAILURE: Build failed with an exception.

Checking the license for package NDK (Side by side) 28.0.12433566 in /usr/local/lib/android/sdk/licenses
Warning: License for package NDK (Side by side) 28.0.12433566 not accepted.
* What went wrong:
A problem occurred configuring project ':app'.
> com.android.builder.sdk.LicenceNotAcceptedException: Failed to install the following Android SDK packages as some licences have not been accepted.
     ndk;28.0.12433566 NDK (Side by side) 28.0.12433566
  To build this project, accept the SDK license agreements and install the missing components using the Android Studio SDK Manager.
  All licenses can be accepted using the sdkmanager command line tool:
  sdkmanager.bat --licenses
  Or, to transfer the license agreements from one workstation to another, see https://developer.android.com/studio/intro/update.html#download-with-gradle

* Use SDL2 2.30.9
* Use Android Gradle plugin 8.7.2
* Use latest NDK 28
* Use latest CMake 3.31.0 for configuration

WARINING! Testing on real devices needed before merging, due to NDK upgrades
I hvae verified everything works on Pixel 8 Pro via emulator, but nothing beats real device testing
@AJenbo
Copy link
Member

AJenbo commented Nov 10, 2024

Added a step to accept the license and install CMake 3.31, but we should probably look to see if we can define CMake in the gradle file instead so that it's also setup correctly for anyone trying to work on the project.

@AJenbo
Copy link
Member

AJenbo commented Nov 10, 2024

This is being spammed all over the build, we should probably see if we can solve that:

C/C++: CMake Deprecation Warning at /usr/local/lib/android/sdk/ndk/28.0.12433566/build/cmake/android-legacy.toolchain.cmake:35 (cmake_minimum_required):
C/C++:   Compatibility with CMake < 3.10 will be removed from a future version of
C/C++:   CMake.
C/C++:   Update the VERSION argument <min> value or use a ...<max> suffix to tell
C/C++:   CMake that the project does not need compatibility with older versions.
C/C++: Call Stack (most recent call first):
C/C++:   /usr/local/lib/android/sdk/ndk/28.0.12433566/build/cmake/android.toolchain.cmake:55 (include)
C/C++:   /home/runner/work/devilutionX/devilutionX/android-project/app/.cxx/Debug/635dt4w3/x86_64/CMakeFiles/3.31.0/CMakeSystem.cmake:6 (include)
C/C++:   /home/runner/work/devilutionX/devilutionX/android-project/app/.cxx/Debug/635dt4w3/x86_64/CMakeFiles/CMakeScratch/TryCompile-Rv1Wv6/CMakeLists.txt:4 (project)

@glebm
Copy link
Collaborator

glebm commented Nov 10, 2024

Could be because some 3rdparty specify a lower minimum CMake version
The warning is benign for now

@tsunamistate
Copy link
Collaborator Author

@glebm I would say, that one of spam warnings is DevX fault

CMake Warning (dev) at /usr/local/lib/android/sdk/cmake/3.31.0/share/cmake-3.31/Modules/FetchContent.cmake:1953 (message):
C/C++:   Calling FetchContent_Populate(bzip2) is deprecated, call
C/C++:   FetchContent_MakeAvailable(bzip2) instead.  Policy CMP0169 can be set to
C/C++:   OLD to allow FetchContent_Populate(bzip2) to be called directly for now,
C/C++:   but the ability to call it with declared details will be removed completely
C/C++:   in a future version.
C/C++: Call Stack (most recent call first):
C/C++:   CMake/functions/FetchContent_MakeAvailableExcludeFromAll.cmake:7 (FetchContent_Populate)
C/C++:   3rdParty/bzip2/CMakeLists.txt:8 (FetchContent_MakeAvailableExcludeFromAll)

This is caused by our macro

But it can be fixed in another PR

@glebm
Copy link
Collaborator

glebm commented Nov 11, 2024

Yeah, I can look into fixing this later, it should be quite easy.

@AJenbo AJenbo merged commit 17ce17e into diasurgical:master Nov 11, 2024
23 checks passed
@tsunamistate tsunamistate deleted the android-major-upgrades branch November 11, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants