Fix part of #2747: Disable Gradle in CI #5629
Conversation
Coverage ReportResultsCoverage Analysis: SKIP ⏭️ This PR did not introduce any changes to Kotlin source or test files.
|
|
PTAL @adhiamboperes and @seanlip. Note that I will merge this on approval from either of you as it's blocking the SDK 34 PR (#5604) which is required for the upcoming 0.14 release of the app. |
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 0 bytes (No change) APK download size (estimated): 17 MiB (old), 17 MiB (new), 42 bytes (Added) Method count: 260202 (old), 260202 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6818 (old), 6818 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 0 bytes (No change) Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 4 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 6 bytes (Added) Method count: 116280 (old), 116280 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5786 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 4 bytes (Removed) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 8 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 6 bytes (Added) Method count: 116286 (old), 116286 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5786 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 4 bytes (Removed) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 4 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 9 bytes (Added) Method count: 116286 (old), 116286 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5786 (old), 5786 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 0 bytes (No change) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
|
LGTM, thanks @BenHenning. Is there a tracking issue for the steps needed for Gradle removal? Might be worth linking to it here. |
|
Ah sorry ignore that, it's literally part of the title :P |
|
Thanks both. Going ahead and merging this to unblock downstream. |
## Explanation Fixes #5012 Removes references and code corresponding to KitKat (SDK version 19) since the app has been set to a minimum version of Lollipop (SDK 21) since #3910 (roughly--technically KitKat was still supported but we no longer shipped a KitKat version). Because of this, the changes in this PR are largely a no-op from a build perspective. More broadly, this change is motivated by a desire to decrease CI time (which was already reduce considerably in the recent merge of #5629) by removing extraneous app builds being done in CI: - ``//:oppia`` since we should really only use ``//:oppia_dev`` moving forward. - Dev and alpha KitKat builds (the latter of which takes a long time since only the alpha job was building 2 proguarded builds of the app and thus taking much more time than the beta and GA build jobs). Separately, ``build_tests.yml`` and ``unit_tests.yml`` were updated to no longer support caching since we disabled this behavior a while back (#2884) and are unlikely to reintroduce it due to high storage costs. Some other noteworthy changes: - The optional providing of ``Retrofit`` and Retrofit services has been reverted since it's no longer necessary (and corresponding tests have been removed since there's no longer any condition to verify). - Code that was gated to run below Lollipop has been removed since it can't execute except on KitKat devices. - Support for a main dex target list has been removed as we're unlikely to need it with native multidex support (which wasn't an option for KitKat builds), though we may choose to reintroduce it to speed up cold app starts since it _can potentially_ help improve time-to-splash app startup performance. - Updated manifest min SDK values to avoid potential developer confusion on the min SDK version supported (though, strictly speaking, this doesn't technically matter with the way the app builds). - This updates the default min version supported for OS deprecation. No additional work is needed to actually activate the deprecation notice on the KitKat version of the app since we no longer deploy it, and OS deprecation wasn't added to the last version that was deployed. - Three file content pattern failure messages were updated to no longer reference KitKat (though the value of these checks mostly still seems realized, so I think it's fine to keep them in). - Some additional licenses were added due to tooling around the dev AAB (even though these aren't dependencies that ship with the app). I think it's fine including extra licenses in this case. - The protobuf license was corrected to BSD 3-Clause as declared for the dependency and in the GitHub repository for the dependencies. - This bumps version codes due to removing two flavors (for KitKat dev and KitKat alpha). - ``NetworkModuleTest`` has had its tests redesigned and better filled in in order to pass the code coverage minimum requirement (since old tests were removed). Note that it's not quite perfect in what it's testing, but it is at least verifying the complete configuration of the ``Retrofit`` instance, and the singleton properties of the two services currently supported (verifying that the services are set up would require a lot more testing that seems outside the direct scope of this test, so it seems okay to ignore here). - As part of the previous item's work, ``NetworkLoggingInterceptor`` had an issue fixed where it could cause an OkHttp exception to be thrown when trying to process logs (due to reading the response body twice). Reverting this change will now cause breakage failures in ``NetworkModuleTest``. - ``NetworkModule``'s initialization order was changed for slightly better network request logging (see the new comment in that file for more context). Note that there are some workflow job removals and renames in this PR. Any required CI checks will be correspondingly updated once this PR is approved and ready to merge (to avoid blocking any other in-flight PRs). ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only This shouldn't have an side effects on non-KitKat UIs, and KitKat support itself is being removed in this PR (so there's nothing to demonstrate).
Explanation
Fix part of #2747.
In order to unblock #5604, this PR disables Gradle in CI completely. This is an extreme mitigation to the issues discovered as part of that PR, but please see #5628 (comment) and that PR's main description for a detailed account of the attempt to migrate to the latest versions of Gradle. It seems that the best path forward both for this upcoming release and long-term is to outright remove Gradle, even though Bazel is not completely developer ready. The alternative (keeping a broken version of Gradle for several months as we migrate dependencies) seems far too disruptive, and not ideal in the long-term since we want to move away from having two build systems, anyway.
Please note that I will remove the required Gradle CI checks once the PR is approved (so that it can be merged).
This change and the eventual removal of Gradle has been announced in the CLaM chat. We'll need to reach out to specific newer team members in the new year once we actually perform the removal and update corresponding documentation.
Essential Checklist
For UI-specific PRs only
This is an infrastructure only change.