Remove --incremental_dexing_binary_types.

It has been set to all for a bit now. It is not in any teams blazerc/tap configs.

RELNOTES: --incremental_dexing_binary_types has been removed. All builds are supported by incremental dexing (modulo proguard and some blacklisted dx flags).
PiperOrigin-RevId: 173931117

3 files changed, 39 insertions, 93 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 1bb27a6e08..38882e3c91 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
@@ -55,7 +55,6 @@ import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.TriState;
 import com.google.devtools.build.lib.rules.android.AndroidBinaryMobileInstall.MobileInstallResourceApks;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
-import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidBinaryType;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.MultidexMode;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
 import com.google.devtools.build.lib.rules.cpp.CppHelper;
@@ -980,14 +979,14 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     }
 
     // Always OFF if finalJarIsDerived
-    ImmutableSet<AndroidBinaryType> incrementalDexing =
+    boolean usesDexArchives =
         getEffectiveIncrementalDexing(
             ruleContext, dexopts, !Objects.equals(binaryJar, proguardedJar));
     Artifact inclusionFilterJar =
         isBinaryJarFiltered && Objects.equals(binaryJar, proguardedJar) ? binaryJar : null;
     if (multidexMode == MultidexMode.OFF) {
       // Single dex mode: generate classes.dex directly from the input jar.
-      if (incrementalDexing.contains(AndroidBinaryType.MONODEX)) {
+      if (usesDexArchives) {
         Artifact classesDex = getDxArtifact(ruleContext, "classes.dex.zip");
         Artifact jarToDex = getDxArtifact(ruleContext, "classes.jar");
         createShuffleJarAction(ruleContext, true, (Artifact) null, ImmutableList.of(jarToDex),
@@ -1023,7 +1022,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         Artifact javaResourceJar =
             createShuffleJarAction(
                 ruleContext,
-                incrementalDexing.contains(AndroidBinaryType.MULTIDEX_SHARDED),
+                usesDexArchives,
                 /*proguardedJar*/ !Objects.equals(binaryJar, proguardedJar) ? proguardedJar : null,
                 shards,
                 common,
@@ -1039,7 +1038,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
           Artifact shard = shards.get(i - 1);
           Artifact shardDex = getDxArtifact(ruleContext, "shard" + i + ".dex.zip");
           shardDexesBuilder.add(shardDex);
-          if (incrementalDexing.contains(AndroidBinaryType.MULTIDEX_SHARDED)) {
+          if (usesDexArchives) {
             // If there's a main dex list then the first shard contains exactly those files.
             // To work with devices that lack native multi-dex support we need to make sure that
             // the main dex list becomes one dex file if at all possible.
@@ -1070,7 +1069,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
                 .addOutput(classesDex)
                 .addCommandLine(mergeCommandLine)
                 .build(ruleContext));
-        if (incrementalDexing.contains(AndroidBinaryType.MULTIDEX_SHARDED)) {
+        if (usesDexArchives) {
           // Using the deploy jar for java resources gives better "bazel mobile-install" performance
           // with incremental dexing b/c bazel can create the "incremental" and "split resource"
           // APKs earlier (b/c these APKs don't depend on code being dexed here).  This is also done
@@ -1079,7 +1078,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         }
         return new DexingOutput(classesDex, javaResourceJar, shardDexes);
       } else {
-        if (incrementalDexing.contains(AndroidBinaryType.MULTIDEX_UNSHARDED)) {
+        if (usesDexArchives) {
           Artifact jarToDex = AndroidBinary.getDxArtifact(ruleContext, "classes.jar");
           createShuffleJarAction(ruleContext, true, (Artifact) null, ImmutableList.of(jarToDex),
               common, inclusionFilterJar, dexopts, androidSemantics, attributes, derivedJarFunction,
@@ -1102,27 +1101,31 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     }
   }
 
-  private static ImmutableSet<AndroidBinaryType> getEffectiveIncrementalDexing(
+  /**
+   * Returns whether incremental dexing should actually be used based on the --incremental_dexing
+   * flag, the incremental_dexing attribute and the target's dexopts.
+   */
+  private static boolean getEffectiveIncrementalDexing(
       RuleContext ruleContext, List<String> dexopts, boolean isBinaryProguarded) {
     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).
+    // Raise an error if proguard is enabled (b/c incompatible with incremental dexing ATM).
     if (isBinaryProguarded && override == TriState.YES) {
       ruleContext.attributeError("incremental_dexing",
           "target cannot be incrementally dexed because it uses Proguard");
-      return ImmutableSet.of();
+      return false;
     }
     if (isBinaryProguarded || override == TriState.NO) {
-      return ImmutableSet.of();
+      // proguard and the attribute silently ignore the --incremental_dexing flag.
+      return false;
     }
-    ImmutableSet<AndroidBinaryType> result =
-        override == TriState.YES
-            ? ImmutableSet.copyOf(AndroidBinaryType.values())
-            : AndroidCommon.getAndroidConfig(ruleContext).getIncrementalDexingBinaries();
-    if (!result.isEmpty()) {
+    AndroidConfiguration config = AndroidCommon.getAndroidConfig(ruleContext);
+    if (override == TriState.YES || config.useIncrementalDexing()) {
       Iterable<String> blacklistedDexopts =
           DexArchiveAspect.blacklistedDexopts(ruleContext, dexopts);
-      if (!Iterables.isEmpty(blacklistedDexopts)) {
+      if (Iterables.isEmpty(blacklistedDexopts)) {
+        // target's dexopts are all compatible with incremental dexing.
+        return true;
+      } else {
         // 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.
@@ -1130,21 +1133,23 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
           Iterable<String> ignored =
               Iterables.filter(
                   blacklistedDexopts,
-                  Predicates.not(
-                      Predicates.in(
-                          AndroidCommon.getAndroidConfig(ruleContext)
-                              .getDexoptsSupportedInIncrementalDexing())));
+                  Predicates.not(Predicates.in(config.getDexoptsSupportedInIncrementalDexing())));
           ruleContext.attributeWarning("incremental_dexing",
               String.format("Using incremental dexing even though dexopts %s indicate this target "
                       + "may be unsuitable for incremental dexing for the moment.%s",
                   blacklistedDexopts,
-                  Iterables.isEmpty(ignored) ? "" : " These will be ignored: " + ignored));
+                  Iterables.isEmpty(ignored) ? "" : " Ignored dexopts: " + ignored));
+          return true;
         } else {
-          result = ImmutableSet.of();
+          // If there are incompatible dexopts and the attribute is not set, we silently don't run
+          // incremental dexing.
+          return false;
         }
       }
+    } else {
+      // attribute is auto and flag is false
+      return false;
     }
-    return result;
   }
 
   private static void createDexMergerAction(
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 be47fe50df..493c98cf2a 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
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.android;
 
 import static com.google.devtools.build.lib.syntax.Type.STRING;
 
-import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -41,16 +40,13 @@ import com.google.devtools.build.lib.rules.cpp.CppOptions.LibcTopLabelConverter;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
-import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Converters;
 import com.google.devtools.common.options.EnumConverter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionMetadataTag;
-import com.google.devtools.common.options.OptionsParsingException;
 import java.util.List;
-import java.util.Set;
 import javax.annotation.Nullable;
 
 /** Configuration fragment for Android rules. */
@@ -82,36 +78,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   }
 
   /**
-   * Converter for a set of {@link AndroidBinaryType}s.
-   */
-  public static final class AndroidBinaryTypesConverter
-      implements Converter<Set<AndroidBinaryType>> {
-
-    private final EnumConverter<AndroidBinaryType> elementConverter =
-        new EnumConverter<AndroidBinaryType>(AndroidBinaryType.class, "Android binary type") {};
-    private final Splitter splitter = Splitter.on(',').omitEmptyStrings().trimResults();
-
-    public AndroidBinaryTypesConverter() {}
-
-    @Override
-    public ImmutableSet<AndroidBinaryType> convert(String input) throws OptionsParsingException {
-      if ("all".equals(input)) {
-        return ImmutableSet.copyOf(AndroidBinaryType.values());
-      }
-      ImmutableSet.Builder<AndroidBinaryType> result = ImmutableSet.builder();
-      for (String opt : splitter.split(input)) {
-        result.add(elementConverter.convert(opt));
-      }
-      return result.build();
-    }
-
-    @Override
-    public String getTypeDescription() {
-      return "comma-separated list of: " + elementConverter.getTypeDescription();
-    }
-  }
-
-  /**
    * Converter for {@link AndroidManifestMerger}
    */
   public static final class AndroidManifestMergerConverter
@@ -150,11 +116,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     }
   }
 
-  /** Types of android binaries as {@link AndroidBinary#dex} distinguishes them. */
-  public enum AndroidBinaryType {
-    MONODEX, MULTIDEX_UNSHARDED, MULTIDEX_SHARDED
-  }
-
   /**
    * Which APK signing method to use with the debug key for rules that build APKs.
    *
@@ -410,21 +371,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     )
     public boolean incrementalDexing;
 
-    // Do not use on the command line.
-    // The idea is that this option lets us gradually turn on incremental dexing for different
-    // binaries.  Users should rely on --noincremental_dexing to turn it off.
-    // TODO(b/31711689): remove this flag from config files and here
-    @Option(
-      name = "incremental_dexing_binary_types",
-      defaultValue = "all",
-      converter = AndroidBinaryTypesConverter.class,
-      implicitRequirements = "--incremental_dexing",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help = "Kinds of binaries to incrementally dex if --incremental_dexing is true."
-    )
-    public Set<AndroidBinaryType> incrementalDexingBinaries;
-
     /** Whether to look for incrementally dex protos built with java_lite_proto_library. */
     // TODO(b/31711689): remove this flag from config files and here
     @Option(
@@ -726,7 +672,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
       host.desugarJava8 = desugarJava8;
       host.checkDesugarDeps = checkDesugarDeps;
       host.incrementalDexing = incrementalDexing;
-      host.incrementalDexingBinaries = incrementalDexingBinaries;
       host.incrementalDexingForLiteProtos = incrementalDexingForLiteProtos;
       host.incrementalDexingErrorOnMissedJars = incrementalDexingErrorOnMissedJars;
       host.assumeMinSdkVersion = assumeMinSdkVersion;
@@ -771,7 +716,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   private final String cpu;
   private final boolean incrementalNativeLibs;
   private final ConfigurationDistinguisher configurationDistinguisher;
-  private final ImmutableSet<AndroidBinaryType> incrementalDexingBinaries;
+  private final boolean incrementalDexing;
   private final boolean incrementalDexingForLiteProtos;
   private final boolean incrementalDexingErrorOnMissedJars;
   private final boolean assumeMinSdkVersion;
@@ -805,11 +750,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     this.incrementalNativeLibs = options.incrementalNativeLibs;
     this.cpu = options.cpu;
     this.configurationDistinguisher = options.configurationDistinguisher;
-    if (options.incrementalDexing) {
-      this.incrementalDexingBinaries = ImmutableSet.copyOf(options.incrementalDexingBinaries);
-    } else {
-      this.incrementalDexingBinaries = ImmutableSet.of();
-    }
+    this.incrementalDexing = options.incrementalDexing;
     this.incrementalDexingForLiteProtos = options.incrementalDexingForLiteProtos;
     this.incrementalDexingErrorOnMissedJars = options.incrementalDexingErrorOnMissedJars;
     this.assumeMinSdkVersion = options.assumeMinSdkVersion;
@@ -861,10 +802,10 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   }
 
   /**
-   * Returns when to use incremental dexing using {@link DexArchiveProvider}.
+   * Returns whether to use incremental dexing.
    */
-  public ImmutableSet<AndroidBinaryType> getIncrementalDexingBinaries() {
-    return incrementalDexingBinaries;
+  public boolean useIncrementalDexing() {
+    return incrementalDexing;
   }
 
   /**
@@ -905,8 +846,8 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   }
 
   /**
-   * Regardless of {@link #getIncrementalDexingBinaries}, incremental dexing must not be used for
-   * binaries that list any of these flags in their {@code dexopts} attribute.
+   * Incremental dexing must not be used for binaries that list any of these flags in their
+   * {@code dexopts} attribute.
    */
   public ImmutableList<String> getTargetDexoptsThatPreventIncrementalDexing() {
     return targetDexoptsThatPreventIncrementalDexing;
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 2970d86d7b..9dd84bb0f6 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
@@ -172,8 +172,8 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu
     TriState incrementalAttr =
         TriState.valueOf(params.getOnlyValueOfAttribute("incremental_dexing"));
     if (incrementalAttr == TriState.NO
-        || (getAndroidConfig(ruleContext).getIncrementalDexingBinaries().isEmpty()
-            && incrementalAttr != TriState.YES)) {
+        || (!getAndroidConfig(ruleContext).useIncrementalDexing()
+            && incrementalAttr == TriState.AUTO)) {
       // Dex archives will never be used, so don't bother setting them up.
       return result.build();
     }