Add option to enable resource cycle shrinking.

This will instruct AAPT2 to produce conditional keep rules to allow for more aggressive code and resource shrinking.

RELNOTES[NEW]: Add --experimental_android_resource_cycle_shrinking option to allow for more aggressive code and resource shrinking.

PiperOrigin-RevId: 176530749

8 files changed, 160 insertions, 2 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index 1bfcac40af..4d389e94d8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -217,6 +217,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
             configurationMap,
             cppSemantics);
 
+    boolean shrinkResources = shouldShrinkResources(ruleContext);
+
     // TODO(bazel-team): Resolve all the different cases of resource handling so this conditional
     // can go away: recompile from android_resources, and recompile from android_binary attributes.
     ApplicationManifest applicationManifest;
@@ -250,6 +252,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
               ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
               ProguardHelper.getProguardConfigArtifact(ruleContext, ""),
               createMainDexProguardSpec(ruleContext),
+              shouldShrinkResourceCycles(ruleContext, shrinkResources),
               ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
               ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP),
               DataBinding.isEnabled(ruleContext)
@@ -294,8 +297,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
 
     }
 
-    boolean shrinkResources = shouldShrinkResources(ruleContext);
-
     // Remove the library resource JARs from the binary's runtime classpath.
     // Resource classes from android_library dependencies are replaced by the binary's resource
     // class. We remove them only at the top level so that resources included by a library that is
@@ -785,6 +786,18 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     return (state == TriState.YES);
   }
 
+  /** Returns {@code true} if resource shrinking should be performed. */
+  private static boolean shouldShrinkResourceCycles(
+      RuleContext ruleContext, boolean shrinkResources) throws RuleErrorException {
+    boolean global =
+        ruleContext.getFragment(AndroidConfiguration.class).useAndroidResourceCycleShrinking();
+    if (global && !shrinkResources) {
+      throw ruleContext.throwWithRuleError(
+          "resource cycle shrinking can only be enabled when resource shrinking is enabled");
+    }
+    return global;
+  }
+
   private static ResourceApk shrinkResources(
       RuleContext ruleContext,
       ResourceApk resourceApk,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
index 41d9f50e02..6b48275d3f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
@@ -524,6 +524,18 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     public boolean useAndroidResourceShrinking;
 
     @Option(
+      name = "experimental_android_resource_cycle_shrinking",
+      defaultValue = "false",
+      category = "semantics",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      help =
+          "Enables more shrinking of code and resources by instructing AAPT2 "
+              + "to emit conditional Proguard keep rules."
+    )
+    public boolean useAndroidResourceCycleShrinking;
+
+    @Option(
       name = "android_manifest_merger",
       defaultValue = "android",
       category = "semantics",
@@ -727,6 +739,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   private final boolean useRexToCompressDexFiles;
   private final boolean allowAndroidLibraryDepsWithoutSrcs;
   private final boolean useAndroidResourceShrinking;
+  private final boolean useAndroidResourceCycleShrinking;
   private final AndroidManifestMerger manifestMerger;
   private final ApkSigningMethod apkSigningMethod;
   private final boolean useSingleJarApkBuilder;
@@ -763,6 +776,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     this.allowAndroidLibraryDepsWithoutSrcs = options.allowAndroidLibraryDepsWithoutSrcs;
     this.useAndroidResourceShrinking = options.useAndroidResourceShrinking
         || options.useExperimentalAndroidResourceShrinking;
+    this.useAndroidResourceCycleShrinking = options.useAndroidResourceCycleShrinking;
     this.manifestMerger = options.manifestMerger;
     this.apkSigningMethod = options.apkSigningMethod;
     this.useSingleJarApkBuilder = options.useSingleJarApkBuilder;
@@ -884,6 +898,10 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     return useAndroidResourceShrinking;
   }
 
+  public boolean useAndroidResourceCycleShrinking() {
+    return useAndroidResourceCycleShrinking;
+  }
+
   public AndroidAaptVersion getAndroidAaptVersion() {
     return androidAaptVersion;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
index 0adb8b52cd..368fee5d40 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
@@ -77,6 +77,7 @@ public class AndroidResourcesProcessorBuilder {
   private ResourceDependencies dependencies;
   private Artifact proguardOut;
   private Artifact mainDexProguardOut;
+  private boolean conditionalKeepRules;
   private Artifact rTxtOut;
   private Artifact sourceJarOut;
   private boolean debug = false;
@@ -165,6 +166,11 @@ public class AndroidResourcesProcessorBuilder {
     return this;
   }
 
+  public AndroidResourcesProcessorBuilder conditionalKeepRules(boolean conditionalKeepRules) {
+    this.conditionalKeepRules = conditionalKeepRules;
+    return this;
+  }
+
   public AndroidResourcesProcessorBuilder setMainDexProguardOut(Artifact mainDexProguardCfg) {
     this.mainDexProguardOut = mainDexProguardCfg;
     return this;
@@ -301,6 +307,10 @@ public class AndroidResourcesProcessorBuilder {
       builder.add("--useCompiledResourcesForMerge");
     }
 
+    if (conditionalKeepRules) {
+      builder.add("--conditionalKeepRules");
+    }
+
     configureCommonFlags(outs, inputs, builder);
 
     ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
index e378f54fba..70ef13b9aa 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
@@ -585,6 +585,7 @@ public final class ApplicationManifest {
       boolean crunchPng,
       Artifact proguardCfg,
       @Nullable Artifact mainDexProguardCfg,
+      boolean conditionalKeepRules,
       Artifact manifestOut,
       Artifact mergedResources,
       @Nullable Artifact dataBindingInfoZip,
@@ -623,6 +624,11 @@ public final class ApplicationManifest {
     boolean skipParsingAction =
         targetAaptVersion == AndroidAaptVersion.AAPT2 && androidConfiguration.skipParsingAction();
 
+    if (conditionalKeepRules && targetAaptVersion != AndroidAaptVersion.AAPT2) {
+      throw ruleContext.throwWithRuleError(
+          "resource cycle shrinking can only be enabled for builds with aapt2");
+    }
+
     ResourceContainer processed =
         new AndroidResourcesProcessorBuilder(ruleContext)
             .setLibrary(false)
@@ -638,6 +644,7 @@ public final class ApplicationManifest {
             .withDependencies(resourceDeps)
             .setProguardOut(proguardCfg)
             .setMainDexProguardOut(mainDexProguardCfg)
+            .conditionalKeepRules(conditionalKeepRules)
             .setDataBindingInfoZip(dataBindingInfoZip)
             .setApplicationId(manifestValues.get("applicationId"))
             .setVersionCode(manifestValues.get("versionCode"))
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index eebd097d06..8845f56714 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -885,6 +885,44 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
         .isEqualTo(flagValue("--rOutput", processingArgs));
     assertThat(flagValue("--primaryManifest", shrinkingArgs))
         .isEqualTo(flagValue("--manifestOutput", processingArgs));
+
+    List<String> packageArgs =
+        getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "_hello_proguard.cfg"));
+
+    assertThat(flagValue("--tool", packageArgs)).isEqualTo("PACKAGE");
+    assertThat(packageArgs).doesNotContain("--conditionalKeepRules");
+  }
+
+  @Test
+  public void testResourceCycleShrinking() throws Exception {
+    useConfiguration("--experimental_android_resource_cycle_shrinking=true");
+    checkError(
+        "java/a",
+        "a",
+        "resource cycle shrinking can only be enabled for builds with aapt2",
+        "android_binary(",
+        "    name = 'a',",
+        "    srcs = ['A.java'],",
+        "    manifest = 'AndroidManifest.xml',",
+        "    resource_files = [ 'res/values/values.xml' ], ",
+        "    shrink_resources = 1,",
+        ")");
+  }
+
+  @Test
+  public void testResourceCycleShrinkingWithoutResourceShinking() throws Exception {
+    useConfiguration("--experimental_android_resource_cycle_shrinking=true");
+    checkError(
+        "java/a",
+        "a",
+        "resource cycle shrinking can only be enabled when resource shrinking is enabled",
+        "android_binary(",
+        "    name = 'a',",
+        "    srcs = ['A.java'],",
+        "    manifest = 'AndroidManifest.xml',",
+        "    resource_files = [ 'res/values/values.xml' ], ",
+        "    shrink_resources = 0,",
+        ")");
   }
 
   @Test
@@ -3459,6 +3497,58 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
         .isEqualTo(flagValue("--rOutput", processingArgs));
     assertThat(flagValue("--primaryManifest", shrinkingArgs))
         .isEqualTo(flagValue("--manifestOutput", processingArgs));
+
+    List<String> packageArgs =
+        getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "_hello_proguard.cfg"));
+
+    assertThat(flagValue("--tool", packageArgs)).isEqualTo("AAPT2_PACKAGE");
+    assertThat(packageArgs).doesNotContain("--conditionalKeepRules");
+  }
+
+  @Test
+  public void testAapt2ResourceCycleShrinking() throws Exception {
+    mockAndroidSdkWithAapt2();
+    useConfiguration(
+        "--android_sdk=//sdk:sdk", "--experimental_android_resource_cycle_shrinking=true");
+    scratch.file(
+        "java/com/google/android/hello/BUILD",
+        "android_binary(name = 'hello',",
+        "               srcs = ['Foo.java'],",
+        "               manifest = 'AndroidManifest.xml',",
+        "               inline_constants = 0,",
+        "               aapt_version='aapt2',",
+        "               resource_files = ['res/values/strings.xml'],",
+        "               shrink_resources = 1,",
+        "               proguard_specs = ['proguard-spec.pro'],)");
+
+    ConfiguredTarget binary = getConfiguredTarget("//java/com/google/android/hello:hello");
+
+    Set<Artifact> artifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(binary));
+
+    List<String> packageArgs =
+        getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "_hello_proguard.cfg"));
+
+    assertThat(flagValue("--tool", packageArgs)).isEqualTo("AAPT2_PACKAGE");
+    assertThat(packageArgs).contains("--conditionalKeepRules");
+  }
+
+  @Test
+  public void testAapt2ResourceCycleShinkingWithoutResourceShrinking() throws Exception {
+    mockAndroidSdkWithAapt2();
+    useConfiguration(
+        "--android_sdk=//sdk:sdk", "--experimental_android_resource_cycle_shrinking=true");
+    checkError(
+        "java/a",
+        "a",
+        "resource cycle shrinking can only be enabled when resource shrinking is enabled",
+        "android_binary(",
+        "    name = 'a',",
+        "    srcs = ['A.java'],",
+        "    manifest = 'AndroidManifest.xml',",
+        "    resource_files = [ 'res/values/values.xml' ], ",
+        "    shrink_resources = 0,",
+        "    aapt_version = 'aapt2'",
+        ")");
   }
 
   @Test
