diff options
Diffstat (limited to 'src/main/java')
3 files changed, 87 insertions, 44 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 65c1669fe1..655d546568 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 @@ -22,7 +22,6 @@ import static com.google.devtools.build.lib.analysis.OutputGroupProvider.INTERNA import com.google.common.base.Function; import com.google.common.base.Functions; import com.google.common.base.Optional; -import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -1369,11 +1368,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { : AndroidCommon.getAndroidConfig(ruleContext).getIncrementalDexingBinaries(); if (!result.isEmpty()) { Iterable<String> blacklistedDexopts = - Iterables.filter( - dexopts, - new FlagMatcher(AndroidCommon - .getAndroidConfig(ruleContext) - .getTargetDexoptsThatPreventIncrementalDexing())); + DexArchiveAspect.blacklistedDexopts(ruleContext, dexopts); 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 @@ -1412,15 +1407,12 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { .addInputArgument(inputJar) .addArgument("--output") .addOutputArgument(classesDex) - .addArguments(DexArchiveAspect.incrementalDexopts(ruleContext, dexopts)) + .addArguments(DexArchiveAspect.mergerDexopts(ruleContext, dexopts)) .addArgument("--multidex=" + multidexStrategy) .setMnemonic("DexMerger") .setProgressMessage("Assembling dex files into " + classesDex.prettyPrint()); if (mainDexList != null) { dexmerger.addArgument("--main-dex-list").addInputArgument(mainDexList); - if (dexopts.contains("--minimal-main-dex")) { - dexmerger.addArgument("--minimal-main-dex"); - } } ruleContext.registerAction(dexmerger.build(ruleContext)); } @@ -1814,22 +1806,4 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { return ruleContext.getUniqueDirectoryArtifact("_dx", baseName, ruleContext.getBinOrGenfilesDirectory()); } - - private static class FlagMatcher implements Predicate<String> { - private final ImmutableList<String> matching; - - FlagMatcher(ImmutableList<String> matching) { - this.matching = matching; - } - - @Override - public boolean apply(String input) { - for (String match : matching) { - if (input.contains(match)) { - return true; - } - } - return false; - } - } } 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 37d3bc36a7..cf9c618d07 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 @@ -318,9 +318,11 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { help = "Do not use.") public boolean incrementalDexingErrorOnMissedJars; + // Do not use on the command line. + // This flag is intended to be updated as we add supported flags to the incremental dexing tools @Option(name = "non_incremental_per_target_dexopts", converter = Converters.CommaSeparatedOptionListConverter.class, - defaultValue = "--set-max-idx-number,--positions", + defaultValue = "--positions", category = "semantics", help = "dx flags that that prevent incremental dexing for binary targets that list any of " + "the flags listed here in their 'dexopts' attribute, which are ignored with " @@ -336,9 +338,19 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { converter = Converters.CommaSeparatedOptionListConverter.class, defaultValue = "--no-optimize,--no-locals", category = "hidden", - help = "dx flags supported in incremental dexing.") + help = "dx flags supported when converting Jars to dex archives incrementally.") public List<String> dexoptsSupportedInIncrementalDexing; + // Do not use on the command line. + // This flag is intended to be updated as we add supported flags to the incremental dexing tools + // TODO(b/31711689): remove --no-optimize and --no-locals as DexFileMerger no longer needs them + @Option(name = "dexopts_supported_in_dexmerger", + converter = Converters.CommaSeparatedOptionListConverter.class, + defaultValue = "--no-optimize,--no-locals,--minimal-main-dex,--set-max-idx-number", + category = "hidden", + help = "dx flags supported in tool that merges dex archives into final classes.dex files.") + public List<String> dexoptsSupportedInDexMerger; + @Option( name = "experimental_android_rewrite_dexes_with_rex", defaultValue = "false", @@ -455,6 +467,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { host.incrementalDexingErrorOnMissedJars = incrementalDexingErrorOnMissedJars; host.nonIncrementalPerTargetDexopts = nonIncrementalPerTargetDexopts; host.dexoptsSupportedInIncrementalDexing = dexoptsSupportedInIncrementalDexing; + host.dexoptsSupportedInDexMerger = dexoptsSupportedInDexMerger; host.manifestMerger = manifestMerger; return host; } @@ -502,6 +515,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final boolean incrementalDexingErrorOnMissedJars; private final ImmutableList<String> dexoptsSupportedInIncrementalDexing; private final ImmutableList<String> targetDexoptsThatPreventIncrementalDexing; + private final ImmutableList<String> dexoptsSupportedInDexMerger; private final boolean desugarJava8; private final boolean useRexToCompressDexFiles; private final boolean allowAndroidLibraryDepsWithoutSrcs; @@ -515,7 +529,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final boolean useSingleJarForProguardLibraryJars; private final boolean compressJavaResources; - AndroidConfiguration(Options options, Label androidSdk) { + AndroidConfiguration(Options options, Label androidSdk) throws InvalidConfigurationException { this.sdk = androidSdk; this.incrementalNativeLibs = options.incrementalNativeLibs; this.cpu = options.cpu; @@ -531,6 +545,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { ImmutableList.copyOf(options.dexoptsSupportedInIncrementalDexing); this.targetDexoptsThatPreventIncrementalDexing = ImmutableList.copyOf(options.nonIncrementalPerTargetDexopts); + this.dexoptsSupportedInDexMerger = ImmutableList.copyOf(options.dexoptsSupportedInDexMerger); this.desugarJava8 = options.desugarJava8; this.allowAndroidLibraryDepsWithoutSrcs = options.allowAndroidLibraryDepsWithoutSrcs; this.useAndroidResourceShrinking = options.useAndroidResourceShrinking @@ -544,6 +559,12 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.useRexToCompressDexFiles = options.useRexToCompressDexFiles; this.resourceFilter = options.resourceFilter; this.compressJavaResources = options.compressJavaResources; + + if (!dexoptsSupportedInIncrementalDexing.contains("--no-locals")) { + // TODO(bazel-team): Still needed? See DexArchiveAspect + throw new InvalidConfigurationException("--dexopts_supported_in_incremental_dexing must " + + "include '--no-locals' to enable coverage builds"); + } } public String getCpu() { @@ -588,6 +609,13 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { } /** + * dx flags supported in dexmerger actions. + */ + public ImmutableList<String> getDexoptsSupportedInDexMerger() { + return dexoptsSupportedInDexMerger; + } + + /** * Regardless of {@link #getIncrementalDexingBinaries}, incremental dexing must not be used for * binaries that list any of these flags in their {@code dexopts} attribute. */ 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 b3f5cc55d5..91aac4f20f 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 @@ -23,6 +23,7 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1; import com.google.common.base.Function; import com.google.common.base.Functions; import com.google.common.base.Joiner; +import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -60,9 +61,9 @@ import com.google.devtools.build.lib.rules.proto.ProtoLangToolchainProvider; import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; -import java.util.TreeSet; /** * Aspect to {@link DexArchiveProvider build .dex Archives} from Jars. @@ -442,9 +443,7 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu private static Set<Set<String>> aspectDexopts(RuleContext ruleContext) { return Sets.powerSet( - normalizeDexopts( - ruleContext, - getAndroidConfig(ruleContext).getDexoptsSupportedInIncrementalDexing())); + normalizeDexopts(getAndroidConfig(ruleContext).getDexoptsSupportedInIncrementalDexing())); } /** @@ -455,25 +454,67 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu */ static ImmutableSet<String> incrementalDexopts(RuleContext ruleContext, Iterable<String> tokenizedDexopts) { + if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { + // TODO(bazel-team): Still needed? No longer done in AndroidCommon.createDexAction + tokenizedDexopts = Iterables.concat(tokenizedDexopts, ImmutableList.of("--no-locals")); + } return normalizeDexopts( - ruleContext, Iterables.filter( tokenizedDexopts, + // dexopts have to match exactly since aspect only creates archives for listed ones Predicates.in(getAndroidConfig(ruleContext).getDexoptsSupportedInIncrementalDexing()))); } - private static ImmutableSet<String> normalizeDexopts( - RuleContext ruleContext, Iterable<String> tokenizedDexopts) { + /** + * Returns the subset of the given dexopts that are blacklisted from using incremental dexing + * by default. + */ + static Iterable<String> blacklistedDexopts( + RuleContext ruleContext, List<String> dexopts) { + return Iterables.filter( + dexopts, + new FlagMatcher( + getAndroidConfig(ruleContext).getTargetDexoptsThatPreventIncrementalDexing())); + } + + /** + * Derives options to use in DexFileMerger actions from the given context and dx flags, where the + * latter typically come from a {@code dexopts} attribute on a top-level target. + */ + static ImmutableSet<String> mergerDexopts(RuleContext ruleContext, + Iterable<String> tokenizedDexopts) { + // We don't need an ordered set but might as well. Note we don't need to worry about coverage + // builds since the merger doesn't use --no-locals. + return normalizeDexopts( + Iterables.filter( + tokenizedDexopts, + new FlagMatcher(getAndroidConfig(ruleContext).getDexoptsSupportedInDexMerger()))); + } + + private static ImmutableSet<String> normalizeDexopts(Iterable<String> tokenizedDexopts) { // Use TreeSet to drop duplicates and get fixed (sorted) order. Fixed order is important so // we generate one dex archive per set of flag in create() method, regardless of how those flags // are listed in all the top-level targets being built. - Set<String> args = new TreeSet<>(); - if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { - // Match what we do in AndroidCommon.createDexAction - args.add("--nolocals"); // TODO(bazel-team): Still needed? See createDexAction + return ImmutableSet.copyOf( + Sets.newTreeSet(Iterables.transform(tokenizedDexopts, FlagConverter.DX_TO_DEXBUILDER))); + } + + private static class FlagMatcher implements Predicate<String> { + private final ImmutableList<String> matching; + + FlagMatcher(ImmutableList<String> matching) { + this.matching = matching; + } + + @Override + public boolean apply(String input) { + for (String match : matching) { + if (input.contains(match)) { + return true; + } + } + return false; } - Iterables.addAll(args, Iterables.transform(tokenizedDexopts, FlagConverter.DX_TO_DEXBUILDER)); - return ImmutableSet.copyOf(args); } private enum FlagConverter implements Function<String, String> { |