Emit warning when select()ing on deprecated flags.

Supports removing legacy repo references to remove legay platform flags. RELNOTES: select() on `cpu`, `host_cpu`, or `crosstool_top now emits a "deprecated flag" warning PiperOrigin-RevId: 706003374 Change-Id: Ifb6fbb22d2686f8f12288c972af09dc412a202bd
bazelbuild · Dec 13, 2024 · 48fe861 · 48fe861
1 parent 499d5ee
commit 48fe861
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 37 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
@@ -78,6 +78,10 @@
  */
 public final class ConfigSetting implements RuleConfiguredTargetFactory {
 
+  /** Flags we'd like to remove once there are no more repo references. */
+  private static final ImmutableSet<String> DEPRECATED_FLAGS =
+      ImmutableSet.of("cpu", "host_cpu", "crosstool_top");
+
   @Override
   @Nullable
   public ConfiguredTarget create(RuleContext ruleContext)
@@ -130,7 +134,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
 
     BuildOptionDetails optionDetails = ruleContext.getConfiguration().getBuildOptionDetails();
     boolean nativeFlagsMatch =
-        matchesConfig(nativeFlagSettings.entries(), optionDetails, ruleContext);
+        nativeFlagsMatch(nativeFlagSettings.entries(), optionDetails, ruleContext);
 
     UserDefinedFlagMatch userDefinedFlags =
         UserDefinedFlagMatch.fromAttributeValueAndPrerequisites(
@@ -294,7 +298,7 @@ private static boolean valuesAreSet(
    * Given a list of [flagName, flagValue] pairs for native Blaze flags, returns true if flagName ==
    * flagValue for every item in the list under this configuration, false otherwise.
    */
-  private static boolean matchesConfig(
+  private static boolean nativeFlagsMatch(
       Collection<Map.Entry<String, String>> expectedSettings,
       BuildOptionDetails options,
       RuleContext ruleContext) {
@@ -305,7 +309,13 @@ private static boolean matchesConfig(
     for (Map.Entry<String, String> setting : expectedSettings) {
       String optionName = setting.getKey();
       String expectedRawValue = setting.getValue();
-
+      if (DEPRECATED_FLAGS.contains(optionName)) {
+        ruleContext.ruleWarning(
+            String.format(
+                "select() on %s is deprecated. Use platform constraints instead:"
+                    + " https://bazel.build/docs/configurable-attributes#platforms.",
+                optionName));
+      }
       Class<? extends FragmentOptions> optionClass = options.getOptionClass(optionName);
       if (optionClass == null) {
         ruleContext.attributeError(

diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
@@ -2935,4 +2935,23 @@ public void labelInValuesError() throws Exception {
             + " not a valid setting name, but appears to be a label. Did you mean to place it in"
             + " flag_values instead?");
   }
+
+  @Test
+  @TestParameters({"{flag: cpu}", "{flag: host_cpu}", "{flag: crosstool_top}"})
+  public void selectOnDeprecatedFlagEmitsWarning(String flag) throws Exception {
+    scratch.file(
+        "test/BUILD",
+        """
+        config_setting(
+            name = "match",
+            values = {
+              "%s": "//foo",
+            },
+        )
+        """
+            .formatted(flag));
+    assertThat(getConfiguredTarget("//test:match")).isNotNull();
+    assertContainsEvent(
+        "select() on %s is deprecated. Use platform constraints instead".formatted(flag));
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
@@ -1579,24 +1579,23 @@ def _impl(ctx):
           }
         )
         """);
-
     scratch.file(
         "BUILD",
-        "load('//:my_rule.bzl', 'my_rule')",
-        "config_setting(",
-        "   name = 'arm_cpu',",
-        "   values = {'cpu': 'arm'},",
-        ")",
-        "my_rule(name='r',",
-        "        label_list=select({",
-        "    ':arm_cpu': [],",
-        "    '//conditions:default': ['a.txt'],",
-        "}) + select({",
-        "    ':arm_cpu': ['a.txt'],",
-        "    '//conditions:default': [],",
-        "}),",
-        ")");
-
+        """
+        load('//:my_rule.bzl', 'my_rule')
+        constraint_setting(name = "cpu")
+        constraint_value(name = "arm_cpu", constraint_setting = "cpu")
+        my_rule(
+          name="r",
+          label_list = select({
+            ":arm_cpu": [],
+            "//conditions:default": ["a.txt"],
+          }) + select({
+              ":arm_cpu": ["a.txt"],
+              "//conditions:default": [],
+          }),
+        )
+        """);
     invalidatePackages();
     getConfiguredTarget("//:r");
     assertNoEvents();
@@ -1618,23 +1617,23 @@ def _impl(ctx):
           }
         )
         """);
-
     scratch.file(
         "BUILD",
-        "load('//:my_rule.bzl', 'my_rule')",
-        "config_setting(",
-        "   name = 'arm_cpu',",
-        "   values = {'cpu': 'arm'},",
-        ")",
-        "my_rule(name='r',",
-        "        label_list=select({",
-        "    ':arm_cpu': [],",
-        "    '//conditions:default': ['a.txt'],",
-        "}) + select({",
-        "    ':arm_cpu': ['a.txt'],",
-        "    '//conditions:default': ['a.txt'],",
-        "}),",
-        ")");
+        """
+        load('//:my_rule.bzl', 'my_rule')
+        constraint_setting(name = "cpu")
+        constraint_value(name = "arm_cpu", constraint_setting = "cpu")
+        my_rule(
+          name="r",
+          label_list = select({
+            ":arm_cpu": [],
+            "//conditions:default": ["a.txt"],
+          }) + select({
+              ":arm_cpu": ["a.txt"],
+              "//conditions:default": ["a.txt"],
+          }),
+        )
+        """);
 
     invalidatePackages();
     getConfiguredTarget("//:r");
@@ -3088,7 +3087,7 @@ public void testCoverageInstrumentedMatchesFilterNonDefaultLabel() throws Except
   }
 
   // A list of attributes and methods ctx objects have
-  private final List<String> ctxAttributes =
+  private static final ImmutableList<String> CTX_ATTRIBUTES =
       ImmutableList.of(
           "attr",
           "split_attr",
@@ -3134,7 +3133,7 @@ public void testFrozenRuleContextHasInaccessibleAttributes() throws Exception {
         """);
     scratch.file("test/rules.bzl");
 
-    for (String attribute : ctxAttributes) {
+    for (String attribute : CTX_ATTRIBUTES) {
       scratch.overwriteFile(
           "test/rules.bzl",
           "load('//myinfo:myinfo.bzl', 'MyInfo')",
@@ -3171,7 +3170,7 @@ public void testFrozenRuleContextHasInaccessibleAttributes() throws Exception {
   @Test
   public void testFrozenRuleContextForAspectsHasInaccessibleAttributes() throws Exception {
     List<String> attributes = new ArrayList<>();
-    attributes.addAll(ctxAttributes);
+    attributes.addAll(CTX_ATTRIBUTES);
     attributes.addAll(
         ImmutableList.of("rule.attr", "rule.executable", "rule.file", "rule.files", "rule.kind"));
     scratch.file(