Add support for strict and unused dependency detection [Blocked: #4920] #4978
Conversation
Targets may not fully build at this stage since some changes to build toolchains (especially rules_kotlin) were done, but are being committed in a follow-up commit. This also involves very broad changes and significant complexity reduction throughout the app targets (by eliminating the need for genrules, reducing the amount of databinding generation needed, and simplifying defining both library and test targets).
This customizes rules_kotlin to support a variety of fixes to make both strict and unused deps viable for the Oppia Android codebase (though much more of the changes benefit unused deps checking). This includes significant patched-in customizations to rules_kotlin and changes the build pipeline such that rules_kotlin itself is built prior to using it. This is a small add-on in build time (particularly noticeable for initial builds) that the team will have to cope with until the patched changes can be accepted into rules_kotlin itself & released. This commit also enables actual error-based checking for both strict and unused deps.
This new script can automatically parse the output from a suggested or unused deps failure and lookup the correct Oppia-specific target to use, and even fix it for the author. This is similar to buildozer except it works better in the Oppia environment.
Enable Kotlin builder option to use ABI jars which introduces compilation avoidance and should improve incremental build times. See: https://github.com/bazelbuild/rules_kotlin/blob/master/CompileAvoidance.md#abi-support. Also, fix one unused dep situation that somehow changed with these recent options.
This addresses a core querying issue that leads to the wrong target being suggested. It also fixes normalization to improve target deletion, and fixes an issue with certain external repository detection based on the target prefix.
Specifically: 1. Adds a force resources option for oppia_android_library that is useful for times when a merge-only resource can't be added but resource generation is still needed. 2. Updates rules_kotlin to prioritize self-generated resources over library resources (since order matters in deps lists), fixing a large number of resource-only linking issues. 3. Updates rules_kotlin to properly add explicit deps for upper bound type generics, fixing cases when those types are only ever referenced in the file by generics bounding.
This improves upon target handling & parsing in SuggestBuildFixes by introducing proper structured parsing. This mainly improves confidence in the behavior of the utility, but it also allowed for a trivial introduction of sorting that matches buildifier's order (so no more needing to run buildifier after auto-fixing a dependency list). This new structure may have future benefits, as well, if it were to be integrated into BazelClient. Besides the parsing, this also updates SuggestBuildFixes to only analyze targets which explicitly have issues after attempting to build provided patterns. This *significantly* improves script performance (to the point where it may be viable to incorporate into an incremental workflow). It *may* be possible to also now incorporate the script into a CI check but building all targets may still be prohibitively expensive.
- Kotlin: introduces new configuration provides and properties to tweak
different settings, including strict/unused deps and incremental
building (which required a change to the rules_kotlin patch).
- Suggested fixes:
- Completely redid Starlark BUILD file parsing to use a more
structured lexical & syntatical parser for simplicity and
correctness. The tool now supports regeneration for much more
complex rule invocations (including single-line and glob
parameters).
- Removed replacement calculations, made auto-fixing more robust when
it doesn't work, and updated the new minimizing target computation
to be experimental rather than default (since it's been observed to
missed many unused_deps cases).
…t-deps Conflicts: app/BUILD.bazel scripts/src/javatests/org/oppia/android/scripts/license/BUILD.bazel scripts/src/javatests/org/oppia/android/scripts/maven/BUILD.bazel testing/src/main/java/org/oppia/android/testing/threading/BUILD.bazel
Specifically:
- It fixes a CODEOWNERS failure (though this might need to go into an
upstream branch, instead).
- It addresses a bunch of new strict/unused deps fixes that were needed
after recent changes to the rules_kotlin patch.
- It updates the SuggestBuildFixes script to:
- Remove fast mode (since it seems to break more often than work).
- Add granular, directory-based target break-downs and
short-circuiting for much better "don't re-run" detection.
- Added missing documentation in a bunch of places.
- Updated KDoc checker to only require KDocs in Kotlin functions that
are actually publicly exposed (based on their containers).
- Added new regex patterns & tests for forcing usage of the new
oppia_android_library and oppia_test targets.
- Added test suite placeholder for SuggestBuildFixes (#4977 is tracking
actually adding these tests in a follow-up PR).
…t-deps Conflicts: app/BUILD.bazel testing/BUILD.bazel
This requires bringing some of the transitive Kotlin dependencies up-to-date with the direct ones in order to fix the incompatibility issues brought up by Proguard. This *does* change runtime behaviors, but it should be more correct now.
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: 13 MiB (old), 12 MiB (new), 1002 KiB (Removed) APK download size (estimated): 12 MiB (old), 11 MiB (new), 977 KiB (Removed) Method count: 154854 (old), 151300 (new), 3554 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6239 (old), 6237 (new), 2 (Removed)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 12 MiB (old), 12 MiB (new), 1002 KiB (Removed)
Configuration hdpiAPK file size: 59 KiB (old), 59 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 56 KiB (old), 56 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 53 KiB (old), 53 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 102 KiB (old), 102 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 67 KiB (old), 67 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 76 KiB (old), 76 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 79 KiB (old), 79 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 8186 KiB (old), 7369 KiB (new), 817 KiB (Removed) APK download size (estimated): 7193 KiB (old), 6359 KiB (new), 833 KiB (Removed) Method count: 70694 (old), 65839 (new), 4855 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5194 (old), 5192 (new), 2 (Removed)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 7951 KiB (old), 7133 KiB (new), 817 KiB (Removed)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 89 KiB (old), 89 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 68 KiB (old), 68 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 8103 KiB (old), 7285 KiB (new), 817 KiB (Removed) APK download size (estimated): 7168 KiB (old), 6336 KiB (new), 831 KiB (Removed) Method count: 70694 (old), 65839 (new), 4855 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5194 (old), 5192 (new), 2 (Removed)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 7868 KiB (old), 7050 KiB (new), 817 KiB (Removed)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 89 KiB (old), 89 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 68 KiB (old), 68 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 8103 KiB (old), 7285 KiB (new), 817 KiB (Removed) APK download size (estimated): 7169 KiB (old), 6336 KiB (new), 832 KiB (Removed) Method count: 70694 (old), 65839 (new), 4855 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5194 (old), 5192 (new), 2 (Removed)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 7868 KiB (old), 7050 KiB (new), 817 KiB (Removed)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 89 KiB (old), 89 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 68 KiB (old), 68 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change) |
|
Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
oppiabot
bot
added
the
stale
oppiabot
bot
removed
the
stale
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: 13 MiB (old), 12 MiB (new), 1002 KiB (Removed) APK download size (estimated): 12 MiB (old), 11 MiB (new), 977 KiB (Removed) Method count: 154854 (old), 151300 (new), 3554 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6239 (old), 6237 (new), 2 (Removed)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 12 MiB (old), 12 MiB (new), 1002 KiB (Removed)
Configuration hdpiAPK file size: 59 KiB (old), 59 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 56 KiB (old), 56 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 53 KiB (old), 53 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 102 KiB (old), 102 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 67 KiB (old), 67 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 76 KiB (old), 76 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 79 KiB (old), 79 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 8186 KiB (old), 7369 KiB (new), 817 KiB (Removed) APK download size (estimated): 7193 KiB (old), 6359 KiB (new), 833 KiB (Removed) Method count: 70694 (old), 65839 (new), 4855 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5194 (old), 5192 (new), 2 (Removed)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 7951 KiB (old), 7133 KiB (new), 817 KiB (Removed)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 89 KiB (old), 89 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 68 KiB (old), 68 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 8103 KiB (old), 7285 KiB (new), 817 KiB (Removed) APK download size (estimated): 7168 KiB (old), 6336 KiB (new), 831 KiB (Removed) Method count: 70694 (old), 65839 (new), 4855 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5194 (old), 5192 (new), 2 (Removed)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 7868 KiB (old), 7050 KiB (new), 817 KiB (Removed)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 89 KiB (old), 89 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 68 KiB (old), 68 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 8103 KiB (old), 7285 KiB (new), 817 KiB (Removed) APK download size (estimated): 7169 KiB (old), 6336 KiB (new), 832 KiB (Removed) Method count: 70694 (old), 65839 (new), 4855 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5194 (old), 5192 (new), 2 (Removed)
Lesson assets: 105 (old), 105 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 7868 KiB (old), 7050 KiB (new), 817 KiB (Removed)
Configuration hdpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 89 KiB (old), 89 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 68 KiB (old), 68 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change) |
…t-deps Conflicts: .github/CODEOWNERS BUILD.bazel app/BUILD.bazel app/src/main/AppAndroidManifest.xml app/src/main/DatabindingAdaptersManifest.xml app/src/main/DatabindingResourcesManifest.xml app/src/main/RecyclerviewAdaptersManifest.xml app/src/main/ViewModelManifest.xml app/src/main/ViewModelsManifest.xml app/src/main/ViewsManifest.xml app/src/main/java/org/oppia/android/app/activity/BUILD.bazel app/src/main/java/org/oppia/android/app/application/BUILD.bazel app/src/main/java/org/oppia/android/app/application/alpha/BUILD.bazel app/src/main/java/org/oppia/android/app/application/alphakenya/BUILD.bazel app/src/main/java/org/oppia/android/app/application/beta/BUILD.bazel app/src/main/java/org/oppia/android/app/application/dev/BUILD.bazel app/src/main/java/org/oppia/android/app/application/ga/BUILD.bazel app/src/main/java/org/oppia/android/app/notice/testing/BUILD.bazel app/src/main/java/org/oppia/android/app/translation/BUILD.bazel config/src/java/org/oppia/android/config/AndroidManifest.xml data/BUILD.bazel data/src/main/java/org/oppia/android/data/backends/gae/BUILD.bazel defs/build_flavors.bzl defs/build_vars.bzl domain/BUILD.bazel domain/src/main/java/org/oppia/android/domain/onboarding/BUILD.bazel domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel instrumentation/src/java/org/oppia/android/instrumentation/application/BUILD.bazel scripts/BUILD.bazel scripts/assets/file_content_validation_checks.textproto scripts/assets/kdoc_validity_exemptions.textproto scripts/assets/maven_dependencies.textproto scripts/assets/todo_open_exemptions.textproto scripts/src/java/org/oppia/android/scripts/todo/TodoOpenCheck.kt scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt scripts/src/javatests/org/oppia/android/scripts/todo/BUILD.bazel scripts/third_party/versions/direct_maven_versions.bzl scripts/third_party/versions/maven_install.json testing/src/main/java/org/oppia/android/testing/network/BUILD.bazel testing/src/main/java/org/oppia/android/testing/platformparameter/BUILD.bazel third_party/macros/direct_dep_downloader.bzl third_party/macros/direct_dep_loader.bzl third_party/tools/kotlin/BUILD.bazel third_party/versions/direct_maven_versions.bzl third_party/versions/maven_install.json third_party/versions/transitive_maven_versions.bzl utility/BUILD.bazel utility/src/main/java/org/oppia/android/util/caching/BUILD.bazel utility/src/main/java/org/oppia/android/util/locale/BUILD.bazel utility/src/main/java/org/oppia/android/util/logging/firebase/BUILD.bazel wiki/Oppia-Bazel-Setup-Instructions.md
This fixes dependencies & gets oppia_dev.aab building at least with strict deps checking off.
This includes a lot, including: - Fixes to all BUILD files to ensure proper strict deps adherence. - A significant revamping of the SuggestBuildFixes utility to use a build graph instead of target expansion (so it's much faster now). Some logical fixes were also added. - A bunch of changes and version upgrades around the proto_rules and protobuf libraries to ensure they interact correctly with android_test_support (which has also been added explicitly). - Some revamping of third-party toolchain init.
Explanation
TODO: Finish.
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: