android_binary doesn't configure ProGuard/R8 for Android?

[cpsauer]

Hi, wonderful Bazel folks,

This issue is about ProGuard/R8 for android_binary, requiring also a minor change to the android_sdk_repository rule. I think it's probably more bug than feature request. If I'm right, I'm guessing the fix shouldn't take too much time.

The issue itself: It looks like android_binary might be forgetting to configure its ProGuard invocation for Android by not including the default Android configuration.

Background: For ProGuard/R8 to work properly for Android, one is supposed to supply the default Android ProGuard configuration file so things are set up to work properly. (For more, see https://developer.android.com/studio/build/shrink-code). I was looking closely at all this because I was building an extension to build, ProGuard, and distribute aars, which would solve #348. That meant I was reading the Android docs and looking at android_binary as a reference.

Evidence that makes me think Bazel isn't supplying this important configuration file:

• In the Bazel source, I don't see any reference to the default Android configurations, (neither proguard-android.txt nor proguard-android-optimize.txt).
• Running a Bazel build with an empty ProGuard configuration strips everything, rather than preserving things protected by android.support.annotation.Keep, or other key things protected by those default files.
• Dumping the ProGuard command with, e.g. bazel aquery "mnemonic('Proguard',deps(//:android_binary_target))", and examining the specs used didn't show any indication of using the Android configuration files.
• The Android configurations don't seem to be made accessible from android_sdk_repository.

Expected behavior would be android_binary correctly, automatically configuring ProGuard/R8 for Android when it runs ProGuard. Or at least having android_sdk_repository expose the specs needed to manually do so.

Suggested fix:

1. Export the default ProGuard configuration files from the android SDK.
   1. Like proguard, these used to be directly available in the SDK (under tool/proguard/), but that's long deprecated in favor of having the gradle plugin generate them on the fly.
      See https://android.googlesource.com/platform/tools/base/+/refs/heads/mirror-goog-studio-main/build-system/gradle-core/src/main/java/com/android/build/gradle/ProguardFiles.java
      I don't see a good way of accessing that code through the SDK (I see neither gradle nor that class in a grep of the SDK's jars), so sadly, we'll probably need to bundle them separately, as Bazel does with ProGuard.
2. Whenever ProGuard/R8 runs, additionally supplying @androidsdk//:tools/proguard/proguard-android-optimize.txt for opt builds and @androidsdk//:tools/proguard/proguard-android.txt otherwise, just as an additional private attribute, and automatically added to the ProGuard configurations on the command line.
   1. Offhand I wonder whether ProGuard should really be getting run in dbg or fastbuild modes, but that's a separable thing...

Thanks!
Chris
(ex-Googler)

P.S. All testing was on bazel rolling, 6.0.0-pre.20220208.2.

[cpsauer]

I took a solid crack at part 1 of the fix (above) in #14910.

Originally, I was thinking I'd just quickly add the configurations to an export_files. But then I decided that weaving them into the Android SDK Provider (AndroidSdkInfo) would be cleaner, if a lot more work. This way, android_binary and friends will already have access and it'll be parallel to ProGuard.

...and then I figured I might as well just take a shot at part 2, too, since I'd made it easy :)

For any Bazelers reading this, I'd love your review!
And while I have you here: Thoughts on changing things so Proguard only runs during opt builds? Feels weird to strip debug...

[ted-xie]

Cross-referencing and copy-pasting some discussion in #14910 about why android_binary doesn't configure proguard by default:

The internal toolchains for proguard point to a sh_binary wrapper around proguard that gets invoked with a config file embedded in the action; the config file itself is configurable between several modes (i.e. don't optimize, regular optimization etc) based on a select. The proguard call in Bazel points to a jar in the remote_java_tools repo, which explains why the behavior differs internally vs externally.