From 3d5faa4eca6b1e47e08b5b4db26a524b85ab8e13 Mon Sep 17 00:00:00 2001 From: ajmichael Date: Mon, 30 Oct 2017 15:22:32 -0400 Subject: 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 --- .../build/lib/rules/android/AndroidBinary.java | 55 ++++++++-------- .../lib/rules/android/AndroidConfiguration.java | 73 +++------------------- .../build/lib/rules/android/DexArchiveAspect.java | 4 +- 3 files changed, 39 insertions(+), 93 deletions(-) (limited to 'src/main') 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 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 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 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 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 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 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. */ @@ -81,36 +77,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { } } - /** - * Converter for a set of {@link AndroidBinaryType}s. - */ - public static final class AndroidBinaryTypesConverter - implements Converter> { - - private final EnumConverter elementConverter = - new EnumConverter(AndroidBinaryType.class, "Android binary type") {}; - private final Splitter splitter = Splitter.on(',').omitEmptyStrings().trimResults(); - - public AndroidBinaryTypesConverter() {} - - @Override - public ImmutableSet convert(String input) throws OptionsParsingException { - if ("all".equals(input)) { - return ImmutableSet.copyOf(AndroidBinaryType.values()); - } - ImmutableSet.Builder 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} */ @@ -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 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 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 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 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(); } -- cgit v1.2.3