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

Draft: Android CI production build and Play Store auto-submission #3392

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pljones
Copy link
Collaborator

@pljones pljones commented Oct 2, 2024

Short description of changes

Revival of #2909

The primary goal of the patch was to enable Google Play Store automated uploads as part of the GitHub build process.

CHANGELOG: DO NOT MERGE

Context: Fixes an issue?

The patch not only adopts the new action (r0adkll/upload-google-play@v1) but changes dependencies:

Variable from to
COMMANDLINETOOLS_VERSION 6858069 7.0
ANDROID_NDK_VERSION r21d 25.1.8937393
ANDROID_PLATFORM android-30 android-33
ANDROID_BUILD_TOOLS 30.0.2 33.0.0
AQTINSTALL_VERSION 3.1.18 2.1.0
QT_VERSION 5.15.2 6.3.2

As a side effect, it enables the Jamulus.pro to build on Windows by removing the Linux dependency (or at least side-stepping it).

Code changes are a side effect of the move to Qt6 (but I think probably work in Qt5.15.2, too, so could be trialled).

Does this change need documentation? What needs to be documented and how?

We should add documentation about the build process somewhere but that's not part of this ticket. Unless it already exists and needs changing.

Status of this Pull Request

Proof of concept (not to be merged soon).

What is missing until this pull request can be merged?

  1. Decide if we do this in r4.0 rather than r3.12.0.
  2. Someone who knows the build chain needs to review it.
  3. We need to understand the proposed Github action and agree it's acceptable (and choose a version to pin).
  4. The updated dependencies need to be understood, including their effect on the supported Android versions and how that affects Jamulus documentation.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

rm -r "${QT_BASEDIR}/${QT_VERSION}/android/qml/"
# icu needs explicit installation
# otherwise: "qmake: error while loading shared libraries: libicui18n.so.56: cannot open shared object file: No such file or directory"
python3 -m aqt install-qt --outputdir "${QT_BASEDIR}" linux desktop "${QT_VERSION}" \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New dependencies need documenting.

needs.create_release.outputs.publish_to_release == 'true' &&
matrix.config.target_os == 'android'
id: publish_android
uses: r0adkll/upload-google-play@v1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to review this action and confirm it's okay.

Copy link
Member

Choose a reason for hiding this comment

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

I would rather check if it's possible like for iOS to use native cli tools or an official action by google.

# ANDROID_ABIS = armeabi-v7a arm64-v8a x86 x86_64

# get ANDROID_ABIS from environment - passed directly to qmake
ANDROID_ABIS = $$getenv(ANDROID_ABIS)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From where will these come when running make distclean; qmake ...?

There should be a set of defaults that can be overridden from the build chain.

ANDROID_VERSION_NAME = $$VERSION
ANDROID_VERSION_CODE = $$system(git log --oneline | wc -l)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should default to 1 and the build chain should pass the correct value:

  • 1 for anything but a release build (as we will not publish to Google Store anything but release builds).
  • retrieve last_release_android_version from permanent storage, add the number of ABI builds to be released plus one, store that in last_release_android_version permanent storage and pass it to the build.

ANDROID_VERSION_CODE = 1234 # dummy int value
} else {
# date-based unique integer value for Play Store submission
!defined(ANDROID_VERSION_CODE, var):ANDROID_VERSION_CODE = $$system(date +%s | cut -c 2-)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See above - I'd rather remove this entirely and default to 1 and use the

 ANDROID_VERSION_CODE = $$num_add($$ANDROID_VERSION_CODE, ...)

approach.


# enabled only for debugging on android devices
DEFINES += ANDROIDDEBUG
#DEFINES += ANDROIDDEBUG
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are we sure we want to remove this?

@@ -40,7 +40,7 @@
#endif
#include "util.h"
#ifdef ANDROID
# include <QtAndroidExtras/QtAndroid>
# include <QtCore/private/qandroidextras_p.h>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

QtCore/private/qandroidextras_p.h I'd rather not be including "private" header files. There is a standardised Qt6 permissions API, as far as I know -- if we're using Qt6, we should make all the necessary changes not hack the code...

So this is a blocker we'll need to address if Qt6 is required for Google Store.

@pljones
Copy link
Collaborator Author

pljones commented Oct 2, 2024

The main issue is the move to Qt6. I think that should be addressed separately. If this change requires Qt6, then it gets blocked until we've made the decision to move Android to Qt6 and made the necessary code changes correctly.

@ann0see
Copy link
Member

ann0see commented Oct 31, 2024

I can only test this on a PC with bliss os. It would be interesting to see if the qt6 build shows the same UI overflow issues as iOS

setup_qt
;;
build)
build_app_as_apk
# Build all targets in sequence
build_app "android_armv7"
Copy link
Member

Choose a reason for hiding this comment

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

It would be neat to parallelize builds of possible. Android takes ages already.

@pljones pljones added feature request Feature request android Android runtime issue release process Changes to the release process labels Nov 24, 2024
@pljones pljones force-pushed the feature/danryu/submit-to-Google-Play-Store branch from 12631a1 to aded45d Compare December 10, 2024 18:34
@ann0see ann0see mentioned this pull request Dec 11, 2024
5 tasks
@pljones pljones added this to the Release 4.0.0 milestone Dec 12, 2024
@pljones pljones linked an issue Dec 12, 2024 that may be closed by this pull request
@pljones pljones force-pushed the feature/danryu/submit-to-Google-Play-Store branch from aded45d to 0f08516 Compare December 18, 2024 13:40
danryu and others added 2 commits December 20, 2024 17:00
* android build conf changes

* fix shellcheck warnings

android fix audio permission handling

android fix androidextras include

android remove redundant jdk install

add ANDROID_NDK_ROOT comment

fix for clang-format style check

android add all ABI support

android: remove redundant minSdkVersion

clang-format fixes

android: fix manifest xml, remove old boilerplate

add ANDROID_SDK_ROOT shellcheck fix

android fix assignments in pro file

fix typo
@pljones pljones force-pushed the feature/danryu/submit-to-Google-Play-Store branch from 0f08516 to 3ef15a2 Compare December 20, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android Android runtime issue feature request Feature request release process Changes to the release process
Projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

Jamlus not supporting android target version 34
3 participants