Enshrine that all ToolchainContexts in a toolchain collection have th…

…e same target platform so the toolchain collection should be able to directly access it. Along the way, rm an unused method. PiperOrigin-RevId: 333365303
Yannic · Oct 5, 2020 · 7af807b · 7af807b
1 parent 2f34b81
commit 7af807b
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 26 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -1056,6 +1056,7 @@ java_library(
     srcs = ["ToolchainCollection.java"],
     deps = [
         ":toolchain_context",
+        "//src/main/java/com/google/devtools/build/lib/analysis/platform",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//third_party:auto_value",
         "//third_party:guava",

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
@@ -1219,16 +1219,11 @@ public ToolchainCollection<ResolvedToolchainContext> getToolchainContexts() {
   }
 
   public boolean targetPlatformHasConstraint(ConstraintValueInfo constraintValue) {
-    if (toolchainContexts == null
-        || toolchainContexts.getDefaultToolchainContext().targetPlatform() == null) {
+    if (toolchainContexts == null || toolchainContexts.getTargetPlatform() == null) {
       return false;
     }
     // All toolchain contexts should have the same target platform so we access via the default.
-    return toolchainContexts
-        .getDefaultToolchainContext()
-        .targetPlatform()
-        .constraints()
-        .hasConstraintValue(constraintValue);
+    return toolchainContexts.getTargetPlatform().constraints().hasConstraintValue(constraintValue);
   }
 
   public ConstraintSemantics<RuleContext> getConstraintSemantics() {

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java
@@ -18,9 +18,9 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.cmdline.Label;
 import java.util.HashMap;
 import java.util.Map;
@@ -63,6 +63,14 @@ public ImmutableSet<String> getExecGroups() {
     return getContextMap().keySet();
   }
 
+  /**
+   * This is safe because all toolchain context in a toolchain collection should have the same
+   * target platform
+   */
+  public PlatformInfo getTargetPlatform() {
+    return getDefaultToolchainContext().targetPlatform();
+  }
+
   @SuppressWarnings("unchecked")
   public ToolchainCollection<ToolchainContext> asToolchainContexts() {
     return (ToolchainCollection<ToolchainContext>) this;
@@ -73,16 +81,6 @@ public static <T extends ToolchainContext> Builder<T> builder() {
     return new Builder<T>();
   }
 
-  /**
-   * Returns every instance of {@link ToolchainContext} that uses {@code resolvedToolchainLabel} as
-   * a resolved toolchain.
-   */
-  public ImmutableList<T> getContextsForResolvedToolchain(Label resolvedToolchainLabel) {
-    return getContextMap().values().stream()
-        .filter(tc -> tc.resolvedToolchainLabels().contains(resolvedToolchainLabel))
-        .collect(ImmutableList.toImmutableList());
-  }
-
   /** Builder for ToolchainCollection. */
   public static final class Builder<T extends ToolchainContext> {
     // This is not immutable so that we can check for duplicate keys easily.

diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java
@@ -230,14 +230,15 @@ private static ToolchainCollection<ToolchainContext> getToolchainContexts(
 
     ToolchainCollection.Builder<UnloadedToolchainContext> toolchainContexts =
         ToolchainCollection.builder();
+    BuildConfigurationValue.Key configurationKey = BuildConfigurationValue.key(config);
     try {
       for (Map.Entry<String, ExecGroup> group : execGroups.entrySet()) {
         ExecGroup execGroup = group.getValue();
         UnloadedToolchainContext context =
             (UnloadedToolchainContext)
                 walkableGraph.getValue(
                     ToolchainContextKey.key()
-                        .configurationKey(BuildConfigurationValue.key(config))
+                        .configurationKey(configurationKey)
                         .requiredToolchainTypeLabels(execGroup.requiredToolchains())
                         .execConstraintLabels(execGroup.execCompatibleWith())
                         .build());
@@ -250,7 +251,7 @@ private static ToolchainCollection<ToolchainContext> getToolchainContexts(
           (UnloadedToolchainContext)
               walkableGraph.getValue(
                   ToolchainContextKey.key()
-                      .configurationKey(BuildConfigurationValue.key(config))
+                      .configurationKey(configurationKey)
                       .requiredToolchainTypeLabels(requiredToolchains)
                       .execConstraintLabels(execConstraintLabels)
                       .build());

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -291,7 +291,7 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc
               transitivePackagesForPackageRootResolution,
               unloadedToolchainContexts == null
                   ? null
-                  : unloadedToolchainContexts.getDefaultToolchainContext().targetPlatform(),
+                  : unloadedToolchainContexts.getTargetPlatform(),
               transitiveRootCauses);
       if (env.valuesMissing()) {
         return null;

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -331,9 +331,7 @@ protected Map<Label, Target> getTargets(
         getConfigurableAttributeKeysForTesting(
             eventHandler,
             ctgNode,
-            toolchainContexts == null
-                ? null
-                : toolchainContexts.getDefaultToolchainContext().targetPlatform()),
+            toolchainContexts == null ? null : toolchainContexts.getTargetPlatform()),
         toolchainContexts,
         DependencyResolver.shouldUseToolchainTransition(configuration, target),
         ruleClassProvider.getTrimmingTransitionFactory());
@@ -519,19 +517,20 @@ public RuleContext getRuleContextForTesting(
         skyframeExecutor.getSkyFunctionEnvironmentForTesting(eventHandler);
 
     Map<String, ToolchainContextKey> toolchainContextKeys = new HashMap<>();
+    BuildConfigurationValue.Key configurationKey = BuildConfigurationValue.key(targetConfig);
     for (Map.Entry<String, ExecGroup> execGroup : execGroups.entrySet()) {
       toolchainContextKeys.put(
           execGroup.getKey(),
           ToolchainContextKey.key()
-              .configurationKey(BuildConfigurationValue.key(targetConfig))
+              .configurationKey(configurationKey)
               .requiredToolchainTypeLabels(execGroup.getValue().requiredToolchains())
               .build());
     }
     String targetUnloadedToolchainContextKey = "target-unloaded-toolchain-context";
     toolchainContextKeys.put(
         targetUnloadedToolchainContextKey,
         ToolchainContextKey.key()
-            .configurationKey(BuildConfigurationValue.key(targetConfig))
+            .configurationKey(configurationKey)
             .requiredToolchainTypeLabels(requiredToolchains)
             .build());