Rollback of commit 270f59b3630ef009f4a3e973581b3fc65a24ecc9.

*** Reason for rollback ***

Broke Android targets with native code when cpu is explicitly set.

*** Original change description ***

RELNOTES: Improve Android split transition handling.

--
MOS_MIGRATED_REVID=127377943

3 files changed, 47 insertions, 25 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 85b2be379f..d8288b9887 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,7 +16,6 @@ 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;
@@ -684,8 +683,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<Optional<String>, ? extends List<? extends TransitiveInfoCollection>> map =
-          getSplitPrerequisites(attributeName);
+      Map<String, ? extends List<? extends TransitiveInfoCollection>> map =
+          getSplitPrerequisites(attributeName, /*requireSplit=*/false);
       return map.isEmpty()
           ? ImmutableList.<TransitiveInfoCollection>of()
           : map.entrySet().iterator().next().getValue();
@@ -696,12 +695,16 @@ public final class RuleContext extends TargetContext
   }
 
   /**
-   * Returns the a prerequisites keyed by the CPU of their configurations.
-   * If no cpu is explicitly specified, an empty Optional is the only key.
-   * This method throws if the attribute cannot split.
+   * Returns the a prerequisites keyed by the CPU of their configurations; this method throws an
+   * exception if the split transition is not active.
    */
-  public Map<Optional<String>, ? extends List<? extends TransitiveInfoCollection>>
+  public Map<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);
@@ -712,10 +715,15 @@ public final class RuleContext extends TargetContext
       // 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.
-      // If --cpu was null, we use an empty Optional in the map.
+      // 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();
-      return ImmutableMap.of(Optional.fromNullable(cpu), targetMap.get(attributeName));
+      if (cpu == null) {
+        Preconditions.checkState(!requireSplit);
+        cpu = "DO_NOT_USE";
+      }
+      return ImmutableMap.of(cpu, targetMap.get(attributeName));
     }
 
     Set<String> cpus = new HashSet<>();
@@ -727,15 +735,15 @@ public final class RuleContext extends TargetContext
     }
 
     // Use an ImmutableListMultimap.Builder here to preserve ordering.
-    ImmutableListMultimap.Builder<Optional<String>, TransitiveInfoCollection> result =
+    ImmutableListMultimap.Builder<String, TransitiveInfoCollection> result =
         ImmutableListMultimap.builder();
     for (TransitiveInfoCollection t : targetMap.get(attributeName)) {
       if (t.getConfiguration() != null) {
-        result.put(Optional.of(t.getConfiguration().getCpu()), t);
+        result.put(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(Optional.of(cpu), t);
+          result.put(cpu, t);
         }
       }
     }
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 f09d09fdbd..d01f525be4 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,21 +150,29 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     // ensure determinism.
     Multimap<String, TransitiveInfoCollection> depsByArchitecture =
         MultimapBuilder.treeKeys().arrayListValues().build();
-    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());
+    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));
     }
     Map<String, BuildConfiguration> configurationMap = new LinkedHashMap<>();
     Map<String, CcToolchainProvider> toolchainMap = new LinkedHashMap<>();
-    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);
+    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));
     }
 
     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 95ea3540d5..eab678210c 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,6 +408,7 @@ 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;
@@ -425,6 +426,7 @@ 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;
@@ -459,6 +461,10 @@ 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.
    */