Add attribute to android binaries to explicitly control incremental dexing.

--
MOS_MIGRATED_REVID=122473346

3 files changed, 47 insertions, 16 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 aa565155a0..b798b4f00b 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
@@ -1079,7 +1079,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       @Nullable Artifact mainDexProguardSpec,
       JavaTargetAttributes attributes)
       throws InterruptedException {
-    boolean finalJarIsDerived = isBinaryJarFiltered || binaryJar != proguardedJar;
+    boolean isFinalJarDerived = isBinaryJarFiltered || binaryJar != proguardedJar;
     List<String> dexopts = ruleContext.getTokenizedStringListAttr("dexopts");
     MultidexMode multidexMode = getMultidexMode(ruleContext);
     if (!supportsMultidexMode(ruleContext, multidexMode)) {
@@ -1111,7 +1111,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
 
     // Always OFF if finalJarIsDerived
     ImmutableSet<AndroidBinaryType> incrementalDexing =
-        getEffectiveIncrementalDexing(ruleContext, dexopts, finalJarIsDerived);
+        getEffectiveIncrementalDexing(ruleContext, dexopts, isFinalJarDerived, isBinaryJarFiltered);
     if (multidexMode == MultidexMode.OFF) {
       // Single dex mode: generate classes.dex directly from the input jar.
       if (incrementalDexing.contains(AndroidBinaryType.MONODEX)) {
@@ -1148,7 +1148,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
             createShuffleJarAction(
                 ruleContext,
                 incrementalDexing.contains(AndroidBinaryType.MULTIDEX_SHARDED),
-                finalJarIsDerived ? proguardedJar : null,
+                isFinalJarDerived ? proguardedJar : null,
                 shards,
                 common,
                 dexopts,
@@ -1219,18 +1219,43 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
   }
 
   private static ImmutableSet<AndroidBinaryType> getEffectiveIncrementalDexing(
-      RuleContext ruleContext, List<String> dexopts, boolean finalJarIsDerived) {
-    if (finalJarIsDerived) {
+      RuleContext ruleContext, List<String> dexopts, boolean isFinalJarDerived,
+      boolean isBinaryJarFiltered) {
+    TriState override = ruleContext.attributes().get("incremental_dexing", BuildType.TRISTATE);
+    // Ignore --incremental_dexing_binary_types if the incremental_dexing attribute is set, but
+    // raise an error if proguard is enabled (b/c incompatible with incremental dexing ATM).
+    if (isFinalJarDerived && override == TriState.YES) {
+      ruleContext.attributeError("incremental_dexing",
+          "target cannot be incrementally dexed because "
+          + (isBinaryJarFiltered ? "it builds a partial APK" : "the target uses Proguard"));
+      return ImmutableSet.of();
+    }
+    if (isFinalJarDerived || override == TriState.NO) {
       return ImmutableSet.of();
     }
     ImmutableSet<AndroidBinaryType> result =
-        AndroidCommon.getAndroidConfig(ruleContext).getIncrementalDexingBinaries();
-    if (!result.isEmpty()
-        && Iterables.any(dexopts,
-            new DexArchiveAspect.FlagMatcher(AndroidCommon
-                .getAndroidConfig(ruleContext)
-                .getTargetDexoptsThatPreventIncrementalDexing()))) {
-      result = ImmutableSet.of();
+        override == TriState.YES
+            ? ImmutableSet.copyOf(AndroidBinaryType.values())
+            : AndroidCommon.getAndroidConfig(ruleContext).getIncrementalDexingBinaries();
+    if (!result.isEmpty()) {
+      Iterable<String> blacklistedDexopts =
+          Iterables.filter(
+              dexopts,
+              new DexArchiveAspect.FlagMatcher(AndroidCommon
+                  .getAndroidConfig(ruleContext)
+                  .getTargetDexoptsThatPreventIncrementalDexing()));
+      if (!Iterables.isEmpty(blacklistedDexopts)) {
+        // target's dexopts include flags blacklisted with --non_incremental_per_target_dexopts. If
+        // incremental_dexing attribute is explicitly set for this target then we'll warn and
+        // incrementally dex anyway.  Otherwise, just don't incrementally dex.
+        if (override == TriState.YES) {
+          ruleContext.attributeWarning("incremental_dexing",
+              "Using incremental dexing even though the following dexopts indicate this target "
+              + "may be unsuitable for incremental dexing for the moment: " + blacklistedDexopts);
+        } else {
+          result = ImmutableSet.of();
+        }
+      }
     }
     return result;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
index 600a68ec3a..99fbebc45c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
@@ -627,6 +627,16 @@ public final class AndroidRuleClasses {
           setting this to more than 1 is not recommended for release binaries.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(attr("dex_shards", INTEGER).value(1))
+          /* <!-- #BLAZE_RULE($android_binary_base).ATTRIBUTE(incremental_dexing) -->
+          Force the target to be built with or without incremental dexing, overriding defaults and
+          --incremental_dexing flag. Users should set this attribute to 0 for release binaries
+          (e.g., to avoid accidental usage of --incremental_dexing), since incremental dexing can
+          produce slightly larger artifacts than dx. It is an error to set this attribute to 1 for
+          android_binary and android_test rules that have proguard enabled, as well as for
+          android_test rules with binary_under_test set. We are working on addressing these
+          shortcomings so please check with us if you run into these limitations.
+          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
+          .add(attr("incremental_dexing", TRISTATE))
           /* <!-- #BLAZE_RULE($android_binary_base).ATTRIBUTE(main_dex_list_opts) -->
           Command line options to pass to the main dex list builder.
           Use this option to affect the classes included in the main dex list.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
index bf2ad53fe3..8efb2cd151 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
@@ -106,10 +106,6 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu
   @Override
   public ConfiguredAspect create(ConfiguredTarget base, RuleContext ruleContext,
       AspectParameters params) throws InterruptedException {
-    if (getAndroidConfig(ruleContext).getIncrementalDexingBinaries().isEmpty()) {
-      // Dex archives will never be used, so don't bother setting them up.
-      return new ConfiguredAspect.Builder(NAME, ruleContext).build();
-    }
     checkState(base.getProvider(DexArchiveProvider.class) == null,
         "dex archive natively generated: %s", ruleContext.getLabel());