From 9bf885afeb01c766d84acf86ca847a7b5e7bd0d8 Mon Sep 17 00:00:00 2001 From: hlopko Date: Thu, 10 Jan 2019 09:24:52 -0800 Subject: [PATCH] Add --incompatible_remove_cpu_and_compiler_attributes_from_cc_toolchain These attributes were introduced to drive the migration towards Crosstool in Starlark, but we ended up not needing them. They are currently unused, but cpu attribute is mandatory. This cleanup gets rid of them. https://github.com/bazelbuild/bazel/issues/7075 RELNOTES: None. PiperOrigin-RevId: 228723472 --- .../cpp/CcToolchainAttributesProvider.java | 8 ++ .../build/lib/rules/cpp/CcToolchainRule.java | 2 +- .../build/lib/rules/cpp/CppConfiguration.java | 4 + .../build/lib/rules/cpp/CppOptions.java | 16 +++ .../rules/cpp/CcToolchainProviderTest.java | 109 ++++++++++++++++++ tools/cpp/BUILD.empty | 1 - tools/cpp/BUILD.static.freebsd | 2 - tools/cpp/BUILD.static.windows | 4 - tools/cpp/BUILD.tpl | 2 - tools/osx/crosstool/BUILD.tpl | 1 - 10 files changed, 138 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainAttributesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainAttributesProvider.java index 72d95ed23909d9..08c4d778e93fd0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainAttributesProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainAttributesProvider.java @@ -103,6 +103,14 @@ public CcToolchainAttributesProvider( super(ImmutableMap.of(), Location.BUILTIN); this.ccToolchainLabel = ruleContext.getLabel(); this.toolchainIdentifier = ruleContext.attributes().get("toolchain_identifier", Type.STRING); + if (ruleContext.getFragment(CppConfiguration.class).removeCpuCompilerCcToolchainAttributes() + && (ruleContext.attributes().isAttributeValueExplicitlySpecified("cpu") + || ruleContext.attributes().isAttributeValueExplicitlySpecified("compiler"))) { + ruleContext.ruleError( + "attributes 'cpu' and 'compiler' have been deprecated, please remove them. See " + + "https://github.com/bazelbuild/bazel/issues/7075 for details."); + } + this.cpu = ruleContext.attributes().get("cpu", Type.STRING); this.compiler = ruleContext.attributes().get("compiler", Type.STRING); this.proto = ruleContext.attributes().get("proto", Type.STRING); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java index ee4c649c910a6f..eff348b9e0c3a6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java @@ -120,7 +120,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)

When set, it will be used to perform crosstool_config.toolchain selection. It will take precedence over --cpu Bazel option.

