Update ResolvedToolchainContext to take an ImmutableSet.

PiperOrigin-RevId: 518407039 Change-Id: Iac17def422b0b6bd493ea97b5c579399edafaeca
fweikert · Mar 21, 2023 · a47a592 · a47a592
1 parent 65b3948
commit a47a592
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 26 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java b/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java
@@ -17,6 +17,7 @@
 import com.google.auto.value.AutoValue;
 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.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
 import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
@@ -46,7 +47,7 @@ public abstract class ResolvedToolchainContext implements ToolchainContext {
   public static ResolvedToolchainContext load(
       UnloadedToolchainContext unloadedToolchainContext,
       String targetDescription,
-      Iterable<ConfiguredTargetAndData> toolchainTargets)
+      ImmutableSet<ConfiguredTargetAndData> toolchainTargets)
       throws ToolchainException {
 
     ImmutableMap.Builder<ToolchainTypeInfo, ToolchainInfo> toolchainsBuilder =

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -94,7 +94,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.StarlarkSemantics;
 
@@ -362,8 +361,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
             ToolchainCollection.builder();
         for (Map.Entry<String, UnloadedToolchainContext> unloadedContext :
             unloadedToolchainContexts.getContextMap().entrySet()) {
-          Set<ConfiguredTargetAndData> toolchainDependencies =
-              depValueMap.get(DependencyKind.forExecGroup(unloadedContext.getKey()));
+          ImmutableSet<ConfiguredTargetAndData> toolchainDependencies =
+              ImmutableSet.copyOf(
+                  depValueMap.get(DependencyKind.forExecGroup(unloadedContext.getKey())));
           contextsBuilder.addContext(
               unloadedContext.getKey(),
               ResolvedToolchainContext.load(

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
@@ -15,6 +15,7 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.GoogleLogger;
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
@@ -65,7 +66,6 @@
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
@@ -230,8 +230,11 @@ public SkyValue compute(SkyKey key, Environment env)
             ToolchainCollection.builder();
         for (Map.Entry<String, UnloadedToolchainContext> unloadedContext :
             prereqs.getUnloadedToolchainContexts().getContextMap().entrySet()) {
-          Set<ConfiguredTargetAndData> toolchainDependencies =
-              prereqs.getDepValueMap().get(DependencyKind.forExecGroup(unloadedContext.getKey()));
+          ImmutableSet<ConfiguredTargetAndData> toolchainDependencies =
+              ImmutableSet.copyOf(
+                  prereqs
+                      .getDepValueMap()
+                      .get(DependencyKind.forExecGroup(unloadedContext.getKey())));
           contextsBuilder.addContext(
               unloadedContext.getKey(),
               ResolvedToolchainContext.load(

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContextTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContextTest.java
@@ -76,10 +76,7 @@ public void load() throws Exception {
 
     // Resolve toolchains.
     ResolvedToolchainContext toolchainContext =
-        ResolvedToolchainContext.load(
-            unloadedToolchainContext,
-            "test",
-            ImmutableList.of(toolchain));
+        ResolvedToolchainContext.load(unloadedToolchainContext, "test", ImmutableSet.of(toolchain));
     assertThat(toolchainContext).isNotNull();
     assertThat(toolchainContext).hasToolchainType(testToolchainTypeLabel);
     assertThat(toolchainContext)
@@ -109,7 +106,7 @@ public void load_mandatory_missing() throws Exception {
     // Resolve toolchains.
     assertThrows(
         ToolchainException.class,
-        () -> ResolvedToolchainContext.load(unloadedToolchainContext, "test", ImmutableList.of()));
+        () -> ResolvedToolchainContext.load(unloadedToolchainContext, "test", ImmutableSet.of()));
   }
 
   @Test
@@ -150,8 +147,7 @@ public void load_optional_present() throws Exception {
 
     // Resolve toolchains.
     ResolvedToolchainContext toolchainContext =
-        ResolvedToolchainContext.load(
-            unloadedToolchainContext, "test", ImmutableList.of(toolchain));
+        ResolvedToolchainContext.load(unloadedToolchainContext, "test", ImmutableSet.of(toolchain));
     assertThat(toolchainContext).isNotNull();
     assertThat(toolchainContext).hasToolchainType(optionalToolchainTypeLabel);
     assertThat(toolchainContext)
@@ -180,7 +176,7 @@ public void load_optional_missing() throws Exception {
 
     // Resolve toolchains.
     ResolvedToolchainContext toolchainContext =
-        ResolvedToolchainContext.load(unloadedToolchainContext, "test", ImmutableList.of());
+        ResolvedToolchainContext.load(unloadedToolchainContext, "test", ImmutableSet.of());
     assertThat(toolchainContext).isNotNull();
 
     // Missing optional toolchain type requirement is present.
@@ -231,7 +227,7 @@ public void load_mixed() throws Exception {
     // Resolve toolchains.
     ResolvedToolchainContext toolchainContext =
         ResolvedToolchainContext.load(
-            unloadedToolchainContext, "test", ImmutableList.of(testToolchain));
+            unloadedToolchainContext, "test", ImmutableSet.of(testToolchain));
     assertThat(toolchainContext).isNotNull();
 
     // Test toolchain is present.
@@ -285,10 +281,7 @@ public void load_aliasedToolchain() throws Exception {
 
     // Resolve toolchains.
     ResolvedToolchainContext toolchainContext =
-        ResolvedToolchainContext.load(
-            unloadedToolchainContext,
-            "test",
-            ImmutableList.of(toolchain));
+        ResolvedToolchainContext.load(unloadedToolchainContext, "test", ImmutableSet.of(toolchain));
     assertThat(toolchainContext).isNotNull();
     assertThat(toolchainContext).hasToolchainType(testToolchainTypeLabel);
     assertThat(toolchainContext)
@@ -331,9 +324,7 @@ public void load_notToolchain() throws Exception {
         ToolchainException.class,
         () ->
             ResolvedToolchainContext.load(
-                unloadedToolchainContext,
-                "test",
-                ImmutableList.of(toolchain)));
+                unloadedToolchainContext, "test", ImmutableSet.of(toolchain)));
   }
 
   @Test
@@ -392,8 +383,7 @@ public void load_withTemplateVariables() throws Exception {
         getConfiguredTargetAndData(
             Label.parseCanonicalUnchecked("//variable:variable_toolchain_impl"), targetConfig);
     ResolvedToolchainContext toolchainContext =
-        ResolvedToolchainContext.load(
-            unloadedToolchainContext, "test", ImmutableList.of(toolchain));
+        ResolvedToolchainContext.load(unloadedToolchainContext, "test", ImmutableSet.of(toolchain));
     assertThat(toolchainContext).isNotNull();
     assertThat(toolchainContext).hasToolchainType(variableToolchainTypeLabel);
     assertThat(toolchainContext.templateVariableProviders()).hasSize(1);

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
@@ -661,7 +661,9 @@ public RuleContext getRuleContextForTesting(
           ResolvedToolchainContext.load(
               unloadedToolchainContext.getValue(),
               targetDescription,
-              prerequisiteMap.get(DependencyKind.forExecGroup(unloadedToolchainContext.getKey())));
+              ImmutableSet.copyOf(
+                  prerequisiteMap.get(
+                      DependencyKind.forExecGroup(unloadedToolchainContext.getKey()))));
       resolvedToolchainContext.addContext(unloadedToolchainContext.getKey(), toolchainContext);
     }