From e6a44f89ec9fb40822fa4588aeb58416075922ee Mon Sep 17 00:00:00 2001 From: jcater Date: Tue, 15 Feb 2022 13:00:13 -0800 Subject: [PATCH] ConfigurationsForTargetsTest should test the exec transition. The host transition is being removed. Unfortunately this requires a lot more setup to call ConfiguredTargetFunction.computeDependencies correctly. PiperOrigin-RevId: 428855026 --- .../skyframe/ToolchainResolutionFunction.java | 3 +- .../UnloadedToolchainContextImpl.java | 9 ++- .../ResolvedToolchainContextTest.java | 12 ++-- .../google/devtools/build/lib/skyframe/BUILD | 1 + .../ConfigurationsForTargetsTest.java | 65 +++++++++++++++---- 5 files changed, 64 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java index ce32bc87b69d11..ca2433aabea126 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java @@ -67,8 +67,7 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env) ToolchainContextKey key = (ToolchainContextKey) skyKey.argument(); try { - UnloadedToolchainContextImpl.Builder builder = - UnloadedToolchainContextImpl.builder().setKey(key); + UnloadedToolchainContextImpl.Builder builder = UnloadedToolchainContextImpl.builder(key); // Determine the configuration being used. BuildConfigurationValue configuration = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/UnloadedToolchainContextImpl.java b/src/main/java/com/google/devtools/build/lib/skyframe/UnloadedToolchainContextImpl.java index ffe84a5dfe161e..56787549bc7b66 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/UnloadedToolchainContextImpl.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/UnloadedToolchainContextImpl.java @@ -30,8 +30,13 @@ @AutoValue public abstract class UnloadedToolchainContextImpl implements SkyValue, UnloadedToolchainContext { - public static Builder builder() { - return new AutoValue_UnloadedToolchainContextImpl.Builder(); + public static Builder builder(ToolchainContextKey key) { + return new AutoValue_UnloadedToolchainContextImpl.Builder() + .setKey(key) + .setRequiredToolchainTypes(ImmutableSet.of()) + .setRequestedLabelToToolchainType(ImmutableMap.of()) + .setToolchainTypeToResolved( + ImmutableSetMultimap.builder().build()); } /** Builder class to help create the {@link UnloadedToolchainContextImpl}. */ 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 0a173cf7be9086..075feccf8f8282 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 @@ -54,8 +54,7 @@ public void load() throws Exception { // Create a static UnloadedToolchainContext. UnloadedToolchainContext unloadedToolchainContext = - UnloadedToolchainContextImpl.builder() - .setKey(toolchainContextKey) + UnloadedToolchainContextImpl.builder(toolchainContextKey) .setExecutionPlatform(linuxPlatform) .setTargetPlatform(linuxPlatform) .setRequiredToolchainTypes(ImmutableSet.of(testToolchainType)) @@ -107,8 +106,7 @@ public void load_aliasedToolchain() throws Exception { // Create a static UnloadedToolchainContext. UnloadedToolchainContext unloadedToolchainContext = - UnloadedToolchainContextImpl.builder() - .setKey(toolchainContextKey) + UnloadedToolchainContextImpl.builder(toolchainContextKey) .setExecutionPlatform(linuxPlatform) .setTargetPlatform(linuxPlatform) .setRequiredToolchainTypes(ImmutableSet.of(testToolchainType)) @@ -150,8 +148,7 @@ public void load_notToolchain() throws Exception { // Create a static UnloadedToolchainContext. UnloadedToolchainContext unloadedToolchainContext = - UnloadedToolchainContextImpl.builder() - .setKey(toolchainContextKey) + UnloadedToolchainContextImpl.builder(toolchainContextKey) .setExecutionPlatform(linuxPlatform) .setTargetPlatform(linuxPlatform) .setRequiredToolchainTypes(ImmutableSet.of(testToolchainType)) @@ -210,8 +207,7 @@ public void load_withTemplateVariables() throws Exception { // Create a static UnloadedToolchainContext. UnloadedToolchainContext unloadedToolchainContext = - UnloadedToolchainContextImpl.builder() - .setKey(toolchainContextKey) + UnloadedToolchainContextImpl.builder(toolchainContextKey) .setExecutionPlatform(linuxPlatform) .setTargetPlatform(linuxPlatform) .setRequiredToolchainTypes(ImmutableSet.of(variableToolchainType)) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD index 4775a7d069d350..072fc4e2e3d335 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD @@ -258,6 +258,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/skyframe:transitive_traversal_value", "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/skyframe:unloaded_toolchain_context", + "//src/main/java/com/google/devtools/build/lib/skyframe:unloaded_toolchain_context_impl", "//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_context_key", "//src/main/java/com/google/devtools/build/lib/skyframe:workspace_info", "//src/main/java/com/google/devtools/build/lib/skyframe:workspace_name_value", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java index 196c0c9734d825..42e63014a2e464 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java @@ -19,6 +19,7 @@ import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Multimap; import com.google.devtools.build.lib.analysis.AliasProvider; @@ -27,12 +28,17 @@ import com.google.devtools.build.lib.analysis.Dependency; import com.google.devtools.build.lib.analysis.DependencyKind; import com.google.devtools.build.lib.analysis.DependencyResolver; +import com.google.devtools.build.lib.analysis.PlatformOptions; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; +import com.google.devtools.build.lib.analysis.ToolchainCollection; +import com.google.devtools.build.lib.analysis.ToolchainContext; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; +import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.RuleClassProvider; @@ -47,7 +53,7 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.util.List; -import org.junit.Ignore; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -75,6 +81,10 @@ @RunWith(JUnit4.class) public class ConfigurationsForTargetsTest extends AnalysisTestCase { + public static final Label TARGET_PLATFORM_LABEL = + Label.parseAbsoluteUnchecked("//platform:target"); + public static final Label EXEC_PLATFORM_LABEL = Label.parseAbsoluteUnchecked("//platform:exec"); + /** * A mock {@link SkyFunction} that just calls {@link ConfiguredTargetFunction#computeDependencies} * and returns its results. @@ -118,6 +128,23 @@ static final class Value implements SkyValue { public SkyValue compute(SkyKey skyKey, Environment env) throws EvalException, InterruptedException { try { + TargetAndConfiguration targetAndConfiguration = (TargetAndConfiguration) skyKey.argument(); + // Set up the toolchain context so that exec transitions resolve properly. + ToolchainCollection toolchainContexts = + ToolchainCollection.builder() + .addDefaultContext( + UnloadedToolchainContextImpl.builder( + ToolchainContextKey.key() + .toolchainTypes(ImmutableSet.of()) + .configurationKey( + targetAndConfiguration.getConfiguration().getKey()) + .build()) + .setTargetPlatform( + PlatformInfo.builder().setLabel(TARGET_PLATFORM_LABEL).build()) + .setExecutionPlatform( + PlatformInfo.builder().setLabel(EXEC_PLATFORM_LABEL).build()) + .build()) + .build(); OrderedSetMultimap depMap = ConfiguredTargetFunction.computeDependencies( new ConfiguredTargetFunction.ComputeDependenciesState(), @@ -125,11 +152,11 @@ public SkyValue compute(SkyKey skyKey, Environment env) /*transitiveRootCauses=*/ NestedSetBuilder.stableOrder(), env, new SkyframeDependencyResolver(env), - (TargetAndConfiguration) skyKey.argument(), + targetAndConfiguration, ImmutableList.of(), ImmutableMap.of(), - /*toolchainContexts=*/ null, - /*useToolchainTransition=*/ false, + toolchainContexts, + /*useToolchainTransition=*/ true, stateProvider.lateBoundRuleClassProvider(), stateProvider.lateBoundHostConfig()); return env.valuesMissing() ? null : new Value(depMap); @@ -243,6 +270,15 @@ protected List getConfiguredDeps(ConfiguredTarget target, Stri String.format("Couldn't find attribute %s for label %s", attrName, targetLabel)); } + @Before + public void setUp() throws Exception { + scratch.file( + "platform/BUILD", + // Add basic target and exec platforms for testing. + "platform(name = 'target')", + "platform(name = 'exec')"); + } + @Test public void nullConfiguredDepsHaveExpectedConfigs() throws Exception { scratch.file( @@ -275,23 +311,24 @@ public void targetDeps() throws Exception { * contexts. */ @Test - @Ignore("b/219749974") - public void hostDeps() throws Exception { + public void execDeps() throws Exception { scratch.file( - "a/host_rule.bzl", - "host_rule = rule(", + "a/exec_rule.bzl", + "exec_rule = rule(", " implementation = lambda ctx: [],", - " attrs = {'tools': attr.label_list(cfg = 'host')},", + " attrs = {'tools': attr.label_list(cfg = 'exec')},", ")"); scratch.file( "a/BUILD", - "load('//a:host_rule.bzl', 'host_rule')", - "cc_binary(name = 'host_tool', srcs = ['host_tool.cc'])", - "host_rule(name = 'gen', tools = [':host_tool'])"); + "load('//a:exec_rule.bzl', 'exec_rule')", + "sh_binary(name = 'exec_tool', srcs = ['exec_tool.sh'])", + "exec_rule(name = 'gen', tools = [':exec_tool'])"); ConfiguredTarget toolDep = Iterables.getOnlyElement(getConfiguredDeps("//a:gen", "tools")); - - assertThat(getConfiguration(toolDep).isHostConfiguration()).isTrue(); + BuildConfigurationValue toolConfiguration = getConfiguration(toolDep); + assertThat(toolConfiguration.isToolConfiguration()).isTrue(); + assertThat(toolConfiguration.getOptions().get(PlatformOptions.class).platforms) + .containsExactly(EXEC_PLATFORM_LABEL); } @Test