Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Android startup, credential handling, Qt 6 compatibility, and Play Store authentication contain blocking defects.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Reworks Android builds into legacy APK and Play Store AAB streams, with local tooling and Qt 6 support.
Changes:
- Externalizes Android toolchain configuration and adds local build wrappers.
- Adds multi-mode/ABI packaging, signing, and Play Store publishing.
- Adds Qt 6 microphone permission handling and documentation.
File summaries
| File | Description |
|---|---|
tools/android-build.sh |
Adds local build driver. |
tools/android-build_play-store.settings.example |
Provides Play Store settings template. |
tools/android-build_legacy.settings.example |
Provides legacy settings template. |
src/main.cpp |
Adds Qt 6 microphone permissions. |
Jamulus.pro |
Updates Android versioning and linking. |
COMPILING.md |
Documents Android build streams. |
android/AndroidManifest.xml |
Changes Android application ID and SDK handling. |
.gitignore |
Ignores Android outputs and credentials. |
.github/workflows/bump-dependencies.yml |
Extends dependency bump coverage. |
.github/workflows/autobuild.yml |
Adds Android matrices, signing, and publishing. |
.github/autobuild/get_build_vars.py |
Adds standalone version output. |
.github/autobuild/android.sh |
Rewrites Android setup and packaging. |
.github/autobuild/android-play-store.sh |
Prepares AAB uploads. |
.github/autobuild/android-dependencies_play-store.sh |
Pins Play Store dependencies. |
.github/autobuild/android-dependencies_legacy.sh |
Pins legacy dependencies. |
Review details
- Files reviewed: 15/16 changed files
- Comments generated: 15
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ANDROID_BUILD_TOOLS=36.0.0 | ||
| AQTINSTALL_VERSION=3.3.0 | ||
| ANDROID_JAVA_VERSION=17 | ||
| QT_VERSION=6.10.2 |
| --> | ||
| <manifest package="com.github.jamulussoftware.jamulus" xmlns:android="http://schemas.android.com/apk/res/android" android:versionName="-- %%INSERT_VERSION_NAME%% --" android:versionCode="-- %%INSERT_VERSION_CODE%% --" android:installLocation="auto"> | ||
| <uses-sdk android:minSdkVersion="21" android:targetSdkVersion="28"/> | ||
| <manifest package="app.jamulus.jamulus" xmlns:android="http://schemas.android.com/apk/res/android" android:versionName="-- %%INSERT_VERSION_NAME%% --" android:versionCode="-- %%INSERT_VERSION_CODE%% --" android:installLocation="auto"> |
| if: steps.upload.outputs.INTERNAL_SHARING_DOWNLOAD_URLS != '' | ||
| run: echo "Internal sharing download URL(s), add to release notes if desired -> ${{ steps.upload.outputs.INTERNAL_SHARING_DOWNLOAD_URLS }}" |
b6709b0 to
bc8eb9f
Compare
15ec53c to
7f1644e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Play Store setup is currently broken, while packaging and keystore handling contain correctness and security defects.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
.github/autobuild/android.sh:345
- This accepts
BUILD_MODES="debug release"for Play Store publication, butget-artifactsthen places two AABs indeploy/andandroid-play-store.shrejects that directory because it requires exactly one AAB. The documented defaults therefore maketools/android-build.sh --stream play-store build get-artifacts play-storefail unless a settings file happens to narrow the modes. Require release as the only mode here, or make the preparation step select the release artifact explicitly.
[[ "$PUBLISH_MODE" == "" || " $BUILD_MODES " == *" release "* ]] || {
echo "Play Store publication requires a release build" >&2
exit 1
}
.github/autobuild/android.sh:479
validate_signingalways initializesANDROID_KEYSTORE_FILE, including with a generated debug key, so this condition adds--releasefor everybuild_mode. Consequently the nominaldebugbuild is packaged as a release package and then renamed as an_debugartifact. Only add release/signing arguments whenbuild_modeisrelease; letandroiddeployqtcreate its normal debug package otherwise.
if [[ -n "${ANDROID_KEYSTORE_FILE:-}" ]]; then
package_args+=(--release)
.github/autobuild/android.sh:427
- The redirected file is created before its permissions are tightened, and the cleanup trap is installed only after decoding succeeds. If decoding an invalid/truncated secret fails under
set -e, partially decoded keystore material remains at a predictable path with the process umask's permissions. Set a restrictive umask and install the trap before creating or decoding the file.
ANDROID_KEYSTORE_FILE=~/.jamulus-android-signing.jks
printf '%s' "$JAMULUS_ANDROID_KEYSTORE_BASE64" | base64 --decode > $ANDROID_KEYSTORE_FILE
chmod 600 $ANDROID_KEYSTORE_FILE
trap 'rm -f $ANDROID_KEYSTORE_FILE' EXIT
android/AndroidManifest.xml:49
- Changing the legacy APK's package name makes Android treat it as a different application, so users of the existing
com.github.jamulussoftware.jamulusAPK cannot install this as an update and will not retain the old app's data/update path. This is also effective whileJamulus.prostill reports3.12.5dev, despite the documentation assigning the migration to 4.0.0. Retain the legacy ID or provide an explicit, version-aligned migration/release plan for this breaking identity change.
<manifest package="app.jamulus.jamulus" xmlns:android="http://schemas.android.com/apk/res/android" android:versionName="-- %%INSERT_VERSION_NAME%% --" android:versionCode="-- %%INSERT_VERSION_CODE%% --" android:installLocation="auto">
- Files reviewed: 16/17 changed files
- Comments generated: 2
- Review effort level: Balanced
| curl -s -o downloadfile.zip "https://dl.google.com/android/repository/android-ndk-${ANDROID_NDK_VERSION}-linux-x86_64.zip" | ||
| unzip -q downloadfile.zip -d ndk-tmpdir | ||
| rm -f downloadfile.zip | ||
|
|
||
| mv ndk-tmpdir/"android-ndk-${ANDROID_NDK_VERSION}"/* . |
| Both streams support devices from Android 5.0/API 21. The Legacy APK is not | ||
| for Google Play. The Play Store AAB targets API 36, as required for current | ||
| Google Play submissions. |
|
27a9bd5 to
17c272f
Compare
|
No, I didn't ask for changes. I was telling you I had done the work. When I say I have clarified something, that doesn't mean "undo what has been done". When I say "done" that doesn't mean "do it differently". When I tell you why something isn't being done, that doesn't mean "do it anyway". There was nothing left to do. |
9c053a9 to
a18eb37
Compare
9fe3779 to
46ec0b8
Compare
4708835 to
ded218c
Compare
63e1f0b to
0f5db5c
Compare
815020a to
7cab639
Compare
7cab639 to
bd689d6
Compare
Short description of changes
cache-keysbranch)debugandreleasebuilds for all four ABIs (two APKs on the release)Note
The existing
com.github.jamulussoftware.jamulusAPK is being changed toapp.jamulus.jamulusfor both Qt5 and Qt6 builds, with only the latter targeting release to Play Store.CHANGELOG: New Android Packaging workflow, with Play Store-ready build
Context: Fixes an issue?
Very low uptake on Android vs Play Store updates.
Does this change need documentation? What needs to be documented and how?
Yes. We'll need the Release Announcement to highlight that the old build needs removing completely and that the new build can, once we stabilise the Qt6/Android build and put it on Play Store, then be updated through Play Store.
Status of this Pull Request
Very much draft.
What is missing until this pull request can be merged?
Needs to actually be tested on multiple devices.
Checklist