RELNOTES: stop using --no-locals in android coverage builds

PiperOrigin-RevId: 184369667
hsyed · Feb 3, 2018 · a4fc77e · a4fc77e
1 parent 5dee3ea
commit a4fc77e
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 28 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
@@ -1003,7 +1003,7 @@ private static void createIncrementalDexingActions(
         DexArchiveAspect.createDexArchiveAction(
             ruleContext,
             proguardedJar,
-            DexArchiveAspect.topLevelDexbuilderDexopts(ruleContext, dexopts),
+            DexArchiveAspect.topLevelDexbuilderDexopts(dexopts),
             dexArchives.get(0));
       } else {
         createShuffleJarActions(
@@ -1428,7 +1428,7 @@ private static Artifact createShuffleJarActions(
         DexArchiveAspect.createDexArchiveAction(
             ruleContext,
             shuffleOutputs.get(i),
-            DexArchiveAspect.topLevelDexbuilderDexopts(ruleContext, dexopts),
+            DexArchiveAspect.topLevelDexbuilderDexopts(dexopts),
             shards.get(i));
       }
     }

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
@@ -875,12 +875,6 @@ public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
     this.skipParsingAction = options.skipParsingAction;
     this.fixedResourceNeverlinking = options.fixedResourceNeverlinking;
 
-    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");
-    }
     if (incrementalDexingShardsAfterProguard < 0) {
       throw new InvalidConfigurationException(
           "--experimental_incremental_dexing_after_proguard must be a positive number");

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
@@ -18,7 +18,6 @@
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
 import static com.google.devtools.build.lib.packages.BuildType.TRISTATE;
 import static com.google.devtools.build.lib.rules.android.AndroidCommon.getAndroidConfig;
-import static java.util.stream.Collectors.toCollection;
 
 import com.google.common.base.Function;
 import com.google.common.base.Functions;
@@ -71,7 +70,6 @@
 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. */
 public final class DexArchiveAspect extends NativeAspectClass implements ConfiguredAspectFactory {
@@ -474,10 +472,6 @@ private static Set<Set<String>> aspectDexopts(RuleContext ruleContext) {
    */
   static ImmutableSet<String> incrementalDexopts(
       RuleContext ruleContext, Iterable<String> tokenizedDexopts) {
-    if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
-      // TODO(b/27382165): Still needed? No longer done in AndroidCommon.createDexAction
-      tokenizedDexopts = Iterables.concat(tokenizedDexopts, ImmutableList.of("--no-locals"));
-    }
     return normalizeDexopts(
         Iterables.filter(
             tokenizedDexopts,
@@ -501,12 +495,7 @@ static Iterable<String> blacklistedDexopts(RuleContext ruleContext, List<String>
    * latter typically come from a {@code dexopts} attribute on a top-level target. This should be a
    * superset of {@link #incrementalDexopts}.
    */
-  static ImmutableSet<String> topLevelDexbuilderDexopts(
-      RuleContext ruleContext, Iterable<String> tokenizedDexopts) {
-    if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
-      // TODO(b/27382165): Still needed? No longer done in AndroidCommon.createDexAction
-      tokenizedDexopts = Iterables.concat(tokenizedDexopts, ImmutableList.of("--no-locals"));
-    }
+  static ImmutableSet<String> topLevelDexbuilderDexopts(Iterable<String> tokenizedDexopts) {
     // We don't need an ordered set but might as well.
     return normalizeDexopts(Iterables.filter(tokenizedDexopts, DEXOPTS_SUPPORTED_IN_DEXBUILDER));
   }
@@ -526,14 +515,13 @@ static ImmutableSet<String> mergerDexopts(
   }
 
   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.
-    return ImmutableSet.copyOf(
-        Streams.stream(tokenizedDexopts)
-            .map(FlagConverter.DX_TO_DEXBUILDER)
-            .collect(toCollection(TreeSet::new))
-            .iterator());
+    // Sort and use ImmutableSet 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.
+    return Streams.stream(tokenizedDexopts)
+        .map(FlagConverter.DX_TO_DEXBUILDER)
+        .sorted()
+        .collect(ImmutableSet.toImmutableSet()); // collector with dedupe
   }
 
   private static class FlagMatcher implements Predicate<String> {