From e959e44a4363fc12ae2df2c5bc0cd0d12f80bbd9 Mon Sep 17 00:00:00 2001 From: John Cater Date: Wed, 28 Feb 2018 07:52:21 -0800 Subject: Update ToolchainUtil to properly load and use the available execution platforms, and correctly merge together the results from TRF. Part of #4442. Change-Id: I31d83fa73a93d39a0e18d05a43a1c8666ac5a2d2 PiperOrigin-RevId: 187324257 --- .../RegisteredExecutionPlatformsFunctionTest.java | 10 +- .../skyframe/ToolchainResolutionFunctionTest.java | 32 ++--- .../build/lib/skyframe/ToolchainUtilTest.java | 139 +++++++++++++++++---- src/test/shell/bazel/toolchain_test.sh | 7 +- 4 files changed, 140 insertions(+), 48 deletions(-) (limited to 'src/test') diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java index 7ebc55b31a..c00261969c 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java @@ -23,6 +23,7 @@ import com.google.common.truth.IterableSubject; import com.google.devtools.build.lib.analysis.platform.PlatformInfo.DuplicateConstraintException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.rules.platform.ToolchainTestCase; +import com.google.devtools.build.lib.skyframe.ToolchainUtil.InvalidPlatformException; import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils; import com.google.devtools.build.skyframe.EvaluationResult; import com.google.devtools.build.skyframe.SkyKey; @@ -115,13 +116,16 @@ public class RegisteredExecutionPlatformsFunctionTest extends ToolchainTestCase SkyKey executionPlatformsKey = RegisteredExecutionPlatformsValue.key(targetConfigKey); EvaluationResult result = requestExecutionPlatformsFromSkyframe(executionPlatformsKey); + assertThatEvaluationResult(result).hasError(); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(executionPlatformsKey) + .hasExceptionThat() + .isInstanceOf(InvalidPlatformException.class); assertThatEvaluationResult(result) .hasErrorEntryForKeyThat(executionPlatformsKey) .hasExceptionThat() .hasMessageThat() - .contains( - "invalid registered execution platform '//error:not_an_execution_platform': " - + "target does not provide the PlatformInfo provider"); + .contains("//error:not_an_execution_platform"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunctionTest.java index 0194819122..702929be47 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunctionTest.java @@ -88,7 +88,7 @@ public class ToolchainResolutionFunctionTest extends ToolchainTestCase { ToolchainResolutionValue toolchainResolutionValue = result.get(key); assertThat(toolchainResolutionValue.availableToolchainLabels()) - .containsExactly(macPlatform, makeLabel("//toolchain:toolchain_2_impl")); + .containsExactly(MAC_CTKEY, makeLabel("//toolchain:toolchain_2_impl")); } @Test @@ -118,9 +118,9 @@ public class ToolchainResolutionFunctionTest extends ToolchainTestCase { ToolchainResolutionValue toolchainResolutionValue = result.get(key); assertThat(toolchainResolutionValue.availableToolchainLabels()) .containsExactly( - linuxPlatform, + LINUX_CTKEY, makeLabel("//extra:extra_toolchain_impl"), - macPlatform, + MAC_CTKEY, makeLabel("//toolchain:toolchain_2_impl")); } @@ -146,37 +146,27 @@ public class ToolchainResolutionFunctionTest extends ToolchainTestCase { new EqualsTester() .addEqualityGroup( ToolchainResolutionValue.create( - ImmutableMap.builder() - .put(linuxPlatform, makeLabel("//test:toolchain_impl_1")) - .build()), + ImmutableMap.of(LINUX_CTKEY, makeLabel("//test:toolchain_impl_1"))), ToolchainResolutionValue.create( - ImmutableMap.builder() - .put(linuxPlatform, makeLabel("//test:toolchain_impl_1")) - .build())) + ImmutableMap.of(LINUX_CTKEY, makeLabel("//test:toolchain_impl_1")))) // Different execution platform, same label. .addEqualityGroup( ToolchainResolutionValue.create( - ImmutableMap.builder() - .put(macPlatform, makeLabel("//test:toolchain_impl_1")) - .build())) + ImmutableMap.of(MAC_CTKEY, makeLabel("//test:toolchain_impl_1")))) // Same execution platform, different label. .addEqualityGroup( ToolchainResolutionValue.create( - ImmutableMap.builder() - .put(linuxPlatform, makeLabel("//test:toolchain_impl_2")) - .build())) + ImmutableMap.of(LINUX_CTKEY, makeLabel("//test:toolchain_impl_2")))) // Different execution platform, different label. .addEqualityGroup( ToolchainResolutionValue.create( - ImmutableMap.builder() - .put(macPlatform, makeLabel("//test:toolchain_impl_2")) - .build())) + ImmutableMap.of(MAC_CTKEY, makeLabel("//test:toolchain_impl_2")))) // Multiple execution platforms. .addEqualityGroup( ToolchainResolutionValue.create( - ImmutableMap.builder() - .put(linuxPlatform, makeLabel("//test:toolchain_impl_1")) - .put(macPlatform, makeLabel("//test:toolchain_impl_1")) + ImmutableMap.builder() + .put(LINUX_CTKEY, makeLabel("//test:toolchain_impl_1")) + .put(MAC_CTKEY, makeLabel("//test:toolchain_impl_1")) .build())); } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainUtilTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainUtilTest.java index 442f5de64b..166bc220e9 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainUtilTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainUtilTest.java @@ -18,14 +18,15 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.assertThatEvaluationResult; 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.BlazeDirectories; import com.google.devtools.build.lib.analysis.ToolchainContext; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.rules.platform.ToolchainTestCase; +import com.google.devtools.build.lib.skyframe.ToolchainUtil.InvalidPlatformException; import com.google.devtools.build.lib.skyframe.ToolchainUtil.ToolchainContextException; import com.google.devtools.build.lib.skyframe.ToolchainUtil.UnresolvedToolchainsException; import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils; @@ -72,11 +73,28 @@ public class ToolchainUtilTest extends ToolchainTestCase { @Test public void createToolchainContext() throws Exception { - useConfiguration( - "--host_platform=//platforms:linux", - "--platforms=//platforms:mac"); + // This should select platform mac, toolchain extra_toolchain_mac, because platform + // mac is listed first. + addToolchain( + "extra", + "extra_toolchain_linux", + ImmutableList.of("//constraints:linux"), + ImmutableList.of("//constraints:linux"), + "baz"); + addToolchain( + "extra", + "extra_toolchain_mac", + ImmutableList.of("//constraints:mac"), + ImmutableList.of("//constraints:linux"), + "baz"); + rewriteWorkspace( + "register_toolchains('//extra:extra_toolchain_linux', '//extra:extra_toolchain_mac')", + "register_execution_platforms('//platforms:mac', '//platforms:linux')"); + + useConfiguration("--platforms=//platforms:linux"); CreateToolchainContextKey key = - CreateToolchainContextKey.create("test", ImmutableSet.of(testToolchainType), targetConfig); + CreateToolchainContextKey.create( + "test", ImmutableSet.of(testToolchainType), targetConfigKey); EvaluationResult result = createToolchainContext(key); @@ -86,15 +104,15 @@ public class ToolchainUtilTest extends ToolchainTestCase { assertThat(toolchainContext.getRequiredToolchains()).containsExactly(testToolchainType); assertThat(toolchainContext.getResolvedToolchainLabels()) - .containsExactly(Label.parseAbsoluteUnchecked("//toolchain:toolchain_1_impl")); + .containsExactly(Label.parseAbsoluteUnchecked("//extra:extra_toolchain_mac_impl")); assertThat(toolchainContext.getExecutionPlatform()).isNotNull(); assertThat(toolchainContext.getExecutionPlatform().label()) - .isEqualTo(Label.parseAbsoluteUnchecked("//platforms:linux")); + .isEqualTo(Label.parseAbsoluteUnchecked("//platforms:mac")); assertThat(toolchainContext.getTargetPlatform()).isNotNull(); assertThat(toolchainContext.getTargetPlatform().label()) - .isEqualTo(Label.parseAbsoluteUnchecked("//platforms:mac")); + .isEqualTo(Label.parseAbsoluteUnchecked("//platforms:linux")); } @Test @@ -107,7 +125,7 @@ public class ToolchainUtilTest extends ToolchainTestCase { "test", ImmutableSet.of( testToolchainType, Label.parseAbsoluteUnchecked("//fake/toolchain:type_1")), - targetConfig); + targetConfigKey); EvaluationResult result = createToolchainContext(key); @@ -140,7 +158,7 @@ public class ToolchainUtilTest extends ToolchainTestCase { testToolchainType, Label.parseAbsoluteUnchecked("//fake/toolchain:type_1"), Label.parseAbsoluteUnchecked("//fake/toolchain:type_2")), - targetConfig); + targetConfigKey); EvaluationResult result = createToolchainContext(key); @@ -156,6 +174,90 @@ public class ToolchainUtilTest extends ToolchainTestCase { // Only one of the missing types will be reported, so do not check the specific error message. } + @Test + public void createToolchainContext_invalidTargetPlatform() throws Exception { + scratch.file("invalid/BUILD", "filegroup(name = 'not_a_platform')"); + useConfiguration("--platforms=//invalid:not_a_platform"); + CreateToolchainContextKey key = + CreateToolchainContextKey.create( + "test", ImmutableSet.of(testToolchainType), targetConfigKey); + + EvaluationResult result = createToolchainContext(key); + + assertThatEvaluationResult(result).hasError(); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .isInstanceOf(ToolchainContextException.class); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .hasCauseThat() + .isInstanceOf(InvalidPlatformException.class); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .hasCauseThat() + .hasMessageThat() + .contains("//invalid:not_a_platform"); + } + + @Test + public void createToolchainContext_invalidHostPlatform() throws Exception { + scratch.file("invalid/BUILD", "filegroup(name = 'not_a_platform')"); + useConfiguration("--host_platform=//invalid:not_a_platform"); + CreateToolchainContextKey key = + CreateToolchainContextKey.create( + "test", ImmutableSet.of(testToolchainType), targetConfigKey); + + EvaluationResult result = createToolchainContext(key); + + assertThatEvaluationResult(result).hasError(); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .isInstanceOf(ToolchainContextException.class); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .hasCauseThat() + .isInstanceOf(InvalidPlatformException.class); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .hasCauseThat() + .hasMessageThat() + .contains("//invalid:not_a_platform"); + } + + @Test + public void createToolchainContext_invalidExecutionPlatform() throws Exception { + scratch.file("invalid/BUILD", "filegroup(name = 'not_a_platform')"); + useConfiguration("--extra_execution_platforms=//invalid:not_a_platform"); + CreateToolchainContextKey key = + CreateToolchainContextKey.create( + "test", ImmutableSet.of(testToolchainType), targetConfigKey); + + EvaluationResult result = createToolchainContext(key); + + assertThatEvaluationResult(result).hasError(); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .isInstanceOf(ToolchainContextException.class); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .hasCauseThat() + .isInstanceOf(InvalidPlatformException.class); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(key) + .hasExceptionThat() + .hasCauseThat() + .hasMessageThat() + .contains("//invalid:not_a_platform"); + } + // Calls ToolchainUtil.createToolchainContext. private static final SkyFunctionName CREATE_TOOLCHAIN_CONTEXT_FUNCTION = SkyFunctionName.create("CREATE_TOOLCHAIN_CONTEXT_FUNCTION"); @@ -171,12 +273,14 @@ public class ToolchainUtilTest extends ToolchainTestCase { abstract Set