diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.11k stars 795 forks source link

Android major upgrades #7524

Closed tsunamistate closed 2 weeks ago

tsunamistate commented 2 weeks ago

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 commented 2 weeks ago

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
AJenbo commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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

tsunamistate commented 2 weeks ago

@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 https://github.com/diasurgical/devilutionX/blob/d61ab52e515cf487feefb3084b1427afb7babfbd/CMake/functions/FetchContent_MakeAvailableExcludeFromAll.cmake#L7

But it can be fixed in another PR

glebm commented 2 weeks ago

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