Update CI Workflow

[AliSoftware]

This is part of project paaHJt-2s8-p2

ℹ️ This PR targets release/18.2. I've made this choice so that it gives me the opportunity to use it for the current release cycle (and test it in real conditions ASAP), assuming it's approved and merged before the end of Sprint.

What it does

• Update the .circleci/config.yml to build Jetpack Vanilla in the same workflow (and single API trigger) that it builds Wordpress Zalpha and WordPress Vanilla
• Update the Fastfile to adapt the trigger_release_build lane accordingly
• Update other lanes of the Fastfile to stop requiring passing the app: parameter when using lanes for usual release duties (code_freeze, new_beta_release, etc)
• Update the app: parameter of version-management-related actions called from Fastlane to a dummy value wp_jp
  • This is a way for me to check that the app: param for those actions don't have any impact anymore, as per Update Android versioning helpers to use unified versions for JP/WP release-toolkit#298 changes. If we confirm after this PR that things continue working, that would confirm we can delete the app: param completely for those actions in the release-toolkit during next iteration.
  • The app: param is still present and used for some other lanes, like build_bundle and friends, as it is used by the .circleci/config.yml to know which app to build; so that param will stay even after the refactor. This is why not all app: params have been changed to dummy values, but only the relevant ones
• Now builds WP WP Alpha, WP Vanilla and JP Vanilla all in parallel instead of series (see full thread in p1630938604091100-slack-CC7L49W13, summary in Slack comment p1631035688017600-slack-C020B5ME5L0, and Update CI Workflow #15273 (comment))

What it does not (yet)

• It still generates a GitHub release for each of the 3 builds; I plan to unify that to generate a single GH release for the 3 soon (we will need to wait on other jobs and pass artefacts around to collect them for the release draft)

This will be addressed in a subsequent PR, as it feels safer to do those kind of changes incrementally – especially to the lack of type-checking in the Fastfile and toolkit there makes me be quite cautious 😉 .

To test

• Run bundle exec fastlane new_beta_release and check that it updates the translations, bumps the version correctly in the version.properties with a single common value for JP and WP, lint the project only for WP (since both are using same strings), then trigger a release build on CI
• Cancel the build on CI, since that build will be triggered on the release/18.2 branch on CI, which doesn't have the changes in the Fastfile nor .circleci/config.yml
• Cut a new branch from this jetpack-infra/ci branch, and cherry-pick the version bump that was made in the last step in release/18.2 branch, and push it to the remote.
• Trigger a release build on this new branch using bundle exec fastlane trigger_release_build branch_to_build:<your-branch-name>
• Check that the CI builds WordPress Zalpha, WordPress Vanilla and Jetpack Vanilla all in parallel, and succeeds the builds(†), then uploads to PlayStore, and creates the GitHub Release for each(‡)
• Verify that Installable Builds for both WP and JP are still properly built and created with the expected version.

Notes

(†) Given p1631035422011900-slack-C020B5ME5L0, there is a chance that one of the 3 jobs in the workflow will fail with an Out of Memory (Gradle Daemon Disappeared) issue, and you will have to restart just the failed job ("Restart from failed") to finally get it green. This is a known issue, that was deemed acceptable to live with until we find time for the BuildKite migration (and not doing parallelization led to the recent aapt2 crash anyway, so at least it's still better)

(‡) Last commit bebbe1b disables the upload to PlayStore and GitHub Release creation, in order to facilitate testing (allowing you to trigger release builds using API multiple times without risking the duplicate versionCode issue or spamming GH releases while testing). I intend to revert bebbe1b before merging that PR.

Cleanup

• Revert the last 2 commits (translations update and version bump) from the release branch
• Delete your temporary branch you cut from this one, both locally and from remote.
• Delete the 3 GitHub release drafts that were created during those tests
• Go to the PlayStore for WordPress, delete the 2 .aab (alpha and beta) in "App Bundler Explorer" tab, and also discard the 2 release drafts (alpha and beta)
• Do the same for the PlayStore for Jetpack – delete the single .aab for beta and the beta release draft

[peril-wordpress-mobile]

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

[peril-wordpress-mobile]

You can test the changes on this Pull Request by downloading the APKs:

• Jetpack-pr-15273-build-115148.apk
• WordPress-pr-15273-build-115148.apk

[oguzkocer]

The base branch was automatically changed to trunk, so I updated it to develop.

[oguzkocer]

The code changes look good to me. I think it'd be best to test this after the upcoming code freeze, so I am skipping that for now.

Temporarily allow force-push on the release/18.1 branch in the GitHub Settings, revert the last 2 commits (translations update and version bump), and forbid force-push in the branch settings again

I think we should just create revert commits instead of force pushing. I don't think the extra few commits is bad enough to worth the hassle.

---

P.S: Should we have a prefix for the release toolkit lanes or the internal ones, it's very difficult to know which is which, so I keep having to look things up. Also the lack of static typing is just unbearable to me, so I appreciate you working on these @AliSoftware 🙇

[AliSoftware]

I think we should just create revert commits instead of force pushing. I don't think the extra few commits is bad enough to worth the hassle.

Yep good point, revert-commits would be better 👍 (updated the instructions)

P.S: Should we have a prefix for the release toolkit lanes or the internal ones, it's very difficult to know which is which, so I keep having to look things up.

Agreed, I still have to look up sometimes to see if it's an internal lane or a release-toolkit action. All of our release-toolkit actions for Android start with the prefix android_ (and ios_ for iOS ones), so that helps. But for the common ones (both applicable to iOS and Android) like check_translation_progress or setbranchprotection and the likes, we don't have such common prefix and I agree that would help!

Also the lack of static typing is just unbearable to me, so I appreciate you working on these @AliSoftware 🙇

Oh yeah I miss the static typing as well!! This is also why (1) I split my plan to do those Fastfile and toolkit changes in very small increments so we can test changes little by little in real world (2) I am really hoping to find some time to get to paaHJt-1Zs-p2 at some point to help a bit with that (even if still not perfect) and why we opened wordpress-mobile/release-toolkit#201 a while ago 😅 . But so far it still always feels like dangerous territory every time I touch the toolkit…

[AliSoftware]

Reminder to self:

• Fix the case when the versionName is overriden via -P via the CLI. See [Tooling] Use same version for WP and JP #15230 (comment)

As this is an issue that is related to CI (since we use that override for Installable Builds built from CI) it would make sense for me to fix this during this PR.

[AliSoftware]

@oguzkocer Figured it'd only take a couple of seconds after all so implemented it in 900f65d to avoid forgetting about it.

$ ./gradlew -q printAllVersions -PinstallableBuildVersionName="pr-123"

Checking whether `configure` binary is present
wordpressWasabiDebug: [alpha-316] ([1102])
wordpressZalphaDebug: [alpha-316] ([1102])
wordpressJalapenoDebug: [pr-123] ([1])
wordpressVanillaDebug: [18.1-rc-2] ([1101])
jetpackWasabiDebug: [alpha-316] ([1102])
jetpackZalphaDebug: [alpha-316] ([1102])
jetpackJalapenoDebug: [pr-123] ([1])
jetpackVanillaDebug: [18.1-rc-2] ([1101])
wordpressWasabiRelease: [alpha-316] ([1102])
wordpressZalphaRelease: [alpha-316] ([1102])
wordpressJalapenoRelease: [pr-123] ([1])
wordpressVanillaRelease: [18.1-rc-2] ([1101])
jetpackWasabiRelease: [alpha-316] ([1102])
jetpackZalphaRelease: [alpha-316] ([1102])
jetpackJalapenoRelease: [pr-123] ([1])
jetpackVanillaRelease: [18.1-rc-2] ([1101])

[oguzkocer]

@AliSoftware That looks good to me, thanks for fixing it! 🙇

[AliSoftware]

PR Update: New changes

• I've rebased the branch on top of release/18.2 and switched the target branch of the PR to it too.
• I've also pushed new commits on top of it to parallelize the 3 builds, given that it is required to make the CI work given the recent aapt2 failures I started to have during last code freeze (and unrelated to this PR as I didn't use it for code freezing 18.2).

This parallelization was planned to be for a separate PR, but since it have become a blocker I've now included it into this one.

Status

• This works and solves the recent aapt2 issues.
• There are now transient Out of Memory ("Gradle Daemon disappeared") errors randomly but often happening in one of the 3 jobs of the workflow (though not always the same it seems) the first time the workflow is run. But at least now I can just retry only the job that failed, and I also don't have to wait as much as before until I can retrigger the next CI, so it is still a win.

The solution for the OOM issues will be to migrate the Release jobs to BuildKite, but that's part of a quite different project, so I'm ok with living with it for a couple of sprints until we get to that – even if that means I'll likely have to retry one of the 3 jobs most of the time when I do a beta or release, that's still way better than the state of things prior to that PR.

Review

As the last changes I made work, this should be ready for another review. \cc @oguzkocer

⚠️ Note though that I will need to revert the last commit bebbe1b – which was made to make testing multiple builds easier – before we merge that PR. I will do that tomorrow (Haven't done so yet because it's past my EOD now – and writing that from mobile – but also because it could help you test the PR more easily by doing multiple dummy CI runs if you need without affecting GH or PlayStore)

[peril-wordpress-mobile]

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

[jkmassel]

This looks solid to me without having tested – just left one note.

[jkmassel]

Just thinking about this one – by doing this we lose the ability to install newer installable builds overtop of older ones. Do we care about that?

I'm not sure how big those numbers can get, but could we work around this by having the versionCode be new Date().to_int() or similar? Or is that just overthinking it?

[AliSoftware]

That's a good question.

Note though that this was already the case (having the versionCode ne the same for multiple installable builds) before that change, because versionCode is only bumped during new betas but there are many PRs done (especially on develop) between two betas (and between two merges of the release branch to develop after a beta). So all those PRs' Installable Builds were already using the same versionCode (the one from the last beta since PR was opened), and it didn't seem to have been a problem so far? 🤔

[oguzkocer]

I think what @jkmassel means is that (correct me if I am wrong), since the previous installable build versions had a higher version code, developers won't be able to update them without first deleting the existing build once. After that, I agree that the same version code should allow updates, but I haven't tested the behavior.

I don't think it's a big issue since these are just test builds, but I do like the date idea 😅

[AliSoftware]

Ah yeah indeed I think people might need to delete old installable builds before installing the ones which will be versionCode 1.

I don't think that's a big issue as it's one time thing; and was also already the case if you tested a PR 1234 first and then test PR 987 which was cut from develop before 1234 and with a potentially older versionCode. In fact now having versionCode 1 for all of them might even solve that case (after the initial deletion past this PR)? 🤔

(Which the date trick won't solve btw—even if it could be worth considering for other reasons than this, ie to easily identify the build date for those?)

[oguzkocer]

I am good either way. If it turns out to be an issue, it's easy enough to improve since we are all aware of it. So, whatever you want to go with, it's a 👍 from me.

[AliSoftware]

Whoops made comments earlier this morning… but didn't realize they were pending and waiting for me to submit review 😅 Here they are.

[AliSoftware]

create_release:false upload_to_play_store:false are from the temporary commit bebbe1b made for testing the PR, and which will be reverted before merge. (Ditto for lines 127 and 137)

[AliSoftware]

Ah yeah indeed I think people might need to delete old installable builds before installing the ones which will be versionCode 1.

I don't think that's a big issue as it's one time thing; and was also already the case if you tested a PR 1234 first and then test PR 987 which was cut from develop before 1234 and with a potentially older versionCode. In fact now having versionCode 1 for all of them might even solve that case (after the initial deletion past this PR)? 🤔

(Which the date trick won't solve btw—even if it could be worth considering for other reasons than this, ie to easily identify the build date for those?)

[oguzkocer]

I followed these final test instructions that were verified by @AliSoftware on Slack:

• Trigger a release build on this branch using bundle exec fastlane trigger_release_build branch_to_build:jetpack-infra/ci
• Check that the CI builds WordPress Zalpha, WordPress Vanilla and Jetpack Vanilla all in parallel, and succeeds the builds(†)
• Verify that Installable Builds for both WP and JP are still properly built and created with the expected version.

The release builds were all green. The installable build was in a failed state due to cancellation, so I restarted the build and then tested both apks and verified the version number. Here is the one from WordPress:

[AliSoftware]

For information: the debug commit 🚧 bebbe1b was reverted post-merge directly on the release/18.2 branch, via fbf2938