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 CI with JDK and SDK #42080

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

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Nov 9, 2024

Reprise from #35845.

@dg0yt dg0yt marked this pull request as draft November 9, 2024 21:46
@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 10, 2024

@m-kuhn Elapsed time to handle qtbase:x64-android: 4.2 min

@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 10, 2024

allegro5: Uses (downloads) gradle 5 which "supports running Gradle builds with JDK 11". Fails with:

Welcome to Gradle 5.0!

Here are the highlights of this release:
 - Kotlin DSL 1.0
 - Task timeouts
 - Dependency alignment aka BOM support
 - Interactive `gradle init`

For more details see https://docs.gradle.org/5.0/release-notes.html

Starting a Gradle Daemon (subsequent builds will be faster)
java.lang.NoClassDefFoundError: Could not initialize class org.codehaus.groovy.vmplugin.v7.Java7

Java 17 support comes with gradle 7.3.

@dg0yt dg0yt force-pushed the qtbase-android branch 3 times, most recently from 7ba693a to c1181b6 Compare November 10, 2024 11:26
@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 10, 2024

qtbase 6.7 silently overrides toolchain API level 21 with actual API level 23.
Some downstream errors are related to API level 21 again, e.g. kdsoap.

qtbase also selects ANDROID_STL=c++_shared despite VCPKG_CRT_LINKAGE=static.
Eventually leading to confusion downstream, and probably also with dependencies.

@JonLiu1993 JonLiu1993 added the category:infrastructure Pertaining to the CI/Testing infrastrucutre label Nov 11, 2024
@@ -10,8 +10,8 @@ vcpkg_from_github(
minimp3-fix.patch
)

if(VCPKG_TARGET_IS_ANDROID AND NOT ENV{ANDROID_HOME})
message(FATAL_ERROR "${PORT} requires environment variable ANDROID_HOME to be set." )
if(VCPKG_TARGET_IS_ANDROID AND "$ENV{ANDROID_HOME}" STREQUAL "")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the difference?

Copy link
Contributor Author

@dg0yt dg0yt Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first one does't work, the second does. (I was surprised, too.)

Comment on lines 345 to 349
if(VCPKG_TARGET_IS_ANDROID)
# Qt requires libc++_shared, cf. <qtbase>/cmake/QtPlatformAndroid.cmake
# and https://developer.android.com/ndk/guides/cpp-support#sr
vcpkg_check_linkage(ONLY_DYNAMIC_CRT)
endif()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modeling what is hard-coded in Qt.
Excludes it from vcpkg CI with its current Android configuration.

Comment on lines -2 to +7
set(VCPKG_CRT_LINKAGE static)
set(VCPKG_CRT_LINKAGE dynamic)
set(VCPKG_LIBRARY_LINKAGE static)
set(VCPKG_CMAKE_SYSTEM_NAME Android)
set(VCPKG_MAKE_BUILD_TRIPLET "--host=aarch64-linux-android")
set(VCPKG_CMAKE_CONFIGURE_OPTIONS -DANDROID_ABI=arm64-v8a)
set(VCPKG_CMAKE_SYSTEM_VERSION 28)
Copy link
Contributor Author

@dg0yt dg0yt Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not acceptable as part of this PR, but I strongly suggest these changes.

Runtime:

API level:

  • At the moment vcpkg initializes API level to 21. This excludes many ports from vcpkg CI. "Cumulative usage": 99.6%
  • Qt 6.7 needs 23. "Cumulative usage": 98.6%
  • Many POSIX APIs come with 24. "Cumulative usage": 97.2%
  • Qt 6.8 (LTS) needs 28. "Cumulative usage": 91.7%
  • I don't know if the default shall be changed in the triplet or in android.cmake. (In manifest mode with CMake, this should be passed through IMO.)

CC @BillyONeal for team feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@m-kuhn m-kuhn Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an API level adjustment which is acceptable for this PR: the qtbase port could show a warning (or error) if there are incompatible API levels and suggest setting it in the triplet, similar to what's discussed here https://github.com/microsoft/vcpkg/pull/41435/files#diff-da814fe5d578d93392e698157af490aeb2f7994eecdf94b272356ae97661ddc4R15-R17

if(VCPKG_TARGET_IS_OSX AND (NOT VCPKG_OSX_DEPLOYMENT_TARGET OR VCPKG_OSX_DEPLOYMENT_TARGET VERSION_GREATER_EQUAL "15.0"))
message(WARNING "Qt6.7 does not yet support macOS 15.0, consider adding set(VCPKG_OSX_DEPLOYMENT_TARGET 14.0) or earlier to a custom triplet (https://learn.microsoft.com/en-us/vcpkg/users/examples/overlay-triplets-linux-dynamic#overriding-default-triplets).")
endif()

A global vcpkg default system version policy similar or equal to the qt guideline could be discussed by the team in parallel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we can show an error for an insufficient API level from the portfile, or add an config-time failure to qtbase.

The osx case is a little bit different: The osx doesn't sufficiently control the deployment target, but silently takes the host osx version. Probably breaking vcpkg's ABI tracking promises.

For now I'm still assessing how much CI can be done for Qt on Android, and I tried to trigger the discussion about where to move the android CI to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next barrier: Pristine Qt doesn't support static library linkage for Android.
Linking apps fails with error: duplicate symbol: JNI_OnLoad (Core lib, platform plugin).

https://bugreports.qt.io/browse/QTBUG-32618
https://bugreports.qt.io/browse/QTBUG-99108

Note that the older issue also links to another issue with androiddeployqt.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there is a patch for qtbase that has received positive feedback from the qtandroid team but didn't end up being merged (without visible reasons)
https://codereview.qt-project.org/c/qt/qtbase/+/279386

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants