Fix test configured target creation code that was dropping split deps.

Short story: Dependency -> BuildConfiguration maps can have multiple values
because of split transitions.

This is unfortunately one of those instances where the test logic is forked from
production. So this doesn't actually demonstrate bugs in production code. We
already have tests in ConfigurationsForTargetsTest that directly check the
production logic. So this cl's primary value is to unbreak tests that depend on
the forked logic.

--
PiperOrigin-RevId: 141094830
MOS_MIGRATED_REVID=141094830

2 files changed, 48 insertions, 44 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index b9297a2b68..8f065efa4d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -22,6 +22,7 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
@@ -865,10 +866,10 @@ public class BuildView {
         new LinkedHashMap<>();
     if (!asDeps.isEmpty()) {
       for (BuildConfiguration fromConfig : asDeps.keySet()) {
-        Map<Dependency, BuildConfiguration> trimmedTargets =
+        Multimap<Dependency, BuildConfiguration> trimmedTargets =
             skyframeExecutor.getConfigurations(eventHandler, fromConfig.getOptions(),
                 asDeps.get(fromConfig));
-        for (Map.Entry<Dependency, BuildConfiguration> trimmedTarget : trimmedTargets.entrySet()) {
+        for (Map.Entry<Dependency, BuildConfiguration> trimmedTarget : trimmedTargets.entries()) {
           Target target = labelsToTargets.get(trimmedTarget.getKey().getLabel());
           successfullyEvaluatedTargets.put(
               new TargetAndConfiguration(target, fromConfig),
@@ -1054,13 +1055,13 @@ public class BuildView {
     OrderedSetMultimap<Attribute, Dependency> depNodeNames =
         getDirectPrerequisiteDependenciesForTesting(eventHandler, target, configurations);
 
-    ImmutableMap<Dependency, ConfiguredTarget> cts = skyframeExecutor.getConfiguredTargetMap(
+    ImmutableMultimap<Dependency, ConfiguredTarget> cts = skyframeExecutor.getConfiguredTargetMap(
         eventHandler,
         target.getConfiguration(), ImmutableSet.copyOf(depNodeNames.values()), false);
 
     OrderedSetMultimap<Attribute, ConfiguredTarget> result = OrderedSetMultimap.create();
     for (Map.Entry<Attribute, Dependency> entry : depNodeNames.entries()) {
-      result.put(entry.getKey(), cts.get(entry.getValue()));
+      result.putAll(entry.getKey(), cts.get(entry.getValue()));
     }
     return result;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index c9c5f837da..a21158a10f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -24,12 +24,15 @@ import com.google.common.base.Supplier;
 import com.google.common.base.Throwables;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
+import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
 import com.google.common.collect.Range;
 import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
@@ -1180,12 +1183,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    * returned list.
    */
   @ThreadSafety.ThreadSafe
-  public ImmutableMap<Dependency, ConfiguredTarget> getConfiguredTargetMap(
+  public ImmutableMultimap<Dependency, ConfiguredTarget> getConfiguredTargetMap(
       EventHandler eventHandler, BuildConfiguration originalConfig, Iterable<Dependency> keys,
       boolean useOriginalConfig) {
     checkActive();
 
-    Map<Dependency, BuildConfiguration> configs;
+    Multimap<Dependency, BuildConfiguration> configs;
     if (originalConfig != null) {
 
       if (useOriginalConfig) {
@@ -1198,13 +1201,13 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         // other configurations. In that case, we need to preserve the original configuration.
         // TODO(bazel-team); make this unnecessary once split transition logic is properly ported
         // out of configurations.
-        configs = new HashMap<>();
+        configs = ArrayListMultimap.<Dependency, BuildConfiguration>create();
         configs.put(Iterables.getOnlyElement(keys), originalConfig);
       } else {
         configs = getConfigurations(eventHandler, originalConfig.getOptions(), keys);
       }
     } else {
-      configs = new HashMap<>();
+      configs = ArrayListMultimap.<Dependency, BuildConfiguration>create();
       for (Dependency key : keys) {
         configs.put(key, null);
       }
@@ -1217,13 +1220,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         // it couldn't be loaded). Exclude it from the results.
         continue;
       }
-      skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), configs.get(key)));
-      for (AspectDescriptor aspectDescriptor : key.getAspects()) {
-        skyKeys.add(
-            ActionLookupValue.key(AspectValue.createAspectKey(
-                key.getLabel(), configs.get(key),
-                aspectDescriptor, configs.get(key)
-            )));
+      for (BuildConfiguration depConfig : configs.get(key)) {
+        skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), depConfig));
+        for (AspectDescriptor aspectDescriptor : key.getAspects()) {
+          skyKeys.add(ActionLookupValue.key(AspectValue.createAspectKey(key.getLabel(), depConfig,
+              aspectDescriptor, depConfig)));
+        }
       }
     }
 
@@ -1232,7 +1234,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       reportCycles(eventHandler, entry.getValue().getCycleInfo(), entry.getKey());
     }
 
-    ImmutableMap.Builder<Dependency, ConfiguredTarget> cts = ImmutableMap.builder();
+    ImmutableMultimap.Builder<Dependency, ConfiguredTarget> cts =
+        ImmutableMultimap.<Dependency, ConfiguredTarget>builder();
 
   DependentNodeLoop:
     for (Dependency key : keys) {
@@ -1241,35 +1244,34 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         // it couldn't be loaded). Exclude it from the results.
         continue;
       }
-      SkyKey configuredTargetKey = ConfiguredTargetValue.key(
-          key.getLabel(), configs.get(key));
-      if (result.get(configuredTargetKey) == null) {
-        continue;
-      }
-
-      ConfiguredTarget configuredTarget =
-          ((ConfiguredTargetValue) result.get(configuredTargetKey)).getConfiguredTarget();
-      List<ConfiguredAspect> configuredAspects = new ArrayList<>();
-
-      for (AspectDescriptor aspectDescriptor : key.getAspects()) {
-        SkyKey aspectKey =
-            ActionLookupValue.key(AspectValue.createAspectKey(
-                key.getLabel(), configs.get(key),
-                aspectDescriptor, configs.get(key)
-            ));
-        if (result.get(aspectKey) == null) {
-          continue DependentNodeLoop;
+      for (BuildConfiguration depConfig : configs.get(key)) {
+        SkyKey configuredTargetKey = ConfiguredTargetValue.key(
+            key.getLabel(), depConfig);
+        if (result.get(configuredTargetKey) == null) {
+          continue;
         }
 
-        configuredAspects.add(((AspectValue) result.get(aspectKey)).getConfiguredAspect());
-      }
+        ConfiguredTarget configuredTarget =
+            ((ConfiguredTargetValue) result.get(configuredTargetKey)).getConfiguredTarget();
+        List<ConfiguredAspect> configuredAspects = new ArrayList<>();
 
-      try {
-        cts.put(key, MergedConfiguredTarget.of(configuredTarget, configuredAspects));
-      } catch (DuplicateException e) {
-        throw new IllegalStateException(
-            String.format("Error creating %s", configuredTarget.getTarget().getLabel()),
-            e);
+        for (AspectDescriptor aspectDescriptor : key.getAspects()) {
+          SkyKey aspectKey = ActionLookupValue.key(AspectValue.createAspectKey(key.getLabel(),
+              depConfig, aspectDescriptor, depConfig));
+          if (result.get(aspectKey) == null) {
+            continue DependentNodeLoop;
+          }
+
+          configuredAspects.add(((AspectValue) result.get(aspectKey)).getConfiguredAspect());
+        }
+
+        try {
+          cts.put(key, MergedConfiguredTarget.of(configuredTarget, configuredAspects));
+        } catch (DuplicateException e) {
+          throw new IllegalStateException(
+              String.format("Error creating %s", configuredTarget.getTarget().getLabel()),
+              e);
+        }
       }
     }
 
@@ -1283,9 +1285,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    *
    * <p>Skips targets with loading phase errors.
    */
-  public Map<Dependency, BuildConfiguration> getConfigurations(EventHandler eventHandler,
+  public Multimap<Dependency, BuildConfiguration> getConfigurations(EventHandler eventHandler,
       BuildOptions fromOptions, Iterable<Dependency> keys) {
-    Map<Dependency, BuildConfiguration> builder = new HashMap<>();
+    Multimap<Dependency, BuildConfiguration> builder =
+        ArrayListMultimap.<Dependency, BuildConfiguration>create();
     Set<Dependency> depsToEvaluate = new HashSet<>();
 
     // Check: if !Configuration.useDynamicConfigs then just return the original configs.