bazelbuild / bazelbuild/rules_android
Embedded shrinker rules from META-INF are selected incorrectly (both with and without r8_extract_embedded_proguard_specs), silently dropping keep rules
- Dominant language
- Java
- Stars
- 203
- Forks
- 95
- PR merge metrics
- No merged PRs in 30d
Description
## Summary
Libraries ship embedded shrinker rules in two locations: legacy `META-INF/proguard/` and versioned `META-INF/com.android.tools/r8[-from-X][-upto-Y]/`. AGP selects between them per jar, with version gating. rules_android gets this wrong in both flag states, and the failure mode is silent loss of keep rules, which under R8 full mode turns into dead-code elimination of live classes.
We hit this migrating a large app from Gradle: R8 removed our Gson-deserialized model classes and the whole push notification pipeline stopped working, with no build error.
Environment: rules_android 0.7.3, Bazel 9, R8 9.1.31.
## Case 1: r8_extract_embedded_proguard_specs=false (default)
R8 is invoked with a single merged deploy jar as its program input (rules/android_binary/r8.bzl:128). Modern R8 (9.x) extracts embedded rules from program inputs itself, per archive: if an archive contains any `META-INF/com.android.tools/` entries, that archive's legacy `META-INF proguard/` rules are ignored (see EmbeddedRulesExtractor in R8).
The merged deploy jar is one archive for all dependencies. kotlinx-coroutines contributes `META-INF/com.android.tools/r8*/`, which suppresses the legacy rules of *unrelated* libraries that only ship legacy rules — in our case gson.pro, protobuf.pro, okio.pro, androidx annotations.pro. Merging destroys the per-archive provenance R8's selection relies on.
Repro sketch: deps on kotlinx-coroutines-core + gson, R8 full mode, a class only instantiated via `Gson.fromJson(json, Foo.class)`. `unzip -l` on the deploy jar shows both `META-INF/com.android.tools/r8-from-1.6.0/coroutines.pro` and `META-INF/proguard/gson.pro`; the resulting dex has the coroutines ServiceLoader optimization applied (proving R8 did extract from the merged jar) but Foo is removed.
## Case 2: r8_extract_embedded_proguard_specs=true
tools/android/proguard_extractor_lib.py concatenates *everything* under both `META-INF/proguard/` and `META-INF/com.android.tools/` with no version gating and no precedence. Concrete consequences with kotlinx-coroutines at R8 9.x:
- `META-INF/com.android.tools/r8-upto-3.0.0/coroutines.pro` is applied even though the current R8 is 9.x. That file contains `-keep class kotlinx.coroutines.android.AndroidDispatcherFactory {*;}`, which defeats R8's ServiceLoader optimization — the exact thing the versioned directory scheme exists to prevent (Kotlin/kotlinx.coroutines#3111).
- Legacy `META-INF/proguard/coroutines.pro` is applied on top, plus `META-INF/com.android.tools/proguard/` files, which AGP never applies.
The existing unit tests (testR8RulesVersionedSubdirs, testLegacyAndR8RulesCombined) assert this concatenation behavior, so the bug is currently encoded as expected behavior.
## Reference semantics
AGP: ExtractProGuardRulesTransform + FilterShrinkerRulesTransform (per-jar; tools dir existence suppresses legacy; only `r8*` subdirs, `from ` inclusive / `-upto-` exclusive against the R8 version; union of all matching dirs; `com.android.tools/proguard/` ignored). R8 side: EmbeddedRulesExtractor implements the same shape per archive. AAR handling differs too: AGP reads `META-INF/com.android.tools/**` from classes.jar and falls back to the AAR's proguard.txt only if none found; rules_android's AAR extractor concatenates both.
## What we did
We're running a local patch (https://github.com/Dolfik1/rules_android/commit/c0e1a1a79c6f0cea9a4c6f222c35d9723a20067f) that:
1. Rewrites the extractor to per-jar AGP semantics and feeds it the individual library runtime jars (via a params file) instead of the merged deploy jar, with `--r8_version` for gating (plumbed as a toolchain attr). It also emits a manifest listing every discovered rule entry with a verdict (SELECTED / SUPPRESSED_BY_TOOLS_DIR / VERSION_EXCLUDED / NON_R8_DIR), which turned out to be very useful for verifying parity with the Gradle build.
2. When the flag is on, strips `META-INF/proguard/**` and `META-INF/com.android.tools/**` from a copy of the deploy jar before it is passed to R8, so R8's own extractor can't re-apply a wrong merged-archive selection on top. The original deploy jar is untouched for its other consumers.
3. Fixes the AAR extractor to match AGP (classes.jar tools rules first, proguard.txt only as fallback).
Would a PR along these lines be welcome? If yes: one PR or split (extractor+gating / strip step / AAR path)? And do you want the new behavior gated behind anything beyond the existing r8_extract_embedded_proguard_specs flag?
Contributor guide
Research direction
Start with rules/android/r8.bzl:128 and tools/android/proguard_extractor_lib.py, then run testR8RulesVersionedSubdirs and testLegacyAndR8RulesCombined. Compare their current concatenation behavior with the referenced AGP and R8 selection semantics, including the AAR path. Done means the tests establish per-jar selection, version gating, and correct AAR fallback without silently dropping or wrongly applying rules.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, java, python
- Domain
- build-system, mobile, tooling
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100