Add the incompatible flag for using auto-configured host platform. Part of the work on #6849. Incompatible change issue: #7081 Closes #7070. RELNOTES[INC]: The default value of --host_platform and --platforms will be changed to not be dependent on the configuration. This means that setting --cpu or --host_cpu will not affect the target or host platform. PiperOrigin-RevId: 228745152
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfigurationLoader.java index 9f397e2..ce34005 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfigurationLoader.java
@@ -35,15 +35,34 @@ public PlatformConfiguration create(BuildOptions buildOptions) throws InvalidConfigurationException { PlatformOptions platformOptions = buildOptions.get(PlatformOptions.class); - if (platformOptions.hostPlatform == null) { - throw new InvalidConfigurationException("Host platform not set"); + + // Handle default values for the host and target platform. + // TODO(https://github.com/bazelbuild/bazel/issues/6849): After migration, set the defaults + // directly. + Label hostPlatform; + if (platformOptions.hostPlatform != null) { + hostPlatform = platformOptions.hostPlatform; + } else if (platformOptions.autoConfigureHostPlatform) { + // Use the auto-configured host platform. + hostPlatform = PlatformOptions.DEFAULT_HOST_PLATFORM; + } else { + // Use the legacy host platform. + hostPlatform = PlatformOptions.LEGACY_DEFAULT_HOST_PLATFORM; } - Label targetPlatform = Iterables.getFirst(platformOptions.platforms, null); - if (targetPlatform == null) { - throw new InvalidConfigurationException("Target platform not set"); + + Label targetPlatform; + if (!platformOptions.platforms.isEmpty()) { + targetPlatform = Iterables.getFirst(platformOptions.platforms, null); + } else if (platformOptions.autoConfigureHostPlatform) { + // Default to the host platform, whatever it is. + targetPlatform = hostPlatform; + } else { + // Use the legacy target platform + targetPlatform = PlatformOptions.LEGACY_DEFAULT_TARGET_PLATFORM; } + return new PlatformConfiguration( - platformOptions.hostPlatform, + hostPlatform, ImmutableList.copyOf(platformOptions.extraExecutionPlatforms), targetPlatform, ImmutableList.copyOf(platformOptions.extraToolchains),
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java index 7aef05d..c0fb6c2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java
@@ -23,23 +23,33 @@ import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; +import com.google.devtools.common.options.OptionMetadataTag; import java.util.List; /** Command-line options for platform-related configuration. */ public class PlatformOptions extends FragmentOptions { + + // TODO(https://github.com/bazelbuild/bazel/issues/6849): After migration, set the defaults + // directly. + public static final Label LEGACY_DEFAULT_HOST_PLATFORM = + Label.parseAbsoluteUnchecked("@bazel_tools//platforms:host_platform"); + public static final Label DEFAULT_HOST_PLATFORM = + Label.parseAbsoluteUnchecked("@local_config_platform//:host"); + public static final Label LEGACY_DEFAULT_TARGET_PLATFORM = + Label.parseAbsoluteUnchecked("@bazel_tools//platforms:target_platform"); + @Option( - name = "host_platform", - oldName = "experimental_host_platform", - converter = BuildConfiguration.EmptyToNullLabelConverter.class, - defaultValue = "@bazel_tools//platforms:host_platform", - documentationCategory = OptionDocumentationCategory.TOOLCHAIN, - effectTags = { - OptionEffectTag.AFFECTS_OUTPUTS, - OptionEffectTag.CHANGES_INPUTS, - OptionEffectTag.LOADING_AND_ANALYSIS - }, - help = "The label of a platform rule that describes the host system." - ) + name = "host_platform", + oldName = "experimental_host_platform", + converter = BuildConfiguration.EmptyToNullLabelConverter.class, + defaultValue = "", + documentationCategory = OptionDocumentationCategory.TOOLCHAIN, + effectTags = { + OptionEffectTag.AFFECTS_OUTPUTS, + OptionEffectTag.CHANGES_INPUTS, + OptionEffectTag.LOADING_AND_ANALYSIS + }, + help = "The label of a platform rule that describes the host system.") public Label hostPlatform; @Option( @@ -69,19 +79,19 @@ public List<String> extraExecutionPlatforms; @Option( - name = "platforms", - oldName = "experimental_platforms", - converter = LabelListConverter.class, - defaultValue = "@bazel_tools//platforms:target_platform", - documentationCategory = OptionDocumentationCategory.TOOLCHAIN, - effectTags = { - OptionEffectTag.AFFECTS_OUTPUTS, - OptionEffectTag.CHANGES_INPUTS, - OptionEffectTag.LOADING_AND_ANALYSIS - }, - help = - "The labels of the platform rules describing the target platforms for the current command." - ) + name = "platforms", + oldName = "experimental_platforms", + converter = LabelListConverter.class, + defaultValue = "", + documentationCategory = OptionDocumentationCategory.TOOLCHAIN, + effectTags = { + OptionEffectTag.AFFECTS_OUTPUTS, + OptionEffectTag.CHANGES_INPUTS, + OptionEffectTag.LOADING_AND_ANALYSIS + }, + help = + "The labels of the platform rules describing the target platforms for the current " + + "command.") public List<Label> platforms; @Option( @@ -145,6 +155,20 @@ ) public List<Label> enabledToolchainTypes; + @Option( + name = "incompatible_auto_configure_host_platform", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "If true, the host platform will be inherited from @local_config_platforms//:host, " + + "instead of being based on the --cpu (and --host_cpu) flags.") + public boolean autoConfigureHostPlatform; + @Override public PlatformOptions getHost() { PlatformOptions host = (PlatformOptions) getDefault(); @@ -157,6 +181,7 @@ host.hostPlatformRemotePropertiesOverride = this.hostPlatformRemotePropertiesOverride; host.toolchainResolutionDebug = this.toolchainResolutionDebug; host.toolchainResolutionOverrides = this.toolchainResolutionOverrides; + host.autoConfigureHostPlatform = this.autoConfigureHostPlatform; return host; } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java index 993acc0..ba2fd67 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java +++ b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java
@@ -43,6 +43,9 @@ public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException, RuleErrorException, ActionConflictException { + PlatformOptions platformOptions = + ruleContext.getConfiguration().getOptions().get(PlatformOptions.class); + PlatformInfo.Builder platformBuilder = PlatformInfo.builder().setLabel(ruleContext.getLabel()); List<PlatformInfo> parentPlatforms = @@ -58,23 +61,9 @@ PlatformInfo parentPlatform = Iterables.getFirst(parentPlatforms, null); platformBuilder.setParent(parentPlatform); - Boolean isHostPlatform = - ruleContext.attributes().get(PlatformRule.HOST_PLATFORM_ATTR, Type.BOOLEAN); - Boolean isTargetPlatform = - ruleContext.attributes().get(PlatformRule.TARGET_PLATFORM_ATTR, Type.BOOLEAN); - if (isHostPlatform && isTargetPlatform) { - ruleContext.attributeError( - PlatformRule.HOST_PLATFORM_ATTR, - "A single platform cannot have both host_platform and target_platform set."); - return null; - } else if (isHostPlatform) { - // Create default constraints based on the current host OS and CPU values. - String cpuOption = ruleContext.getConfiguration().getHostCpu(); - autodetectConstraints(cpuOption, ruleContext, platformBuilder); - } else if (isTargetPlatform) { - // Create default constraints based on the current OS and CPU values. - String cpuOption = ruleContext.getConfiguration().getCpu(); - autodetectConstraints(cpuOption, ruleContext, platformBuilder); + if (!platformOptions.autoConfigureHostPlatform) { + // If the flag is set, the constraints are defaulted by @local_config_platforms. + setDefaultConstraints(platformBuilder, ruleContext); } // Add the declared constraints. Because setting the host_platform or target_platform attribute @@ -86,10 +75,9 @@ String remoteExecutionProperties = ruleContext.attributes().get(PlatformRule.REMOTE_EXECUTION_PROPS_ATTR, Type.STRING); - if (Strings.isNullOrEmpty(platformBuilder.getRemoteExecutionProperties()) && isHostPlatform) { + if (Strings.isNullOrEmpty(platformBuilder.getRemoteExecutionProperties()) + && ruleContext.attributes().get(PlatformRule.HOST_PLATFORM_ATTR, Type.BOOLEAN)) { // Use the default override. - PlatformOptions platformOptions = - ruleContext.getConfiguration().getOptions().get(PlatformOptions.class); remoteExecutionProperties = platformOptions.hostPlatformRemotePropertiesOverride; } platformBuilder.setRemoteExecutionProperties(remoteExecutionProperties); @@ -111,6 +99,27 @@ .build(); } + private void setDefaultConstraints(PlatformInfo.Builder platformBuilder, RuleContext ruleContext) + throws RuleErrorException { + Boolean isHostPlatform = + ruleContext.attributes().get(PlatformRule.HOST_PLATFORM_ATTR, Type.BOOLEAN); + Boolean isTargetPlatform = + ruleContext.attributes().get(PlatformRule.TARGET_PLATFORM_ATTR, Type.BOOLEAN); + if (isHostPlatform && isTargetPlatform) { + throw ruleContext.throwWithAttributeError( + PlatformRule.HOST_PLATFORM_ATTR, + "A single platform cannot have both host_platform and target_platform set."); + } else if (isHostPlatform) { + // Create default constraints based on the current host OS and CPU values. + String cpuOption = ruleContext.getConfiguration().getHostCpu(); + autodetectConstraints(cpuOption, ruleContext, platformBuilder); + } else if (isTargetPlatform) { + // Create default constraints based on the current OS and CPU values. + String cpuOption = ruleContext.getConfiguration().getCpu(); + autodetectConstraints(cpuOption, ruleContext, platformBuilder); + } + } + private void autodetectConstraints( String cpuOption, RuleContext ruleContext, PlatformInfo.Builder platformBuilder) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index 7ccc9d1..15fe4ea 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -60,6 +60,8 @@ @Override public List<String> getWorkspaceContents(MockToolsConfig config) { String bazelToolWorkspace = config.getPath("/bazel_tools_workspace").getPathString(); + String localConfigPlatformWorkspace = + config.getPath("/local_config_platform_workspace").getPathString(); return new ArrayList<>( ImmutableList.of( "local_repository(name = 'bazel_tools', path = '" + bazelToolWorkspace + "')", @@ -69,7 +71,10 @@ "bind(name = 'tools/python', actual='//tools/python')", "register_toolchains('@bazel_tools//tools/cpp:all')", "register_toolchains('@bazel_tools//tools/jdk:dummy_java_toolchain')", - "register_toolchains('@bazel_tools//tools/jdk:dummy_java_runtime_toolchain')")); + "register_toolchains('@bazel_tools//tools/jdk:dummy_java_runtime_toolchain')", + "local_repository(name = 'local_config_platform', path = '" + + localConfigPlatformWorkspace + + "')")); } @Override
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java index a22577d..317dce7 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java
@@ -340,7 +340,8 @@ } else { config.create("tools/cpp/link_dynamic_library.sh", ""); } - MockPlatformSupport.setup(config, "/bazel_tools_workspace/platforms"); + MockPlatformSupport.setup( + config, "/bazel_tools_workspace/platforms", "/local_config_platform_workspace"); } @Override
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockPlatformSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockPlatformSupport.java index dcb8c18..30f8a3b 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockPlatformSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockPlatformSupport.java
@@ -17,15 +17,25 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.testutil.TestConstants; import java.io.IOException; +import javax.annotation.Nullable; /** Mocking support for platforms and toolchains. */ public class MockPlatformSupport { /** Adds mocks for basic host and target platform. */ - public static void setup(MockToolsConfig mockToolsConfig, String platformsPath) + public static void setup(MockToolsConfig mockToolsConfig, String bazelToolsPlatformsPath) + throws IOException { + setup(mockToolsConfig, bazelToolsPlatformsPath, null); + } + + /** Adds mocks for basic host and target platform. */ + public static void setup( + MockToolsConfig mockToolsConfig, + String bazelToolsPlatformsPath, + @Nullable String localConfigPlatformPath) throws IOException { mockToolsConfig.create( - platformsPath + "/BUILD", + bazelToolsPlatformsPath + "/BUILD", "package(default_visibility=['//visibility:public'])", "constraint_setting(name = 'cpu')", "constraint_value(", @@ -107,6 +117,15 @@ " ':windows',", " ],", ")"); + if (localConfigPlatformPath != null) { + // Only create these if the local config workspace exists. + mockToolsConfig.create( + localConfigPlatformPath + "/WORKSPACE", "workspace(name = 'local_config_platform')"); + mockToolsConfig.create( + localConfigPlatformPath + "/BUILD", + "package(default_visibility=['//visibility:public'])", + "platform(name = 'host')"); + } } /** Adds a mock piii platform. */
diff --git a/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformConfigurationApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformConfigurationApiTest.java index 07c5462..f35776c 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformConfigurationApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformConfigurationApiTest.java
@@ -14,12 +14,10 @@ package com.google.devtools.build.lib.rules.platform; import static com.google.common.truth.Truth.assertThat; -import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.ConfiguredTarget; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey; @@ -54,11 +52,6 @@ } @Test - public void testHostPlatform_unset() { - assertThrows(InvalidConfigurationException.class, () -> useConfiguration("--host_platform=")); - } - - @Test public void testTargetPlatform_single() throws Exception { scratch.file("platforms/BUILD", "platform(name = 'test_platform')"); @@ -81,11 +74,6 @@ } @Test - public void testTargetPlatform_unset() { - assertThrows(InvalidConfigurationException.class, () -> useConfiguration("--platforms=")); - } - - @Test public void testTargetPlatform_multiple() throws Exception { scratch.file( "platforms/BUILD",
diff --git a/src/test/shell/bazel/toolchain_test.sh b/src/test/shell/bazel/toolchain_test.sh index 3ba917e..6ee05fa 100755 --- a/src/test/shell/bazel/toolchain_test.sh +++ b/src/test/shell/bazel/toolchain_test.sh
@@ -349,10 +349,11 @@ bazel build \ --toolchain_resolution_debug \ + --incompatible_auto_configure_host_platform \ //demo:use &> $TEST_log || fail "Build failed" expect_log 'ToolchainResolution: Looking for toolchain of type //toolchain:test_toolchain' - expect_log 'ToolchainResolution: For toolchain type //toolchain:test_toolchain, possible execution platforms and toolchains: {@bazel_tools//platforms:host_platform -> //:test_toolchain_impl_1}' - expect_log 'ToolchainResolver: Selected execution platform @bazel_tools//platforms:host_platform, type //toolchain:test_toolchain -> toolchain //:test_toolchain_impl_1' + expect_log 'ToolchainResolution: For toolchain type //toolchain:test_toolchain, possible execution platforms and toolchains: {@local_config_platform//:host -> //:test_toolchain_impl_1}' + expect_log 'ToolchainResolver: Selected execution platform @local_config_platform//:host, type //toolchain:test_toolchain -> toolchain //:test_toolchain_impl_1' expect_log 'Using toolchain: rule message: "this is the rule", toolchain extra_str: "foo from test_toolchain"' }