Fix label_flag and label_setting to not have a dependency on the default

value. This prevents an extra analysis, since the dependency should only be on the value being used. Fixes bazelbuild#11291.
katre · Apr 19, 2021 · 4a44dab · 4a44dab
1 parent db8a7b5
commit 4a44dab
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 4 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -56,6 +56,7 @@
 import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
 import com.google.devtools.build.lib.packages.RuleFactory.AttributeValues;
 import com.google.devtools.build.lib.packages.Type.ConversionException;
+import com.google.devtools.build.lib.packages.Type.LabelClass;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
 import com.google.devtools.build.lib.util.FileTypeSet;
@@ -941,7 +942,7 @@ public RuleClass build(String name, String key) {
             attr(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, type)
                 .nonconfigurable(BUILD_SETTING_DEFAULT_NONCONFIGURABLE)
                 .mandatory();
-        if (BuildType.isLabelType(type)) {
+        if (type.getLabelClass() == LabelClass.DEPENDENCY) {
           defaultAttrBuilder.allowedFileTypes(FileTypeSet.ANY_FILE);
           defaultAttrBuilder.allowedRuleClasses(ANY_RULE);
         }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/LabelBuildSettings.java b/src/main/java/com/google/devtools/build/lib/rules/LabelBuildSettings.java
@@ -15,6 +15,7 @@
 
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
+import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL;
 import static com.google.devtools.build.lib.packages.RuleClass.Builder.STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME;
 
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
@@ -56,15 +57,15 @@ public class LabelBuildSettings {
           null,
           (rule, attributes, configuration) -> {
             if (rule == null || configuration == null) {
-              return attributes.get(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, LABEL);
+              return attributes.get(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, NODEP_LABEL);
             }
             Object commandLineValue =
                 configuration.getOptions().getStarlarkOptions().get(rule.getLabel());
             Label asLabel;
             try {
               asLabel =
                   commandLineValue == null
-                      ? attributes.get(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, LABEL)
+                      ? attributes.get(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, NODEP_LABEL)
                       : LABEL.convert(commandLineValue, "label_flag value resolution");
             } catch (ConversionException e) {
               throw new IllegalStateException(
@@ -81,7 +82,7 @@ private static RuleClass buildRuleClass(RuleClass.Builder builder, boolean flag)
         .removeAttribute("licenses")
         .removeAttribute("distribs")
         .add(attr(":alias", LABEL).value(ACTUAL))
-        .setBuildSetting(BuildSetting.create(flag, LABEL))
+        .setBuildSetting(BuildSetting.create(flag, NODEP_LABEL))
         .canHaveAnyProvider()
         .useToolchainResolution(ToolchainResolutionMode.DISABLED)
         .build();