diff --git a/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java b/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java
index 9bdd1a098f..5739afa377 100644
--- a/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java
+++ b/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java
@@ -29,6 +29,7 @@ import com.google.devtools.build.android.aapt2.ResourceLinker;
 import com.google.devtools.build.android.aapt2.StaticLibrary;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.ShellQuotedParamsFilePreProcessor;
+import com.google.devtools.common.options.TriState;
 import java.io.Closeable;
 import java.io.IOException;
 import java.nio.file.FileSystems;
@@ -186,6 +187,7 @@ public class Aapt2ResourcePackagingAction {
                 .include(compiledResourceDeps)
                 .withAssets(assetDirs)
                 .buildVersion(aaptConfigOptions.buildToolsVersion)
+                .conditionalKeepRules(aaptConfigOptions.conditionalKeepRules == TriState.YES)
                 .filterToDensity(densitiesToFilter)
                 .includeOnlyConfigs(aaptConfigOptions.resourceConfigs)
                 .link(compiled)
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2ConfigOptions.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2ConfigOptions.java
index aa02712957..6d8d614116 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2ConfigOptions.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2ConfigOptions.java
@@ -90,6 +90,16 @@ public class Aapt2ConfigOptions extends OptionsBase {
   public TriState useAaptCruncher;
 
   @Option(
+    name = "conditionalKeepRules",
+    defaultValue = "auto",
+    category = "config",
+    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+    effectTags = {OptionEffectTag.UNKNOWN},
+    help = "Have AAPT2 produce conditional keep rules."
+  )
+  public TriState conditionalKeepRules;
+
+  @Option(
       name = "uncompressedExtensions",
       defaultValue = "",
       converter = CommaSeparatedOptionListConverter.class,
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
index 309fa235ab..2787722c36 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
@@ -63,6 +63,7 @@ public class ResourceLinker {
   private Path baseApk;
   private List<CompiledResources> include = ImmutableList.of();
   private List<Path> assetDirs = ImmutableList.of();
+  private boolean conditionalKeepRules = false;
 
   private ResourceLinker(Path aapt2, Path workingDirectory) {
     this.aapt2 = aapt2;
@@ -101,6 +102,11 @@ public class ResourceLinker {
     return this;
   }
 
+  public ResourceLinker conditionalKeepRules(boolean conditionalKeepRules) {
+    this.conditionalKeepRules = conditionalKeepRules;
+    return this;
+  }
+
   public ResourceLinker baseApkToLinkAgainst(Path baseApk) {
     this.baseApk = baseApk;
     return this;
@@ -243,6 +249,8 @@ public class ResourceLinker {
               .add("--java", javaSourceDirectory)
               .add("--proguard", proguardConfig)
               .add("--proguard-main-dex", mainDexProguard)
+              .when(conditionalKeepRules)
+              .thenAdd("--proguard-conditional-keep-rules")
               .add("-o", outPath)
               .execute(String.format("Linking %s", compiled.getManifest())));
       profiler.recordEndOf("fulllink");