Remove some more #getConfiguration calls (and some transitive ones through ConfiguredTargetKey#of(ConfiguredTarget)).

PiperOrigin-RevId: 190663565

5 files changed, 35 insertions, 15 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
index d8d0f6751c..e79d0d0d80 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
@@ -57,6 +57,8 @@ public class ConfiguredTargetAndData {
         target.getLabel());
     BuildConfiguration innerConfiguration = configuredTarget.getConfiguration();
     if (configuration != innerConfiguration) {
+      Preconditions.checkNotNull(configuration, configuredTarget);
+      Preconditions.checkNotNull(innerConfiguration, configuredTarget);
       // We can't always assert that configurations are equal, because fragments, which are used in
       // the equality check, don't implement .equals(), so two configurations constructed from the
       // same options may end up unequal.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
index 50d86008fc..92874215cd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
@@ -49,13 +49,25 @@ public class ConfiguredTargetKey extends ActionLookupKey {
     return of(configuredTarget, configuredTarget.getConfiguration());
   }
 
+  private static Label getLabel(ConfiguredTarget configuredTarget) {
+    AliasProvider aliasProvider = configuredTarget.getProvider(AliasProvider.class);
+    return aliasProvider != null
+        ? aliasProvider.getAliasChain().get(0)
+        : configuredTarget.getLabel();
+  }
+
   public static ConfiguredTargetKey of(
       ConfiguredTarget configuredTarget, BuildConfiguration buildConfiguration) {
-    AliasProvider aliasProvider = configuredTarget.getProvider(AliasProvider.class);
-    Label label =
-        aliasProvider != null ? aliasProvider.getAliasChain().get(0) : configuredTarget.getLabel();
-    return of(label, buildConfiguration);
+    return of(getLabel(configuredTarget), buildConfiguration);
   }
+
+  public static ConfiguredTargetKey of(
+      ConfiguredTarget configuredTarget,
+      BuildConfigurationValue.Key configurationKey,
+      boolean isHostConfiguration) {
+    return of(getLabel(configuredTarget), configurationKey, isHostConfiguration);
+  }
+
   /**
    * Caches so that the number of ConfiguredTargetKey instances is {@code O(configured targets)} and
    * not {@code O(edges between configured targets)}.
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 cd5b827773..2f35accc9f 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
@@ -1355,7 +1355,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    * returned list.
    */
   @ThreadSafety.ThreadSafe
-  public ImmutableMultimap<Dependency, ConfiguredTargetAndData> getConfiguredTargetMapForTesting(
+  private ImmutableMultimap<Dependency, ConfiguredTargetAndData> getConfiguredTargetMapForTesting(
       ExtendedEventHandler eventHandler,
       BuildConfiguration originalConfig,
       Iterable<Dependency> keys) {
@@ -1452,7 +1452,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
                 new ConfiguredTargetAndData(
                     mergedTarget,
                     packageValue.getPackage().getTarget(configuredTarget.getLabel().getName()),
-                    mergedTarget.getConfiguration()));
+                    // This is terrible, but our tests' use of configurations is terrible. It's only
+                    // by accident that getting a null-configuration ConfiguredTarget works even if
+                    // depConfig is not null.
+                    mergedTarget.getConfigurationKey() == null ? null : depConfig));
 
           } catch (DuplicateException | NoSuchTargetException e) {
             throw new IllegalStateException(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
index a62196bc1b..f15e6fbf49 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.auto.value.AutoValue;
-import com.google.common.base.Function;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
@@ -52,13 +51,13 @@ public class TargetCompletionValue implements SkyValue {
       final Set<ConfiguredTarget> targetsToTest) {
     return Iterables.transform(
         targets,
-        new Function<ConfiguredTarget, SkyKey>() {
-          @Override
-          public SkyKey apply(ConfiguredTarget ct) {
-            return TargetCompletionKey.create(
-                ConfiguredTargetKey.of(ct), ctx, targetsToTest.contains(ct));
-          }
-        });
+        ct ->
+            TargetCompletionKey.create(
+                // Can't build top-level targets in host configuration.
+                ConfiguredTargetKey.of(
+                    ct, ct.getConfigurationKey(), /*isHostConfiguration=*/ false),
+                ctx,
+                targetsToTest.contains(ct)));
   }
 
   /** {@link SkyKey} for {@link TargetCompletionValue}. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java
index 20a4dc9e3e..d289ae4e75 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java
@@ -48,7 +48,11 @@ public class TestCompletionValue implements SkyValue {
         targets,
         ct ->
             TestCompletionKey.create(
-                ConfiguredTargetKey.of(ct), topLevelArtifactContext, exclusiveTesting));
+                // Tests are never in host configuration.
+                ConfiguredTargetKey.of(
+                    ct, ct.getConfigurationKey(), /*isHostConfiguration=*/ false),
+                topLevelArtifactContext,
+                exclusiveTesting));
   }
 
   @AutoCodec