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.

Sign up for GitHub

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: Set proper versionCode and versionName #2398

Merged
merged 4 commits into from
Feb 26, 2022
Merged

Android: Set proper versionCode and versionName #2398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc533bd
Autobuild: Allow overriding checkout's fetch-depth
hoffie Feb 16, 2022
1afc082
Android: Set ANDROID_VERSION_{CODE,NAME}
hoffie Feb 14, 2022
b49dec4
Jamulus.pro: Move ANDROID_ABIS to conditional
hoffie Feb 16, 2022
079a52a
Jamulus.pro: Use DISTFILES for AndroidManifest.xml
hoffie Feb 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/autobuild.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ jobs:
cmd2_build: "./autobuild/android/autobuild_apk_2_build.sh"
cmd3_postbuild: "./autobuild/android/autobuild_apk_3_copy_files.sh"
uses_codeql: true
checkout_fetch_depth: '0' # Jamulus.pro needs to count git history length for android versioning

- config_name: Linux (artifacts+codeQL)
target_os: linux
Expand Down Expand Up @@ -171,6 +172,7 @@ jobs:
uses: actions/checkout@v2
with:
submodules: true
fetch-depth: ${{ matrix.config.checkout_fetch_depth || '1' }}
Copy link
Member

Choose a reason for hiding this comment

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

Does the checkout_fetch_depth: '0' further up evaluate here as false or true?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't found explicit documentation, but the CI confirms that it does what it should: Android uses a full checkout while the other targets (e.g. Linux) only fetch the most recent commit:

Therefore, I assume that '0' as a non-empty string evaluates to true (probably like Python).

Copy link
Member

Choose a reason for hiding this comment

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

Therefore, I assume that '0' as a non-empty string evaluates to true (probably like Python).

And unlike Perl and PHP!


# Enable caching of downloaded dependencies
- name: "Cache Mac dependencies"
Expand Down
9 changes: 6 additions & 3 deletions Jamulus.pro
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ win32 {
LIBS += -framework AVFoundation \
-framework AudioToolbox
} else:android {
ANDROID_ABIS = armeabi-v7a arm64-v8a x86 x86_64
ANDROID_VERSION_NAME = $$VERSION
ANDROID_VERSION_CODE = $$system(git log --oneline | wc -l)
message("Setting ANDROID_VERSION_NAME=$${ANDROID_VERSION_NAME} ANDROID_VERSION_CODE=$${ANDROID_VERSION_CODE}")

Copy link
Member

Choose a reason for hiding this comment

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

I couldn't see in the build log where these get used. If they are used silently, then perhaps they could be echoed too?

Copy link
Member Author

@hoffie hoffie Feb 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yep, they're qmake-internal variables. I've added a message() call to output them now.

Copy link

@NickHyHo NickHyHo Feb 22, 2022 •
edited by hoffie
Loading

Choose a reason for hiding this comment

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

Just tried installing jamulus_3.8.2dev-5f108fd_android.apk over 3.8.0 ... result is as before, i.e. version number is reported as being different (see screenshot attached), but install ends with message "App not installed". I suspect this is "as expected" at the moment! BTW, if you want me to check any functionality on any new build let me know - I have several Android devices at my disposal, and can force a new build to install by uninstalling any old one, of course.

Copy link

Choose a reason for hiding this comment

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

Sorry, first time I've posted an image - didn't realise it'd come out that big on screen!

Copy link
Member Author

Choose a reason for hiding this comment

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

(I've edited your comment to use <img src="...existing url.." width="2002>to reduce the size a bit.

result is as before, i.e. version number is reported as being different (see screenshot attached)
but install ends with message "App not installed". I suspect this is "as expected" at the moment!

Thanks for the screenshot. Yes, seeing the real version number here is definitely an improvement. And yes, it's expected that this PR does not solve all update problems on Android. While I had hoped for this to be the case, the PR doesn't claim to do it :)

BTW, if you want me to check any functionality on any new build let me know - I have several Android devices at my disposal, and can force a new build to install by uninstalling any old one, of course.

Great, thanks! I'll likely ask again as part of #2404.

# we want to compile with C++14
CONFIG += c++14

Expand All @@ -221,7 +226,7 @@ win32 {

LIBS += -lOpenSLES
ANDROID_PACKAGE_SOURCE_DIR = $$PWD/android
OTHER_FILES += android/AndroidManifest.xml
DISTFILES += android/AndroidManifest.xml

# if compiling for android you need to use Oboe library which is included as a git submodule
# make sure you git pull with submodules to pull the latest Oboe library
Expand Down Expand Up @@ -1166,5 +1171,3 @@ contains(CONFIG, "disable_version_check") {
message(The version check is disabled.)
DEFINES += DISABLE_VERSION_CHECK
}

ANDROID_ABIS = armeabi-v7a arm64-v8a x86 x86_64