Make the platform rule non-configurable.

Also remove the unneeded PlatformBaseRule. Work towards platform-based flags: #19409. PiperOrigin-RevId: 567075741 Change-Id: I5ba99d3b52d17b5786de49561a497b5802001c00
bazelbuild · Sep 20, 2023 · 87fb462 · 87fb462
1 parent 36ca2ac
commit 87fb462
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 65 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Util.java b/src/main/java/com/google/devtools/build/lib/analysis/Util.java
@@ -105,6 +105,10 @@ public static ImmutableSet<ConfiguredTargetKey> findImplicitDeps(RuleContext rul
         maybeImplicitDeps.add(
             ConfiguredTargetKey.builder()
                 .setLabel(platformConfiguration.getTargetPlatform())
+                // This is technically the wrong configuration, because PlatformRule uses
+                // NoConfigTransition to reduce configured target fanout. However, it still works,
+                // because PostAnalysisQueryEnvironment also guesses using the wrong configuration,
+                // so the target platform dependency is correctly marked as implicit anyway.
                 .setConfiguration(ruleContext.getConfiguration())
                 .build());
       }

diff --git a/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java
@@ -502,7 +502,11 @@ private ImmutableList<ClassifiedDependency<T>> targetifyValues(
             key);
 
         boolean implicit =
-            implicitDeps == null || implicitDeps.contains(getConfiguredTargetKey(dependency));
+            // Check both the original guess key and the second correct key. In the case of the
+            // target platform, Util.findImplicitDeps also uses the original guess key.
+            implicitDeps == null
+                || implicitDeps.contains(key)
+                || implicitDeps.contains(getConfiguredTargetKey(dependency));
         values.add(new ClassifiedDependency<>(dependency, implicit));
         knownCtDeps.add(key);
       } else if (settings.contains(Setting.INCLUDE_ASPECTS)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformBaseRule.java b/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformBaseRule.java
diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRule.java b/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRule.java
@@ -16,13 +16,17 @@
 
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
+import com.google.devtools.build.lib.analysis.config.transitions.NoConfigTransition;
 import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.RuleClass;
+import com.google.devtools.build.lib.packages.RuleClass.ToolchainResolutionMode;
 import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.util.FileTypeSet;
 
@@ -40,7 +44,16 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
     <!-- #END_BLAZE_RULE.NAME --> */
     return builder
         .advertiseStarlarkProvider(PlatformInfo.PROVIDER.id())
-
+        .cfg(NoConfigTransition.createFactory())
+        .exemptFromConstraintChecking("this rule helps *define* a constraint")
+        .useToolchainResolution(ToolchainResolutionMode.DISABLED)
+        .removeAttribute(":action_listener")
+        .removeAttribute("applicable_licenses")
+        .override(
+            attr("tags", Type.STRING_LIST)
+                // No need to show up in ":all", etc. target patterns.
+                .value(ImmutableList.of("manual"))
+                .nonconfigurable("low-level attribute, used in platform configuration"))
         /* <!-- #BLAZE_RULE(platform).ATTRIBUTE(constraint_values) -->
         The combination of constraint choices that this platform comprises. In order for a platform
         to apply to a given environment, the environment must have at least the values in this list.
@@ -77,7 +90,9 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
         by using the macro "{PARENT_REMOTE_EXECUTION_PROPERTIES}". See the section on
         <a href="#platform_inheritance">Platform Inheritance</a> for details.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr(REMOTE_EXECUTION_PROPS_ATTR, Type.STRING))
+        .add(
+            attr(REMOTE_EXECUTION_PROPS_ATTR, Type.STRING)
+                .nonconfigurable("Part of the configuration"))
 
         /* <!-- #BLAZE_RULE(platform).ATTRIBUTE(exec_properties) -->
         A map of strings that affect the way actions are executed remotely. Bazel makes no attempt
@@ -91,15 +106,18 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
         This attribute is a full replacement for the deprecated
         <code>remote_execution_properties</code>.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr(EXEC_PROPS_ATTR, Type.STRING_DICT).value(ImmutableMap.of()))
+        .add(
+            attr(EXEC_PROPS_ATTR, Type.STRING_DICT)
+                .value(ImmutableMap.of())
+                .nonconfigurable("Part of the configuration"))
         .build();
   }
 
   @Override
   public Metadata getMetadata() {
     return Metadata.builder()
         .name(RULE_NAME)
-        .ancestors(PlatformBaseRule.class)
+        .ancestors(BaseRuleClasses.NativeBuildRule.class)
         .factoryClass(Platform.class)
         .build();
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRules.java b/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRules.java
@@ -34,7 +34,6 @@ protected PlatformRules() {
   public void init(ConfiguredRuleClassProvider.Builder builder) {
     builder.addConfigurationFragment(PlatformConfiguration.class);
 
-    builder.addRuleDefinition(new PlatformBaseRule());
     builder.addRuleDefinition(new ConstraintSettingRule());
     builder.addRuleDefinition(new ConstraintValueRule());
     builder.addRuleDefinition(new PlatformRule());

diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
@@ -947,6 +947,7 @@ public void testRdeps() throws Exception {
   public void testLet() throws Exception {
     writeBuildFiles3();
 
+    helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS);
     assertContains(
         eval("//b + //c + //d"),
         eval("let x = //a in deps($x) except $x" + getDependencyCorrectionWithGen()));