*/ - .add(attr("cpu", STRING).nonconfigurable("Used in configuration creation").mandatory()) + .add(attr("cpu", STRING).nonconfigurable("Used in configuration creation")) /* Deprecated. Use toolchain_identifier attribute instead. It will be a noop after CROSSTOOL migration to Starlark diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 0770e89306c9ad..af4c80eef355eb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -568,6 +568,10 @@ public boolean disableDepsetInUserFlags() { return cppOptions.disableDepsetInUserFlags; } + public boolean removeCpuCompilerCcToolchainAttributes() { + return cppOptions.removeCpuCompilerCcToolchainAttributes; + } + public static PathFragment computeDefaultSysroot(String builtInSysroot) { if (builtInSysroot.isEmpty()) { return null; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index e914499302ae1f..2830b6eed28fdc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -688,6 +688,21 @@ public Label getFdoPrefetchHintsLabel() { + "(see https://github.com/bazelbuild/bazel/issues/6861 for migration instructions).") public boolean disableLegacyCrosstoolFields; + @Option( + name = "incompatible_remove_cpu_and_compiler_attributes_from_cc_toolchain", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.TOOLCHAIN, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "If true, Bazel will complain when cc_toolchain.cpu and cc_toolchain.compiler attribtues " + + "are set " + + "(see https://github.com/bazelbuild/bazel/issues/7075 for migration instructions).") + public boolean removeCpuCompilerCcToolchainAttributes; + @Option( name = "incompatible_disable_expand_if_all_available_in_flag_set", defaultValue = "false", @@ -868,6 +883,7 @@ public FragmentOptions getHost() { host.disableRuntimesFilegroups = disableRuntimesFilegroups; host.disableExpandIfAllAvailableInFlagSet = disableExpandIfAllAvailableInFlagSet; host.disableLegacyCcProvider = disableLegacyCcProvider; + host.removeCpuCompilerCcToolchainAttributes = removeCpuCompilerCcToolchainAttributes; return host; } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java index d0c51010553def..2f7d2c6ed27006 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java @@ -80,6 +80,115 @@ public void testSkylarkCallables() throws Exception { assertThat(usePicForDynamicLibraries).isTrue(); } + @Test + public void testRemoveCpuAndCompiler() throws Exception { + scratch.file( + "a/BUILD", + "filegroup(name = 'empty')", + "cc_toolchain_suite(", + " name = 'a_suite',", + " toolchains = { 'k8': ':a' },", + ")", + "cc_toolchain_suite(", + " name = 'b_suite',", + " toolchains = { 'k9': ':b', },", + ")", + "cc_toolchain_suite(", + " name = 'c_suite',", + " toolchains = { 'k10': ':c', },", + ")", + "cc_toolchain(", + " name = 'a',", + " cpu = 'banana',", + " all_files = ':empty',", + " ar_files = ':empty',", + " as_files = ':empty',", + " compiler_files = ':empty',", + " dwp_files = ':empty',", + " linker_files = ':empty',", + " strip_files = ':empty',", + " objcopy_files = ':empty',", + " proto = \"\"\"", + " toolchain_identifier: \"a\"", + " host_system_name: \"a\"", + " target_system_name: \"a\"", + " target_cpu: \"a\"", + " target_libc: \"a\"", + " compiler: \"a\"", + " abi_version: \"a\"", + " abi_libc_version: \"a\"", + "\"\"\")", + "cc_toolchain(", + " name = 'b',", + " compiler = 'banana',", + " all_files = ':empty',", + " ar_files = ':empty',", + " as_files = ':empty',", + " compiler_files = ':empty',", + " dwp_files = ':empty',", + " linker_files = ':empty',", + " strip_files = ':empty',", + " objcopy_files = ':empty',", + " proto = \"\"\"", + " toolchain_identifier: \"a\"", + " host_system_name: \"a\"", + " target_system_name: \"a\"", + " target_cpu: \"a\"", + " target_libc: \"a\"", + " compiler: \"a\"", + " abi_version: \"a\"", + " abi_libc_version: \"a\"", + "\"\"\")", + "cc_toolchain(", + " name = 'c',", + " all_files = ':empty',", + " ar_files = ':empty',", + " as_files = ':empty',", + " compiler_files = ':empty',", + " dwp_files = ':empty',", + " linker_files = ':empty',", + " strip_files = ':empty',", + " objcopy_files = ':empty',", + " proto = \"\"\"", + " toolchain_identifier: \"a\"", + " host_system_name: \"a\"", + " target_system_name: \"a\"", + " target_cpu: \"a\"", + " target_libc: \"a\"", + " compiler: \"a\"", + " abi_version: \"a\"", + " abi_libc_version: \"a\"", + "\"\"\")"); + reporter.removeHandler(failFastHandler); + useConfiguration( + "--crosstool_top=//a:a_suite", + "--cpu=k8", + "--host_cpu=k8", + "--incompatible_remove_cpu_and_compiler_attributes_from_cc_toolchain"); + assertThat(getConfiguredTarget("//a:a_suite")).isNull(); + assertContainsEvent( + "attributes 'cpu' and 'compiler' have been deprecated, please remove them."); + eventCollector.clear(); + + useConfiguration( + "--crosstool_top=//a:b_suite", + "--cpu=k9", + "--host_cpu=k9", + "--incompatible_remove_cpu_and_compiler_attributes_from_cc_toolchain"); + assertThat(getConfiguredTarget("//a:b_suite")).isNull(); + assertContainsEvent( + "attributes 'cpu' and 'compiler' have been deprecated, please remove them."); + eventCollector.clear(); + + useConfiguration( + "--crosstool_top=//a:c_suite", + "--cpu=k10", + "--host_cpu=k10", + "--incompatible_remove_cpu_and_compiler_attributes_from_cc_toolchain"); + getConfiguredTarget("//a:c_suite"); + assertNoEvents(); + } + @Test public void testDisablingCompilationModeFlags() throws Exception { reporter.removeHandler(failFastHandler); diff --git a/tools/cpp/BUILD.empty b/tools/cpp/BUILD.empty index aa03b4a6fa6dba..91a9a2a1ced980 100644 --- a/tools/cpp/BUILD.empty +++ b/tools/cpp/BUILD.empty @@ -42,7 +42,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":empty", - cpu = "local", dwp_files = ":empty", linker_files = ":empty", objcopy_files = ":empty", diff --git a/tools/cpp/BUILD.static.freebsd b/tools/cpp/BUILD.static.freebsd index 934a31f43d5f84..43189ee85abc33 100644 --- a/tools/cpp/BUILD.static.freebsd +++ b/tools/cpp/BUILD.static.freebsd @@ -47,7 +47,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":empty", - cpu = "local", dwp_files = ":empty", linker_files = ":empty", objcopy_files = ":empty", @@ -76,7 +75,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":empty", - cpu = "local", dwp_files = ":empty", linker_files = ":empty", objcopy_files = ":empty", diff --git a/tools/cpp/BUILD.static.windows b/tools/cpp/BUILD.static.windows index 1d3141b219dfe8..8e0ff772d6969e 100644 --- a/tools/cpp/BUILD.static.windows +++ b/tools/cpp/BUILD.static.windows @@ -50,7 +50,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":empty", - cpu = "local", dwp_files = ":empty", linker_files = ":empty", objcopy_files = ":empty", @@ -80,7 +79,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":empty", - cpu = "x64_windows", dwp_files = ":empty", linker_files = ":empty", objcopy_files = ":empty", @@ -110,7 +108,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":empty", - cpu = "x64_windows", dwp_files = ":empty", linker_files = ":empty", objcopy_files = ":empty", @@ -139,7 +136,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":empty", - cpu = "local", dwp_files = ":empty", linker_files = ":empty", objcopy_files = ":empty", diff --git a/tools/cpp/BUILD.tpl b/tools/cpp/BUILD.tpl index 050f29f4bc7c53..f5eb6b27e83416 100644 --- a/tools/cpp/BUILD.tpl +++ b/tools/cpp/BUILD.tpl @@ -61,7 +61,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":compiler_deps", - cpu = "%{name}", dwp_files = ":empty", linker_files = ":compiler_deps", objcopy_files = ":empty", @@ -89,7 +88,6 @@ cc_toolchain( ar_files = ":empty", as_files = ":empty", compiler_files = ":empty", - cpu = "local", dwp_files = ":empty", linker_files = ":empty", objcopy_files = ":empty", diff --git a/tools/osx/crosstool/BUILD.tpl b/tools/osx/crosstool/BUILD.tpl index b11853e96e0762..903c701c02ba13 100644 --- a/tools/osx/crosstool/BUILD.tpl +++ b/tools/osx/crosstool/BUILD.tpl @@ -64,7 +64,6 @@ cc_toolchain_suite( ar_files = ":empty", as_files = ":empty", compiler_files = ":osx_tools_" + arch, - cpu = arch, dwp_files = ":empty", linker_files = ":osx_tools_" + arch, objcopy_files = ":empty",