From 4ba4464cb96fac14ddedbf6044e2c4d2c062e50e Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 24 May 2016 21:58:40 +0000 Subject: Allow honoring select dexopts in incremental dexing -- MOS_MIGRATED_REVID=123149803 --- .../build/lib/rules/android/AndroidBinary.java | 59 +++++----- .../lib/rules/android/AndroidConfiguration.java | 26 ++++- .../lib/rules/android/AndroidRuleClasses.java | 6 +- .../build/lib/rules/android/DexArchiveAspect.java | 122 ++++++++++++++++++--- .../lib/rules/android/DexArchiveProvider.java | 46 +++++--- 5 files changed, 198 insertions(+), 61 deletions(-) (limited to 'src/main/java') 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 ff7128523d..ebe8967386 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 @@ -16,8 +16,10 @@ package com.google.devtools.build.lib.rules.android; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Strings.isNullOrEmpty; +import com.google.common.base.Function; 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; import com.google.common.collect.ImmutableSet; @@ -68,6 +70,7 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; +import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -1087,9 +1090,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { Artifact classesDex = getDxArtifact(ruleContext, "classes.dex.zip"); Artifact jarToDex = getDxArtifact(ruleContext, "classes.jar"); createShuffleJarAction(ruleContext, true, (Artifact) null, ImmutableList.of(jarToDex), - common, attributes, (Artifact) null); - createDexMergerAction(ruleContext, "off", jarToDex, classesDex, (Artifact) null, - /* minimalMainDex */ false); + common, dexopts, attributes, (Artifact) null); + createDexMergerAction(ruleContext, "off", jarToDex, classesDex, (Artifact) null, dexopts); return new DexingOutput(classesDex, binaryJar, ImmutableList.of(classesDex)); } else { // By *not* writing a zip we get dx to drop resources on the floor. @@ -1121,6 +1123,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { isFinalJarDerived ? proguardedJar : null, shards, common, + dexopts, attributes, mainDexList); @@ -1137,7 +1140,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { // instead of being a conventional Jar file with .class files. String multidexStrategy = mainDexList != null && i == 1 ? "minimal" : "best_effort"; createDexMergerAction(ruleContext, multidexStrategy, shard, shardDex, (Artifact) null, - /* minimalMainDex */ false); + dexopts); } else { AndroidCommon.createDexAction( ruleContext, shard, shardDex, dexopts, /* multidex */ true, (Artifact) null); @@ -1168,10 +1171,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { if (incrementalDexing.contains(AndroidBinaryType.MULTIDEX_UNSHARDED)) { Artifact jarToDex = AndroidBinary.getDxArtifact(ruleContext, "classes.jar"); createShuffleJarAction(ruleContext, true, (Artifact) null, ImmutableList.of(jarToDex), - common, attributes, (Artifact) null); - createDexMergerAction(ruleContext, "minimal", jarToDex, classesDex, mainDexList, - // unlike dexopts.contains(), this works even for "--a --b" in one string - Iterables.any(dexopts, FlagMatcher.MINIMAL_MAIN_DEX)); + common, dexopts, attributes, (Artifact) null); + createDexMergerAction(ruleContext, "minimal", jarToDex, classesDex, mainDexList, dexopts); } else { // Because the dexer also places resources into this zip, we also need to create a cleanup // action that removes all non-.dex files before staging for apk building. @@ -1220,9 +1221,18 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { // 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) { + Iterable ignored = + Iterables.filter( + blacklistedDexopts, + Predicates.not( + Predicates.in( + AndroidCommon.getAndroidConfig(ruleContext) + .getDexoptsSupportedInIncrementalDexing()))); 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); + 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)); } else { result = ImmutableSet.of(); } @@ -1237,24 +1247,20 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { Artifact inputJar, Artifact classesDex, @Nullable Artifact mainDexList, - boolean minimalMainDex) { - checkArgument(!minimalMainDex || mainDexList != null, "expected mainDexList"); + Collection dexopts) { SpawnAction.Builder dexmerger = new SpawnAction.Builder() .setExecutable(ruleContext.getExecutablePrerequisite("$dexmerger", Mode.HOST)) .addArgument("--input") .addInputArgument(inputJar) .addArgument("--output") .addOutputArgument(classesDex) + .addArguments(DexArchiveAspect.incrementalDexopts(ruleContext, dexopts)) .addArgument("--multidex=" + multidexStrategy) .setMnemonic("DexMerger") .setProgressMessage("Assembling dex files into " + classesDex.prettyPrint()); - if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { - // Match what we do in AndroidCommon.createDexAction - dexmerger.addArgument("--nolocals"); // TODO(bazel-team): Still needed? See createDexAction - } if (mainDexList != null) { dexmerger.addArgument("--main-dex-list").addInputArgument(mainDexList); - if (minimalMainDex) { + if (dexopts.contains("--minimal-main-dex")) { dexmerger.addArgument("--minimal-main-dex"); } } @@ -1265,12 +1271,14 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { * Returns a {@link DexArchiveProvider} of all transitively generated dex archives as well as * dex archives for the Jars produced by the binary target itself. */ - private static DexArchiveProvider collectDexArchives( - RuleContext ruleContext, AndroidCommon common) { + private static Function collectDexArchives( + RuleContext ruleContext, AndroidCommon common, List dexopts) { DexArchiveProvider.Builder result = new DexArchiveProvider.Builder() // Use providers from all attributes that declare DexArchiveAspect .addTransitiveProviders( ruleContext.getPrerequisites("deps", Mode.TARGET, DexArchiveProvider.class)); + ImmutableSet incrementalDexopts = + DexArchiveAspect.incrementalDexopts(ruleContext, dexopts); for (Artifact jar : common.getJarsProducedForRuntime()) { // Create dex archives next to all Jars produced by AndroidCommon for this rule. We need to // do this (instead of placing dex archives into the _dx subdirectory like DexArchiveAspect @@ -1281,10 +1289,10 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { Artifact dexArchive = ruleContext.getDerivedArtifact( jarPath.replaceName(jarPath.getBaseName() + ".dex.zip"), jar.getRoot()); - DexArchiveAspect.createDexArchiveAction(ruleContext, jar, dexArchive); - result.addDexArchive(dexArchive, jar); + DexArchiveAspect.createDexArchiveAction(ruleContext, jar, dexArchive, incrementalDexopts); + result.addDexArchive(incrementalDexopts, dexArchive, jar); } - return result.build(); + return result.build().archivesForDexopts(incrementalDexopts); } private static Artifact createShuffleJarAction( @@ -1293,6 +1301,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { @Nullable Artifact proguardedJar, List shards, AndroidCommon common, + List dexopts, JavaTargetAttributes attributes, @Nullable Artifact mainDexList) throws InterruptedException { @@ -1335,7 +1344,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { // Use dex archives instead of their corresponding Jars wherever we can. At this point // there should be very few or no Jar files that still end up in shards. The dexing // step below will have to deal with those in addition to merging .dex files together. - classpath = Iterables.transform(classpath, collectDexArchives(ruleContext, common)); + classpath = Iterables + .transform(classpath, collectDexArchives(ruleContext, common, dexopts)); shardCommandLine.add("--split_dexed_classes"); } shardCommandLine.addBeforeEachExecPath("--input_jar", classpath); @@ -1719,9 +1729,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { } private static class FlagMatcher implements Predicate { - static final FlagMatcher MINIMAL_MAIN_DEX = - new FlagMatcher(ImmutableList.of("--minimal-main-dex")); - private final ImmutableList matching; FlagMatcher(ImmutableList matching) { 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 606a35e9a0..3966eee33f 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 @@ -262,11 +262,21 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { 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 " - + "incremental dexing. Defaults to --no-locals for safety but can in general be used " + + "incremental dexing (superseding --dexopts_supported_in_incremental_dexing). " + + "Defaults to --no-locals for safety but can in general be used " + "to make sure the listed dx flags are honored, with additional build latency. " + "Please notify us if you find yourself needing this flag.") public List nonIncrementalPerTargetDexopts; + // 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 = "dexopts_supported_in_incremental_dexing", + converter = Converters.CommaSeparatedOptionListConverter.class, + defaultValue = "", + category = "hidden", + help = "dx flags supported in incremental dexing.") + public List dexoptsSupportedInIncrementalDexing; + @Option(name = "experimental_allow_android_library_deps_without_srcs", defaultValue = "true", category = "undocumented", @@ -345,6 +355,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final boolean useJackForDexing; private final boolean jackSanityChecks; private final ImmutableSet incrementalDexingBinaries; + private final ImmutableList dexoptsSupportedInIncrementalDexing; private final ImmutableList targetDexoptsThatPreventIncrementalDexing; private final boolean allowAndroidLibraryDepsWithoutSrcs; private final boolean useAndroidResourceShrinking; @@ -364,6 +375,8 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { } else { this.incrementalDexingBinaries = options.dexingStrategy.binaryTypes; } + this.dexoptsSupportedInIncrementalDexing = + ImmutableList.copyOf(options.dexoptsSupportedInIncrementalDexing); this.targetDexoptsThatPreventIncrementalDexing = ImmutableList.copyOf(options.nonIncrementalPerTargetDexopts); this.allowAndroidLibraryDepsWithoutSrcs = options.allowAndroidLibraryDepsWithoutSrcs; @@ -418,8 +431,15 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { } /** - * Regardless of {@link #getIncrementalDexing}, incremental dexing must not be used for binaries - * that list any of these flags in their {@code dexopts} attribute. + * dx flags supported in incremental dexing actions. + */ + public ImmutableList getDexoptsSupportedInIncrementalDexing() { + return dexoptsSupportedInIncrementalDexing; + } + + /** + * Regardless of {@link #getIncrementalDexingBinaries}, 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/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index efd3d1a9e1..91606e0531 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 @@ -584,7 +584,7 @@ public final class AndroidRuleClasses { .allowedRuleClasses(ALLOWED_DEPENDENCIES) .allowedFileTypes() .aspect(androidNeverlinkAspect) - .aspect(dexArchiveAspect) + .aspect(dexArchiveAspect, DexArchiveAspect.PARAM_EXTRACTOR) .aspect(jackAspect)) // Proguard rule specifying master list of classes to keep during legacy multidexing. .add(attr("$build_incremental_dexmanifest", LABEL).cfg(HOST).exec() @@ -634,7 +634,9 @@ public final class AndroidRuleClasses { 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. */ - .add(attr("incremental_dexing", TRISTATE)) + .add(attr("incremental_dexing", TRISTATE) + // Read by DexArchiveAspect's attribute extractor + .nonconfigurable("AspectParameters don't support configurations.")) /* 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 8920cfbd90..6d5a36ff13 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 @@ -18,9 +18,17 @@ import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTran import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; +import static com.google.devtools.build.lib.packages.BuildType.TRISTATE; +import static com.google.devtools.build.lib.rules.android.AndroidCommon.getAndroidConfig; import static java.nio.charset.StandardCharsets.ISO_8859_1; +import com.google.common.base.Function; +import com.google.common.base.Joiner; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ParameterFile; import com.google.devtools.build.lib.analysis.ConfiguredAspect; @@ -34,16 +42,38 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.AspectDefinition; import com.google.devtools.build.lib.packages.AspectParameters; +import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.NativeAspectClass; +import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; +import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.java.JavaCommon; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; import com.google.devtools.build.lib.rules.java.JavaRuntimeJarProvider; +import java.util.Set; +import java.util.TreeSet; + /** * Aspect to {@link DexArchiveProvider build .dex Archives} from Jars. */ public final class DexArchiveAspect extends NativeAspectClass implements ConfiguredAspectFactory { public static final String NAME = "DexArchiveAspect"; + /** + * Function that returns a {@link Rule}'s {@code incremental_dexing} attribute for use by this + * aspect. Must be provided when attaching this aspect to a rule. + */ + static final Function PARAM_EXTRACTOR = + new Function() { + @Override + public AspectParameters apply(Rule rule) { + AttributeMap attributes = NonconfigurableAttributeMapper.of(rule); + AspectParameters.Builder result = new AspectParameters.Builder(); + TriState incrementalAttr = attributes.get("incremental_dexing", TRISTATE); + result.addAttribute("incremental_dexing", incrementalAttr.name()); + return result.build(); + } + }; private static final String ASPECT_DEXBUILDER_PREREQ = "$dex_archive_dexbuilder"; private static final ImmutableList TRANSITIVE_ATTRIBUTES = ImmutableList.of("deps", "exports", "runtime_deps"); @@ -71,6 +101,14 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu @Override public ConfiguredAspect create(ConfiguredTarget base, RuleContext ruleContext, AspectParameters params) throws InterruptedException { + TriState incrementalAttr = + TriState.valueOf(params.getOnlyValueOfAttribute("incremental_dexing")); + if (incrementalAttr == TriState.NO + || (getAndroidConfig(ruleContext).getIncrementalDexingBinaries().isEmpty() + && incrementalAttr != TriState.YES)) { + // 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()); @@ -83,9 +121,12 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu DexArchiveProvider.Builder result = createArchiveProviderBuilderFromDeps(ruleContext); JavaRuntimeJarProvider jarProvider = base.getProvider(JavaRuntimeJarProvider.class); if (jarProvider != null) { + Set> aspectDexopts = aspectDexopts(ruleContext); for (Artifact jar : jarProvider.getRuntimeJars()) { - Artifact dexArchive = createDexArchiveAction(ruleContext, jar); - result.addDexArchive(dexArchive, jar); + for (Set incrementalDexopts : aspectDexopts) { + Artifact dexArchive = createDexArchiveAction(ruleContext, jar, incrementalDexopts); + result.addDexArchive(incrementalDexopts, dexArchive, jar); + } } } return new ConfiguredAspect.Builder(NAME, ruleContext) @@ -105,8 +146,13 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu return result; } - private static Artifact createDexArchiveAction(RuleContext ruleContext, Artifact jar) { - Artifact result = AndroidBinary.getDxArtifact(ruleContext, jar.getFilename() + ".dex.zip"); + private static Artifact createDexArchiveAction(RuleContext ruleContext, Artifact jar, + Set incrementalDexopts) { + // Since we're potentially dexing the same jar multiple times with different flags, we need to + // write out unique artifacts for each flag combinations. Here, it is convenient to distinguish + // them by putting the flags that were used for creating the artifacts into their filenames. + String filename = jar.getFilename() + Joiner.on("").join(incrementalDexopts) + ".dex.zip"; + Artifact result = AndroidBinary.getDxArtifact(ruleContext, filename); // Aspect must use attribute name for dexbuilder prereq that's different from the prerequisite // declared on AndroidBinaryBaseRule because the two prereq's can otherwise name-clash when // android_binary targets are built as part of an android_test: building android_test causes @@ -115,7 +161,7 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu // RuleContext.getExecutablePrerequisite would fail with "$dexbuilder produces multiple prereqs" // (note they both resolve to the same artifact but that doesn't seem to prevent the exception // from being thrown). - createDexArchiveAction(ruleContext, ASPECT_DEXBUILDER_PREREQ, jar, result); + createDexArchiveAction(ruleContext, ASPECT_DEXBUILDER_PREREQ, jar, result, incrementalDexopts); return result; } @@ -125,20 +171,22 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu * {@link #getDefinition} does it for {@link DexArchiveAspect} under a different name. */ // Package-private method for use in AndroidBinary - static void createDexArchiveAction(RuleContext ruleContext, Artifact jar, Artifact dexArchive) { - createDexArchiveAction(ruleContext, "$dexbuilder", jar, dexArchive); + static void createDexArchiveAction(RuleContext ruleContext, Artifact jar, Artifact dexArchive, + Set tokenizedDexopts) { + createDexArchiveAction(ruleContext, "$dexbuilder", jar, dexArchive, tokenizedDexopts); } + /** + * Creates a dexbuilder action with the given input, output, and flags. Flags must have been + * filtered and normalized to a set that the dexbuilder tool can understand. + */ private static void createDexArchiveAction(RuleContext ruleContext, String dexbuilderPrereq, - Artifact jar, Artifact dexArchive) { + Artifact jar, Artifact dexArchive, Set incrementalDexopts) { // Write command line arguments into a params file for compatibility with WorkerSpawnStrategy CustomCommandLine.Builder args = new CustomCommandLine.Builder() .addExecPath("--input_jar", jar) - .addExecPath("--output_zip", dexArchive); - if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { - // Match what we do in AndroidCommon.createDexAction - args.add("--nolocals"); // TODO(bazel-team): Still needed? See createDexAction - } + .addExecPath("--output_zip", dexArchive) + .add(incrementalDexopts); Artifact paramFile = ruleContext.getDerivedArtifact( ParameterFile.derivePath(dexArchive.getRootRelativePath()), dexArchive.getRoot()); @@ -157,7 +205,53 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu .addInput(paramFile) .addOutput(dexArchive) .setMnemonic("DexBuilder") - .setProgressMessage("Dexing " + jar.prettyPrint()); + .setProgressMessage( + "Dexing " + jar.prettyPrint() + " with applicable dexopts " + incrementalDexopts); ruleContext.registerAction(dexbuilder.build(ruleContext)); } + + private static Set> aspectDexopts(RuleContext ruleContext) { + return Sets.powerSet( + normalizeDexopts( + ruleContext, + getAndroidConfig(ruleContext).getDexoptsSupportedInIncrementalDexing())); + } + + /** + * Derives options to use in incremental dexing actions from the given context and dx flags, where + * the latter typically come from a {@code dexopts} attribute on a top-level target. This method + * only works reliably if the given dexopts were tokenized, e.g., using + * {@link RuleContext#getTokenizedStringListAttr}. + */ + static ImmutableSet incrementalDexopts(RuleContext ruleContext, + Iterable tokenizedDexopts) { + return normalizeDexopts( + ruleContext, + Iterables.filter( + tokenizedDexopts, + Predicates.in(getAndroidConfig(ruleContext).getDexoptsSupportedInIncrementalDexing()))); + } + + private static ImmutableSet normalizeDexopts( + RuleContext ruleContext, Iterable 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 args = new TreeSet<>(); + if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { + // Match what we do in AndroidCommon.createDexAction + args.add("--nolocals"); // TODO(bazel-team): Still needed? See createDexAction + } + Iterables.addAll(args, Iterables.transform(tokenizedDexopts, FlagConverter.DX_TO_DEXBUILDER)); + return ImmutableSet.copyOf(args); + } + + private enum FlagConverter implements Function { + DX_TO_DEXBUILDER; + + @Override + public String apply(String input) { + return input.replace("--no-", "--no"); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveProvider.java index 6ced3815de..ed75111cff 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveProvider.java @@ -17,13 +17,17 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.base.Function; -import com.google.common.collect.BiMap; -import com.google.common.collect.HashBiMap; +import com.google.common.collect.HashBasedTable; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableTable; +import com.google.common.collect.Table; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import java.util.Set; + import javax.annotation.Nullable; /** @@ -36,7 +40,7 @@ import javax.annotation.Nullable; * available (returns the given Jar otherwise). */ @Immutable -public class DexArchiveProvider implements TransitiveInfoProvider, Function { +public class DexArchiveProvider implements TransitiveInfoProvider { /** * Provider that doesn't provide any dex archives, which is what any neverlink target should use. @@ -50,7 +54,8 @@ public class DexArchiveProvider implements TransitiveInfoProvider, Function dexArchives = HashBiMap.create(); + private final Table, Artifact, Artifact> dexArchives = + HashBasedTable.create(); public Builder() { } @@ -68,17 +73,21 @@ public class DexArchiveProvider implements TransitiveInfoProvider, Function dexopts, Artifact dexArchive, Artifact dexedJar) { checkArgument(dexArchive.getFilename().endsWith(".dex.zip"), "Doesn't look like a dex archive: %s", dexArchive); // Adding this artifact will fail iff dexArchive already appears as the value of another jar. // It's ok and expected to put the same pair multiple times. Note that ImmutableBiMap fails // in that situation, which is why we're not using it here. // It's weird to put a dexedJar that's already in the map with a different value so we fail. - Artifact old = dexArchives.put(checkNotNull(dexedJar, "dexedJar"), dexArchive); + Artifact old = + dexArchives.put( + ImmutableSet.copyOf(dexopts), checkNotNull(dexedJar, "dexedJar"), dexArchive); checkArgument(old == null || old.equals(dexedJar), - "We already had mapping %s-%s, so we don't also need %s", dexedJar, old, dexArchive); + "We already had mapping %s-%s for dexopts %s, so we don't also need %s", + dexedJar, old, dexopts, dexArchive); return this; } @@ -86,23 +95,28 @@ public class DexArchiveProvider implements TransitiveInfoProvider, Function dexArchives; + private final ImmutableTable, Artifact, Artifact> dexArchives; - private DexArchiveProvider(ImmutableMap dexArchives) { + private DexArchiveProvider(ImmutableTable, Artifact, Artifact> dexArchives) { this.dexArchives = dexArchives; } - /** Maps Jars to available dex archives and returns the given Jar otherwise. */ - @Override - @Nullable - public Artifact apply(@Nullable Artifact jar) { - Artifact dexArchive = dexArchives.get(jar); - return dexArchive != null ? dexArchive : jar; // return null iff input == null + public Function archivesForDexopts(ImmutableSet dexopts) { + final ImmutableMap dexArchivesForDexopts = dexArchives.row(dexopts); + return new Function() { + /** Maps Jars to available dex archives and returns the given Jar otherwise. */ + @Override + @Nullable + public Artifact apply(@Nullable Artifact jar) { + Artifact dexArchive = dexArchivesForDexopts.get(jar); + return dexArchive != null ? dexArchive : jar; // return null iff input == null + } + }; } @Override -- cgit v1.2.3