Skip to content

[Tooling] Fix Wear App Universal .apk download #11786

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

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

iangmaia
Copy link
Contributor

During the GitHub Release creation step (create_gh_release lane), the build failed for the Wear app.

This is due to the fact that we have a convention to increment the Wear App by 50000.
While running the create_gh_release lane, we try to download the Universal .apk from Google Play (calling the action download_universal_apk_from_google_play) using the App version code, which fails for the Wear app.

This PR fixes the issue by also adding a 50000 increment to the version code when trying to download the universal .apk while running a Wear App build.

@iangmaia iangmaia added the category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. label Jun 24, 2024
@iangmaia iangmaia added this to the 19.2 ❄️ milestone Jun 24, 2024
@iangmaia iangmaia requested review from AliSoftware and ThomazFB June 24, 2024 13:12
@iangmaia iangmaia self-assigned this Jun 24, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Jun 24, 2024

1 Warning
⚠️ This PR is assigned to the milestone 19.2 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@iangmaia iangmaia force-pushed the iangmaia/fix-wear-app-universal-apk-download branch from 215bbad to 30c3adb Compare June 24, 2024 13:18
@@ -1132,9 +1132,11 @@ platform :android do
private_lane :create_gh_release do |app:, version:, prerelease: false|
universal_apk_path = File.join(PROJECT_ROOT_FOLDER, 'artifacts', universal_apk_name(app, version))

# We have a convention that the Wear app's version code is incremented by 50000
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# We have a convention that the Wear app's version code is incremented by 50000
# We have a convention that the Wear app's version code is offset by 50000 from the Mobile app's one
# See https://github.com/woocommerce/woocommerce-android/blob/11ae376f2922e4b2eac3f87971b8c2246e56b37b/WooCommerce-Wear/build.gradle#L29

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed on 44dfb92

universal_apk_path = File.join(PROJECT_ROOT_FOLDER, 'artifacts', universal_apk_name(app, version))

# We have a convention that the Wear app's version code is incremented by 50000
build_code_to_download = app == MOBILE_APP ? build_code_current : (build_code_current.to_i + 50_000).to_s
Copy link
Contributor

@AliSoftware AliSoftware Jun 24, 2024

Choose a reason for hiding this comment

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

What about, to be consistent with how we implement this in other places:

validate_app_param!(app)
version_code_offset = { MOBILE_APP: 0, WEAR_APP: 50_000 }.fetch(app, 0)
download_universal_apk_from_google_play(
  
  version_code: build_code_current + version_code_offset
  
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea 👍 updated on 44dfb92.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jun 24, 2024

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commita6f44bd
Direct Downloadwoocommerce-wear-prototype-build-pr11786-a6f44bd.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jun 24, 2024

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commita6f44bd
Direct Downloadwoocommerce-prototype-build-pr11786-a6f44bd.apk

Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com>
@iangmaia iangmaia merged commit 37bef7e into release/19.2 Jun 25, 2024
15 checks passed
@iangmaia iangmaia deleted the iangmaia/fix-wear-app-universal-apk-download branch June 25, 2024 12:00
universal_apk_path = File.join(PROJECT_ROOT_FOLDER, 'artifacts', universal_apk_name(app, version))

# We have a convention that the Wear app's version code is offset by 50000 from the Mobile app's one
# See https://github.com/woocommerce/woocommerce-android/blob/11ae376f2922e4b2eac3f87971b8c2246e56b37b/WooCommerce-Wear/build.gradle#L29
version_code_offset = { MOBILE_APP: 0, WEAR_APP: 50_000 }.fetch(app, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

So this ended up not working, because the { key1: value1 } syntax makes a hash whose key is the symbol :key1

So here our hash ends up having the literal keys :MOBILE_APP and :WEAR_APP instead of interpreting those as constant names representing strings.

The fix for this will be:

Suggested change
version_code_offset = { MOBILE_APP: 0, WEAR_APP: 50_000 }.fetch(app, 0)
version_code_offset = { MOBILE_APP => 0, WEAR_APP => 50_000 }.fetch(app, 0)

Copy link
Contributor

Choose a reason for hiding this comment

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

Fix for this in #11854

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants