RELNOTES: Improve Android split transition handling.

--
MOS_MIGRATED_REVID=128184440

4 files changed, 34 insertions, 53 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index f4aeb47918..48e7f98d6d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
+import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableBiMap;
@@ -681,8 +682,8 @@ public final class RuleContext extends TargetContext
       // deeply nested and we can't easily inject the behavior we want. However, we should fix all
       // such call sites.
       checkAttribute(attributeName, Mode.SPLIT);
-      Map<String, ? extends List<? extends TransitiveInfoCollection>> map =
-          getSplitPrerequisites(attributeName, /*requireSplit=*/false);
+      Map<Optional<String>, ? extends List<? extends TransitiveInfoCollection>> map =
+          getSplitPrerequisites(attributeName);
       return map.isEmpty()
           ? ImmutableList.<TransitiveInfoCollection>of()
           : map.entrySet().iterator().next().getValue();
@@ -693,35 +694,21 @@ public final class RuleContext extends TargetContext
   }
 
   /**
-   * Returns the a prerequisites keyed by the CPU of their configurations; this method throws an
-   * exception if the split transition is not active.
+   * Returns the a prerequisites keyed by the CPU of their configurations.
+   * If the split transition is not active (e.g. split() returned an empty
+   * list), the key is an empty Optional.
    */
-  public Map<String, ? extends List<? extends TransitiveInfoCollection>>
+  public Map<Optional<String>, ? extends List<? extends TransitiveInfoCollection>>
       getSplitPrerequisites(String attributeName) {
-    return getSplitPrerequisites(attributeName, /*requireSplit*/true);
-  }
-
-  private Map<String, ? extends List<? extends TransitiveInfoCollection>>
-      getSplitPrerequisites(String attributeName, boolean requireSplit) {
     checkAttribute(attributeName, Mode.SPLIT);
 
     Attribute attributeDefinition = getAttribute(attributeName);
     SplitTransition<?> transition = attributeDefinition.getSplitTransition(rule);
     List<BuildConfiguration> configurations =
-        getConfiguration().getTransitions().getSplitConfigurations(transition);
-    if (configurations.size() == 1) {
-      // There are two cases here:
-      // 1. Splitting is enabled, but only one target cpu.
-      // 2. Splitting is disabled, and no --cpu value was provided on the command line.
-      // In the first case, the cpu value is non-null, but in the second case it is null. We only
-      // allow that to proceed if the caller specified that he is going to ignore the cpu value
-      // anyway.
-      String cpu = configurations.get(0).getCpu();
-      if (cpu == null) {
-        Preconditions.checkState(!requireSplit);
-        cpu = "DO_NOT_USE";
-      }
-      return ImmutableMap.of(cpu, targetMap.get(attributeName));
+        getConfiguration().getTransitions().getSplitConfigurationsNoSelf(transition);
+    if (configurations.isEmpty()) {
+      // The split transition is not active. Defer the decision on which CPU to use.
+      return ImmutableMap.of(Optional.<String>absent(), targetMap.get(attributeName));
     }
 
     Set<String> cpus = new HashSet<>();
@@ -733,15 +720,15 @@ public final class RuleContext extends TargetContext
     }
 
     // Use an ImmutableListMultimap.Builder here to preserve ordering.
-    ImmutableListMultimap.Builder<String, TransitiveInfoCollection> result =
+    ImmutableListMultimap.Builder<Optional<String>, TransitiveInfoCollection> result =
         ImmutableListMultimap.builder();
     for (TransitiveInfoCollection t : targetMap.get(attributeName)) {
       if (t.getConfiguration() != null) {
-        result.put(t.getConfiguration().getCpu(), t);
+        result.put(Optional.of(t.getConfiguration().getCpu()), t);
       } else {
         // Source files don't have a configuration, so we add them to all architecture entries.
         for (String cpu : cpus) {
-          result.put(cpu, t);
+          result.put(Optional.of(cpu), t);
         }
       }
     }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
index 3b71ba34f2..3b6c338e71 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
@@ -185,6 +185,14 @@ public final class BuildConfigurationCollection {
       return transitionTable;
     }
 
+    public List<BuildConfiguration> getSplitConfigurationsNoSelf(SplitTransition<?> transition) {
+      if (splitTransitionTable.containsKey(transition)) {
+        return splitTransitionTable.get(transition);
+      } else {
+        return ImmutableList.of();
+      }
+    }
+
     public List<BuildConfiguration> getSplitConfigurations(SplitTransition<?> transition) {
       if (splitTransitionTable.containsKey(transition)) {
         return splitTransitionTable.get(transition);
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 d01f525be4..f09d09fdbd 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
@@ -150,29 +150,21 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     // ensure determinism.
     Multimap<String, TransitiveInfoCollection> depsByArchitecture =
         MultimapBuilder.treeKeys().arrayListValues().build();
-    AndroidConfiguration config = ruleContext.getFragment(AndroidConfiguration.class);
-    if (config.usesAndroidCrosstool()) {
-      for (Map.Entry<String, ? extends List<? extends TransitiveInfoCollection>> entry :
-          ruleContext.getSplitPrerequisites("deps").entrySet()) {
-        depsByArchitecture.putAll(entry.getKey(), entry.getValue());
-      }
-    } else {
-      depsByArchitecture.putAll(
-          config.getCpu(), ruleContext.getPrerequisites("deps", Mode.TARGET));
+    AndroidConfiguration androidConfig = ruleContext.getFragment(AndroidConfiguration.class);
+    for (Map.Entry<Optional<String>, ? extends List<? extends TransitiveInfoCollection>> entry :
+        ruleContext.getSplitPrerequisites("deps").entrySet()) {
+      String cpu = entry.getKey().or(androidConfig.getCpu());
+      depsByArchitecture.putAll(cpu, entry.getValue());
     }
     Map<String, BuildConfiguration> configurationMap = new LinkedHashMap<>();
     Map<String, CcToolchainProvider> toolchainMap = new LinkedHashMap<>();
-    if (config.usesAndroidCrosstool()) {
-      for (Map.Entry<String, ? extends List<? extends TransitiveInfoCollection>> entry :
-          ruleContext.getSplitPrerequisites(":cc_toolchain_split").entrySet()) {
-        TransitiveInfoCollection dep = Iterables.getOnlyElement(entry.getValue());
-        CcToolchainProvider toolchain = CppHelper.getToolchain(ruleContext, dep);
-        configurationMap.put(entry.getKey(), dep.getConfiguration());
-        toolchainMap.put(entry.getKey(), toolchain);
-      }
-    } else {
-      configurationMap.put(config.getCpu(), ruleContext.getConfiguration());
-      toolchainMap.put(config.getCpu(), CppHelper.getToolchain(ruleContext));
+    for (Map.Entry<Optional<String>, ? extends List<? extends TransitiveInfoCollection>> entry :
+        ruleContext.getSplitPrerequisites(":cc_toolchain_split").entrySet()) {
+      String cpu = entry.getKey().or(androidConfig.getCpu());
+      TransitiveInfoCollection dep = Iterables.getOnlyElement(entry.getValue());
+      CcToolchainProvider toolchain = CppHelper.getToolchain(ruleContext, dep);
+      configurationMap.put(cpu, dep.getConfiguration());
+      toolchainMap.put(cpu, toolchain);
     }
 
     NativeLibs nativeLibs = shouldLinkNativeDeps(ruleContext)
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 eab678210c..95ea3540d5 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
@@ -408,7 +408,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   private final boolean legacyNativeSupport;
   private final String cpu;
   private final boolean incrementalNativeLibs;
-  private final boolean usesAndroidCrosstool;
   private final ConfigurationDistinguisher configurationDistinguisher;
   private final boolean useJackForDexing;
   private final boolean jackSanityChecks;
@@ -426,7 +425,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     this.strictDeps = options.strictDeps;
     this.legacyNativeSupport = options.legacyNativeSupport;
     this.cpu = options.cpu;
-    this.usesAndroidCrosstool = (options.androidCrosstoolTop != null);
     this.configurationDistinguisher = options.configurationDistinguisher;
     this.useJackForDexing = options.useJackForDexing;
     this.jackSanityChecks = options.jackSanityChecks;
@@ -461,10 +459,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     return strictDeps;
   }
 
-  public boolean usesAndroidCrosstool() {
-    return usesAndroidCrosstool;
-  }
-
   /**
    * Returns true if Jack should be used in place of javac/dx for Android compilation.
    */