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. #7075 RELNOTES: None. PiperOrigin-RevId: 228723472
bazelbuild · Jan 10, 2019 · 9bf885a · 9bf885a
1 parent f5b7122
commit 9bf885a
Showing 10 changed files with 138 additions and 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
@@ -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
@@ -120,7 +120,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
          <p>When set, it will be used to perform crosstool_config.toolchain selection. It will take
          precedence over --cpu Bazel option.</p>
         <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
-        .add(attr("cpu", STRING).nonconfigurable("Used in configuration creation").mandatory())
+        .add(attr("cpu", STRING).nonconfigurable("Used in configuration creation"))
         /* <!-- #BLAZE_RULE(cc_toolchain).ATTRIBUTE(compiler) -->
          Deprecated. Use toolchain_identifier attribute instead. It will be a noop after
          <a href="https://github.com/bazelbuild/bazel/issues/5380">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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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",