From a47a59237c816845b5881b7ce883e29702dc0267 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 21 Mar 2023 16:01:47 -0700 Subject: [PATCH] Update ResolvedToolchainContext to take an ImmutableSet. PiperOrigin-RevId: 518407039 Change-Id: Iac17def422b0b6bd493ea97b5c579399edafaeca --- .../analysis/ResolvedToolchainContext.java | 3 ++- .../build/lib/skyframe/AspectFunction.java | 6 ++--- .../skyframe/ConfiguredTargetFunction.java | 9 ++++--- .../ResolvedToolchainContextTest.java | 26 ++++++------------- .../analysis/util/BuildViewForTesting.java | 4 ++- 5 files changed, 22 insertions(+), 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 index da6ecbd0343849..62057241e9d890 100644 --- 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 toolchainTargets) + ImmutableSet toolchainTargets) throws ToolchainException { ImmutableMap.Builder 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 index bc3deab6d5203c..597d3e0afaf366 100644 --- 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 unloadedContext : unloadedToolchainContexts.getContextMap().entrySet()) { - Set toolchainDependencies = - depValueMap.get(DependencyKind.forExecGroup(unloadedContext.getKey())); + ImmutableSet 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 index 6fe2ec1994ece1..834c2ee2dcd10a 100644 --- 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 unloadedContext : prereqs.getUnloadedToolchainContexts().getContextMap().entrySet()) { - Set toolchainDependencies = - prereqs.getDepValueMap().get(DependencyKind.forExecGroup(unloadedContext.getKey())); + ImmutableSet 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 index 47f6f6dba9439e..0b2d0620cce19d 100644 --- 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 index 74e83bdf99a11f..d81dbe4321630e 100644 --- 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); }