From 9a2d3fe37e96f56c9ef1e6844bded0eb9ffabef9 Mon Sep 17 00:00:00 2001 From: kmb Date: Fri, 17 Nov 2017 08:04:47 -0800 Subject: Support incremental dexing tools in proguarded Android builds RELNOTES: None. PiperOrigin-RevId: 176109497 --- .../build/lib/rules/android/AndroidBinary.java | 364 +++++++++++++++++---- .../lib/rules/android/AndroidConfiguration.java | 24 +- .../lib/rules/android/AndroidRuleClasses.java | 5 + .../build/lib/rules/android/DexArchiveAspect.java | 20 +- 4 files changed, 347 insertions(+), 66 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 b021a8977a..ee4413d277 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 @@ -15,7 +15,9 @@ package com.google.devtools.build.lib.rules.android; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; +import static java.nio.charset.StandardCharsets.ISO_8859_1; import com.google.common.base.Function; import com.google.common.base.Functions; @@ -29,6 +31,7 @@ import com.google.common.collect.Multimap; import com.google.common.collect.MultimapBuilder; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FailAction; +import com.google.devtools.build.lib.actions.ParameterFile; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.OutputGroupProvider; @@ -41,8 +44,9 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg; +import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction; -import com.google.devtools.build.lib.analysis.actions.SpawnAction.Builder; +import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -84,6 +88,8 @@ import javax.annotation.Nullable; */ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { + private static final String DX_MINIMAL_MAIN_DEX_OPTION = "--minimal-main-dex"; + protected abstract JavaSemantics createJavaSemantics(); protected abstract AndroidSemantics createAndroidSemantics(); @@ -471,7 +477,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { Artifact finalProguardMap; if (rexEnabled) { finalDexes = getDxArtifact(ruleContext, "rexed_dexes.zip"); - Builder rexActionBuilder = new SpawnAction.Builder(); + SpawnAction.Builder rexActionBuilder = new SpawnAction.Builder(); CustomCommandLine.Builder commandLine = CustomCommandLine.builder(); rexActionBuilder .useDefaultShellEnvironment() @@ -879,27 +885,39 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { "Both \"main_dex_list\" and \"multidex='manual_main_dex'\" must be specified."); } - // Always OFF if finalJarIsDerived boolean usesDexArchives = getEffectiveIncrementalDexing( ruleContext, dexopts, !Objects.equals(binaryJar, proguardedJar)); Artifact inclusionFilterJar = isBinaryJarFiltered && Objects.equals(binaryJar, proguardedJar) ? binaryJar : null; + Artifact singleJarToDex = !Objects.equals(binaryJar, proguardedJar) ? proguardedJar : null; if (multidexMode == MultidexMode.OFF) { // Single dex mode: generate classes.dex directly from the input jar. if (usesDexArchives) { Artifact classesDex = getDxArtifact(ruleContext, "classes.dex.zip"); - Artifact jarToDex = getDxArtifact(ruleContext, "classes.jar"); - createShuffleJarAction(ruleContext, true, (Artifact) null, ImmutableList.of(jarToDex), - common, inclusionFilterJar, dexopts, androidSemantics, attributes, derivedJarFunction, - (Artifact) null); - createDexMergerAction(ruleContext, "off", jarToDex, classesDex, (Artifact) null, dexopts); + createIncrementalDexingActions( + ruleContext, + singleJarToDex, + common, + inclusionFilterJar, + dexopts, + androidSemantics, + attributes, + derivedJarFunction, + /*multidex=*/ false, + /*mainDexList=*/ null, + classesDex); return new DexingOutput(classesDex, binaryJar, ImmutableList.of(classesDex)); } else { // By *not* writing a zip we get dx to drop resources on the floor. Artifact classesDex = getDxArtifact(ruleContext, "classes.dex"); AndroidCommon.createDexAction( - ruleContext, proguardedJar, classesDex, dexopts, /* multidex */ false, (Artifact) null); + ruleContext, + proguardedJar, + classesDex, + dexopts, + /*multidex=*/ false, + /*mainDexList=*/ null); return new DexingOutput(classesDex, binaryJar, ImmutableList.of(classesDex)); } } else { @@ -914,17 +932,16 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { Artifact classesDex = getDxArtifact(ruleContext, "classes.dex.zip"); if (dexShards > 1) { - ImmutableList.Builder shardsBuilder = ImmutableList.builder(); - for (int i = 1; i <= dexShards; i++) { - shardsBuilder.add(getDxArtifact(ruleContext, "shard" + i + ".jar")); - } - ImmutableList shards = shardsBuilder.build(); + ImmutableList shards = makeShardArtifacts( + ruleContext, + dexShards, + usesDexArchives ? ".jar.dex.zip" : ".jar"); Artifact javaResourceJar = - createShuffleJarAction( + createShuffleJarActions( ruleContext, usesDexArchives, - /*proguardedJar*/ !Objects.equals(binaryJar, proguardedJar) ? proguardedJar : null, + singleJarToDex, shards, common, inclusionFilterJar, @@ -945,12 +962,16 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { // the main dex list becomes one dex file if at all possible. // Note shard here (mostly) contains of .class.dex files from shuffled dex archives, // 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, + createDexMergerAction( + ruleContext, + mainDexList != null && i == 1 ? "minimal" : "best_effort", + ImmutableList.of(shard), + shardDex, + /*mainDexList=*/ null, dexopts); } else { AndroidCommon.createDexAction( - ruleContext, shard, shardDex, dexopts, /* multidex */ true, (Artifact) null); + ruleContext, shard, shardDex, dexopts, /*multidex=*/ true, /*mainDexList=*/ null); } } ImmutableList shardDexes = shardDexesBuilder.build(); @@ -980,11 +1001,18 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { return new DexingOutput(classesDex, javaResourceJar, shardDexes); } else { if (usesDexArchives) { - Artifact jarToDex = AndroidBinary.getDxArtifact(ruleContext, "classes.jar"); - createShuffleJarAction(ruleContext, true, (Artifact) null, ImmutableList.of(jarToDex), - common, inclusionFilterJar, dexopts, androidSemantics, attributes, derivedJarFunction, - (Artifact) null); - createDexMergerAction(ruleContext, "minimal", jarToDex, classesDex, mainDexList, dexopts); + createIncrementalDexingActions( + ruleContext, + singleJarToDex, + common, + inclusionFilterJar, + dexopts, + androidSemantics, + attributes, + derivedJarFunction, + /*multidex=*/ true, + mainDexList, + classesDex); } 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. @@ -994,7 +1022,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { // Have the dexer generate the intermediate file and the "cleaner" action consume this to // generate the final archive with only .dex files. AndroidCommon.createDexAction(ruleContext, proguardedJar, - classesDexIntermediate, dexopts, /* multidex */ true, mainDexList); + classesDexIntermediate, dexopts, /*multidex=*/ true, mainDexList); createCleanDexZipAction(ruleContext, classesDexIntermediate, classesDex); } return new DexingOutput(classesDex, binaryJar, ImmutableList.of(classesDex)); @@ -1002,6 +1030,81 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { } } + /** + * Helper that sets up dexbuilder/dexmerger actions when dex_shards attribute is not set, for use + * with or without multidex. + */ + private static void createIncrementalDexingActions( + RuleContext ruleContext, + @Nullable Artifact proguardedJar, + AndroidCommon common, + @Nullable Artifact inclusionFilterJar, + List dexopts, + AndroidSemantics androidSemantics, + JavaTargetAttributes attributes, + Function derivedJarFunction, + boolean multidex, + @Nullable Artifact mainDexList, + Artifact classesDex) + throws InterruptedException, RuleErrorException { + ImmutableList dexArchives; + if (proguardedJar != null + && AndroidCommon.getAndroidConfig(ruleContext).incrementalDexingShardsAfterProguard() > 1) { + dexArchives = + makeShardArtifacts( + ruleContext, + AndroidCommon.getAndroidConfig(ruleContext).incrementalDexingShardsAfterProguard(), + ".jar.dex.zip"); + } else { + // TODO(b/36527448): Always use sharder, skipping shuffleJar when not using proguard + dexArchives = ImmutableList.of(AndroidBinary.getDxArtifact(ruleContext, "classes.jar")); + } + + if (proguardedJar != null && dexArchives.size() == 1) { + // No need to shuffle, just run proguarded Jar through dexbuilder + DexArchiveAspect.createDexArchiveAction( + ruleContext, + proguardedJar, + DexArchiveAspect.topLevelDexbuilderDexopts(ruleContext, dexopts), + dexArchives.get(0)); + } else { + createShuffleJarActions( + ruleContext, + /*makeDexArchives=*/ true, + proguardedJar, + dexArchives, + common, + inclusionFilterJar, + dexopts, + androidSemantics, + attributes, + derivedJarFunction, + (Artifact) null); + } + + if (dexArchives.size() == 1 || !multidex) { + createDexMergerAction( + ruleContext, multidex ? "minimal" : "off", dexArchives, classesDex, mainDexList, dexopts); + } else { + Artifact shardsToMerge = + createSharderAction(ruleContext, dexArchives, mainDexList, + dexopts.contains(DX_MINIMAL_MAIN_DEX_OPTION)); + Artifact multidexShards = + createTemplatedMergerActions(ruleContext, shardsToMerge, dexopts); + // TODO(b/69431301): avoid this action and give the files to apk build action directly + createZipMergeAction(ruleContext, multidexShards, classesDex); + } + } + + private static ImmutableList makeShardArtifacts( + RuleContext ruleContext, int shardCount, String suffix) { + ImmutableList.Builder shardsBuilder = ImmutableList.builder(); + for (int i = 1; i <= shardCount; i++) { + shardsBuilder.add(getDxArtifact(ruleContext, "shard" + i + suffix)); + } + return shardsBuilder.build(); + } + /** * Returns whether incremental dexing should actually be used based on the --incremental_dexing * flag, the incremental_dexing attribute and the target's dexopts. @@ -1009,43 +1112,49 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { private static boolean getEffectiveIncrementalDexing( RuleContext ruleContext, List dexopts, boolean isBinaryProguarded) { TriState override = ruleContext.attributes().get("incremental_dexing", BuildType.TRISTATE); - // Raise an error if proguard is enabled (b/c incompatible with incremental dexing ATM). - if (isBinaryProguarded && override == TriState.YES) { + AndroidConfiguration config = AndroidCommon.getAndroidConfig(ruleContext); + // Ignore --incremental_dexing if the incremental_dexing attribute is set, but require the + // attribute to be YES for proguarded binaries and binaries with blacklisted dexopts. + if (isBinaryProguarded + && override == TriState.YES + && config.incrementalDexingShardsAfterProguard() <= 0) { ruleContext.attributeError("incremental_dexing", "target cannot be incrementally dexed because it uses Proguard"); return false; } - if (isBinaryProguarded || override == TriState.NO) { - // proguard and the attribute silently ignore the --incremental_dexing flag. + + if (override == TriState.NO) { return false; } - AndroidConfiguration config = AndroidCommon.getAndroidConfig(ruleContext); + if (isBinaryProguarded) { + // Require explicit opt-in for proguarded binaries, but no need to check dexopts since we'll + // be able to create dexbuilder actions with the appropriate flags as part of this rule. + return override == TriState.YES; + } if (override == TriState.YES || config.useIncrementalDexing()) { Iterable blacklistedDexopts = DexArchiveAspect.blacklistedDexopts(ruleContext, dexopts); if (Iterables.isEmpty(blacklistedDexopts)) { // target's dexopts are all compatible with incremental dexing. return true; - } else { + } else if (override == TriState.YES) { // 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) { - Iterable ignored = - Iterables.filter( - blacklistedDexopts, - 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) ? "" : " Ignored dexopts: " + ignored)); - return true; - } else { - // If there are incompatible dexopts and the attribute is not set, we silently don't run - // incremental dexing. - return false; - } + Iterable ignored = + Iterables.filter( + blacklistedDexopts, + 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) ? "" : " Ignored dexopts: " + ignored)); + return true; + } else { + // 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 @@ -1053,10 +1162,120 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { } } + /** + * Sets up a {@code $dexsharder} action for the given {@code dexArchives} and returns the output + * tree artifact. + * + * @return Tree artifact containing dex archives to merge into exactly one .dex file each + */ + private static Artifact createSharderAction( + RuleContext ruleContext, + ImmutableList dexArchives, + @Nullable Artifact mainDexList, + boolean minimalMainDex) { + Artifact outputTree = + ruleContext.getTreeArtifact( + ruleContext.getUniqueDirectory("dexsplits"), ruleContext.getBinOrGenfilesDirectory()); + + SpawnAction.Builder shardAction = + new SpawnAction.Builder() + .useDefaultShellEnvironment() + .setMnemonic("ShardClassesToDex") + .setProgressMessage("Assembling dex files for %s", + ruleContext.getLabel().getCanonicalForm()) + .setExecutable(ruleContext.getExecutablePrerequisite("$dexsharder", Mode.HOST)) + .addInputs(dexArchives) + .addOutput(outputTree); + + CustomCommandLine.Builder shardCommandLine = + CustomCommandLine.builder() + .addExecPaths(VectorArg.addBefore("--input").each(dexArchives)) + .addExecPath("--output", outputTree); + + if (mainDexList != null) { + shardAction.addInput(mainDexList); + shardCommandLine.addExecPath("--main-dex-list", mainDexList); + } + if (minimalMainDex) { + shardCommandLine.add(DX_MINIMAL_MAIN_DEX_OPTION); + } + ruleContext.registerAction( + shardAction.addCommandLine(shardCommandLine.build()).build(ruleContext)); + + return outputTree; + } + + /** + * Sets up a monodex {@code $dexmerger} actions for each dex archive in the given tree artifact + * and returns the output tree artifact. + * + * @return Tree artifact containing zips with final dex files named for inclusion in an APK. + */ + private static Artifact createTemplatedMergerActions( + RuleContext ruleContext, + Artifact inputTree, + Collection dexopts) { + Artifact outputTree = + ruleContext.getTreeArtifact( + ruleContext.getUniqueDirectory("dexfiles"), ruleContext.getBinOrGenfilesDirectory()); + SpawnActionTemplate.Builder dexmerger = + new SpawnActionTemplate.Builder(inputTree, outputTree) + .setExecutable(ruleContext.getExecutablePrerequisite("$dexmerger", Mode.HOST)) + .setMnemonics("DexShardsToMerge", "DexMerger") + .setOutputPathMapper(input -> input.getParentRelativePath()); + CustomCommandLine.Builder commandLine = + CustomCommandLine.builder() + .addPlaceholderTreeArtifactExecPath("--input", inputTree) + .addPlaceholderTreeArtifactExecPath("--output", outputTree) + .add("--multidex=given_shard") + .addAll( + DexArchiveAspect.mergerDexopts( + ruleContext, + Iterables.filter( + dexopts, Predicates.not(Predicates.equalTo(DX_MINIMAL_MAIN_DEX_OPTION))))); + dexmerger.setCommandLineTemplate(commandLine.build()); + ruleContext.registerAction(dexmerger.build(ruleContext.getActionOwner())); + + return outputTree; + } + + private static void createZipMergeAction( + RuleContext ruleContext, Artifact inputTree, Artifact outputZip) { + CustomCommandLine args = CustomCommandLine.builder() + .add("--exclude_build_data") + .add("--dont_change_compression") + .add("--sources") + .addExpandedTreeArtifactExecPaths(inputTree) + .addExecPath("--output", outputZip) + .add("--no_duplicates") // safety: expect distinct entry names in all inputs + .build(); + // Must use params file as otherwise expanding the input tree artifact doesn't work + Artifact paramFile = + ruleContext.getDerivedArtifact( + ParameterFile.derivePath(outputZip.getRootRelativePath()), outputZip.getRoot()); + ruleContext.registerAction( + new ParameterFileWriteAction( + ruleContext.getActionOwner(), + ImmutableList.of(inputTree), + paramFile, + args, + ParameterFile.ParameterFileType.SHELL_QUOTED, + ISO_8859_1)); + ruleContext.registerAction( + singleJarSpawnActionBuilder(ruleContext) + .setMnemonic("MergeDexZips") + .setProgressMessage("Merging dex shards for %s", ruleContext.getLabel()) + .addInput(inputTree) + .addInput(paramFile) + .addOutput(outputZip) + .addCommandLine(CustomCommandLine.builder().addPrefixedExecPath("@", paramFile).build()) + .build(ruleContext)); + } + private static void createDexMergerAction( RuleContext ruleContext, String multidexStrategy, - Artifact inputJar, + ImmutableList dexArchives, Artifact classesDex, @Nullable Artifact mainDexList, Collection dexopts) { @@ -1066,11 +1285,11 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { .setExecutable(ruleContext.getExecutablePrerequisite("$dexmerger", Mode.HOST)) .setMnemonic("DexMerger") .setProgressMessage("Assembling dex files into %s", classesDex.getRootRelativePath()) - .addInput(inputJar) + .addInputs(dexArchives) .addOutput(classesDex); CustomCommandLine.Builder commandLine = CustomCommandLine.builder() - .addExecPath("--input", inputJar) + .addExecPaths(VectorArg.addBefore("--input").each(dexArchives)) .addExecPath("--output", classesDex) .addAll(DexArchiveAspect.mergerDexopts(ruleContext, dexopts)) .addPrefixed("--multidex=", multidexStrategy); @@ -1165,9 +1384,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { return result.build().archivesForDexopts(incrementalDexopts); } - private static Artifact createShuffleJarAction( + private static Artifact createShuffleJarActions( RuleContext ruleContext, - boolean useDexArchives, + boolean makeDexArchives, @Nullable Artifact proguardedJar, ImmutableList shards, AndroidCommon common, @@ -1178,10 +1397,20 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { Function derivedJarFunction, @Nullable Artifact mainDexList) throws InterruptedException, RuleErrorException { + checkArgument(!shards.isEmpty()); checkArgument(mainDexList == null || shards.size() > 1); checkArgument(proguardedJar == null || inclusionFilterJar == null); + Artifact javaResourceJar = ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.JAVA_RESOURCES_JAR); + ImmutableList shuffleOutputs; + if (makeDexArchives && proguardedJar != null) { + checkArgument(shards.size() > 1); + // Split proguardedJar into N shards and run dexbuilder over each one below + shuffleOutputs = makeShardArtifacts(ruleContext, shards.size(), ".jar"); + } else { + shuffleOutputs = shards; + } SpawnAction.Builder shardAction = new SpawnAction.Builder() @@ -1189,12 +1418,12 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { .setMnemonic("ShardClassesToDex") .setProgressMessage("Sharding classes for dexing for %s", ruleContext.getLabel()) .setExecutable(ruleContext.getExecutablePrerequisite("$shuffle_jars", Mode.HOST)) - .addOutputs(shards) + .addOutputs(shuffleOutputs) .addOutput(javaResourceJar); CustomCommandLine.Builder shardCommandLine = CustomCommandLine.builder() - .addExecPaths(VectorArg.addBefore("--output_jar").each(shards)) + .addExecPaths(VectorArg.addBefore("--output_jar").each(shuffleOutputs)) .addExecPath("--output_resources", javaResourceJar); if (mainDexList != null) { @@ -1202,14 +1431,10 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { shardAction.addInput(mainDexList); } - // If we need to run Proguard, all the class files will be in the Proguarded jar and the - // deploy jar will already have been built (since it's the input of Proguard) and it will - // contain all the Java resources. Otherwise, we don't want to have deploy jar creation on - // the critical path, so we put all the jar files that constitute it on the inputs of the - // jar shuffler. + // If we need to run Proguard, all the class files will be in the Proguarded jar, which has to + // be converted to dex. Otherwise we can use the transitive classpath directly and can leverage + // incremental dexing outputs for classpath Jars if applicable. if (proguardedJar != null) { - // When proguard is used we can't use dex archives, so just shuffle the proguarded jar - checkArgument(!useDexArchives, "Dex archives are incompatible with Proguard"); shardCommandLine.addExecPath("--input_jar", proguardedJar); shardAction.addInput(proguardedJar); } else { @@ -1220,7 +1445,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { .build(); // Check whether we can use dex archives. Besides the --incremental_dexing flag, also // make sure the "dexopts" attribute on this target doesn't mention any problematic flags. - if (useDexArchives) { + if (makeDexArchives) { // 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. @@ -1247,7 +1472,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { classpath = dexedClasspath.build(); shardCommandLine.add("--split_dexed_classes"); } else { - classpath = classpath.stream().map(derivedJarFunction::apply).collect(toImmutableList()); + classpath = classpath.stream().map(derivedJarFunction).collect(toImmutableList()); } shardCommandLine.addExecPaths(VectorArg.addBefore("--input_jar").each(classpath)); shardAction.addInputs(classpath); @@ -1260,6 +1485,17 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { shardAction.addCommandLine(shardCommandLine.build()); ruleContext.registerAction(shardAction.build(ruleContext)); + + if (makeDexArchives && proguardedJar != null) { + for (int i = 0; i < shards.size(); ++i) { + checkState(shuffleOutputs.get(i) != shards.get(i)); + DexArchiveAspect.createDexArchiveAction( + ruleContext, + shuffleOutputs.get(i), + DexArchiveAspect.topLevelDexbuilderDexopts(ruleContext, dexopts), + shards.get(i)); + } + } return javaResourceJar; } @@ -1363,7 +1599,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { // Create the main dex classes list. Artifact mainDexList = AndroidBinary.getDxArtifact(ruleContext, "main_dex_list.txt"); - Builder builder = new Builder() + SpawnAction.Builder builder = new SpawnAction.Builder() .setMnemonic("MainDexClasses") .setProgressMessage("Generating main dex classes list"); 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 52269bb5a8..8006e99ff9 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 @@ -371,6 +371,16 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { ) public boolean incrementalDexing; + @Option( + name = "experimental_incremental_dexing_after_proguard", + defaultValue = "0", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.LOSES_INCREMENTAL_STATE}, + help = "Whether to use incremental dexing tools when building proguarded Android binaries. " + + "Values > 0 turn the feature on, values > 1 run that many dexbuilder shards." + ) + public int incrementalDexingShardsAfterProguard; + /** 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( @@ -433,7 +443,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { "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 (superseding --dexopts_supported_in_incremental_dexing). " - + "Defaults to --no-locals for safety but can in general be used " + + "Defaults to --positions 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." ) @@ -693,6 +703,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { host.desugarJava8 = desugarJava8; host.checkDesugarDeps = checkDesugarDeps; host.incrementalDexing = incrementalDexing; + host.incrementalDexingShardsAfterProguard = incrementalDexingShardsAfterProguard; host.incrementalDexingForLiteProtos = incrementalDexingForLiteProtos; host.incrementalDexingErrorOnMissedJars = incrementalDexingErrorOnMissedJars; host.assumeMinSdkVersion = assumeMinSdkVersion; @@ -738,6 +749,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final boolean incrementalNativeLibs; private final ConfigurationDistinguisher configurationDistinguisher; private final boolean incrementalDexing; + private final int incrementalDexingShardsAfterProguard; private final boolean incrementalDexingForLiteProtos; private final boolean incrementalDexingErrorOnMissedJars; private final boolean assumeMinSdkVersion; @@ -774,6 +786,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.cpu = options.cpu; this.configurationDistinguisher = options.configurationDistinguisher; this.incrementalDexing = options.incrementalDexing; + this.incrementalDexingShardsAfterProguard = options.incrementalDexingShardsAfterProguard; this.incrementalDexingForLiteProtos = options.incrementalDexingForLiteProtos; this.incrementalDexingErrorOnMissedJars = options.incrementalDexingErrorOnMissedJars; this.assumeMinSdkVersion = options.assumeMinSdkVersion; @@ -811,6 +824,10 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { throw new InvalidConfigurationException("--dexopts_supported_in_incremental_dexing must " + "include '--no-locals' to enable coverage builds"); } + if (incrementalDexingShardsAfterProguard < 0) { + throw new InvalidConfigurationException( + "--experimental_incremental_dexing_after_proguard must be a positive number"); + } } public String getCpu() { @@ -833,6 +850,11 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { return incrementalDexing; } + /** Returns whether to process proguarded Android binaries with incremental dexing tools. */ + public int incrementalDexingShardsAfterProguard() { + return incrementalDexingShardsAfterProguard; + } + /** * Returns whether to look for Jars produced by {@code JavaLiteProtoAspect}. */ 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 22e0385f7b..977cdc39bd 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 @@ -693,6 +693,11 @@ public final class AndroidRuleClasses { .cfg(HOST) .exec() .value(env.getToolsLabel("//tools/android:dexbuilder"))) + .add( + attr("$dexsharder", LABEL) + .cfg(HOST) + .exec() + .value(env.getToolsLabel("//tools/android:dexsharder"))) .add( attr("$dexmerger", LABEL) .cfg(HOST) 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 9dd84bb0f6..90c2e69336 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 @@ -111,6 +111,9 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu ImmutableList.builder().addAll(TRANSITIVE_ATTRIBUTES_EXCEPT_FOR_PROTOS) // To get from proto_library through proto_lang_toolchain rule to proto runtime library. .add(JavaLiteProtoAspect.PROTO_TOOLCHAIN_ATTR, "runtime").build(); + private static final FlagMatcher DEXOPTS_SUPPORTED_IN_DEXBUILDER = + new FlagMatcher( + ImmutableList.of("--no-locals", "--no-optimize", "--no-warnings", "--positions")); private final String toolsRepository; @@ -481,7 +484,7 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu static ImmutableSet incrementalDexopts(RuleContext ruleContext, Iterable tokenizedDexopts) { if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { - // TODO(bazel-team): Still needed? No longer done in AndroidCommon.createDexAction + // TODO(b/27382165): Still needed? No longer done in AndroidCommon.createDexAction tokenizedDexopts = Iterables.concat(tokenizedDexopts, ImmutableList.of("--no-locals")); } return normalizeDexopts( @@ -503,6 +506,21 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu getAndroidConfig(ruleContext).getTargetDexoptsThatPreventIncrementalDexing())); } + /** + * Derives options to use in DexBuilder actions from the given context and dx flags, where the + * latter typically come from a {@code dexopts} attribute on a top-level target. This should be + * a superset of {@link #incrementalDexopts}. + */ + static ImmutableSet topLevelDexbuilderDexopts( + RuleContext ruleContext, Iterable tokenizedDexopts) { + if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { + // TODO(b/27382165): Still needed? No longer done in AndroidCommon.createDexAction + tokenizedDexopts = Iterables.concat(tokenizedDexopts, ImmutableList.of("--no-locals")); + } + // We don't need an ordered set but might as well. + return normalizeDexopts(Iterables.filter(tokenizedDexopts, DEXOPTS_SUPPORTED_IN_DEXBUILDER)); + } + /** * 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. -- cgit v1.2.3