diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptionConverters.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptionConverters.java index fc76634c3cb7dd..2b2ac09eadd11e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptionConverters.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptionConverters.java @@ -16,6 +16,7 @@ import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; +import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL; import static com.google.devtools.build.lib.packages.Type.BOOLEAN; import static com.google.devtools.build.lib.packages.Type.INTEGER; import static com.google.devtools.build.lib.packages.Type.STRING; @@ -57,6 +58,7 @@ public class CoreOptionConverters { .put(STRING_LIST, new CommaSeparatedOptionListConverter()) .put(LABEL, new LabelConverter()) .put(LABEL_LIST, new LabelListConverter()) + .put(NODEP_LABEL, new LabelConverter()) .build(); /** A converter from strings to Starlark int values. */ diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildSetting.java b/src/main/java/com/google/devtools/build/lib/packages/BuildSetting.java index ac5f4936bf9751..c27ebdf2cea0a6 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BuildSetting.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BuildSetting.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.packages; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.packages.Type.LabelClass; import com.google.devtools.build.lib.starlarkbuildapi.StarlarkConfigApi.BuildSettingApi; import net.starlark.java.eval.Printer; @@ -28,6 +30,9 @@ public class BuildSetting implements BuildSettingApi { public BuildSetting(boolean isFlag, Type type) { this.isFlag = isFlag; this.type = type; + Preconditions.checkState( + type.getLabelClass() != LabelClass.DEPENDENCY, + "Build settings should not create a dependency with their default attribute"); } public Type getType() { 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 index 3b3b02d9a03d52..d7616ab2175d65 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.packages; -import static com.google.devtools.build.lib.packages.Attribute.ANY_RULE; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import static com.google.devtools.build.lib.packages.ExecGroup.COPY_FROM_RULE_EXEC_GROUP; @@ -56,7 +55,6 @@ import com.google.devtools.build.lib.packages.Type.ConversionException; 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; import com.google.devtools.build.lib.util.StringUtil; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -858,10 +856,6 @@ 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)) { - attrBuilder.allowedFileTypes(FileTypeSet.ANY_FILE); - attrBuilder.allowedRuleClasses(ANY_RULE); - } this.add(attrBuilder); // Build setting rules should opt out of toolchain resolution, since they form part of the 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 index 723346d4f572f2..2440f3830361b4 100644 --- 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; @@ -55,7 +56,7 @@ 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()); @@ -63,7 +64,7 @@ public class LabelBuildSettings { 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( @@ -80,7 +81,7 @@ private static RuleClass buildRuleClass(RuleClass.Builder builder, boolean flag) .removeAttribute("licenses") .removeAttribute("distribs") .add(attr(":alias", LABEL).value(ACTUAL)) - .setBuildSetting(new BuildSetting(flag, LABEL)) + .setBuildSetting(new BuildSetting(flag, NODEP_LABEL)) .canHaveAnyProvider() .useToolchainResolution(false) .build(); diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java index 4512457ef9d731..24f268c262b384 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java @@ -18,6 +18,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.LABEL_LIST; +import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL; import static com.google.devtools.build.lib.packages.BuildType.OUTPUT_LIST; import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.substitutePlaceholderIntoTemplate; import static com.google.devtools.build.lib.packages.RuleClass.Builder.STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME; @@ -1127,7 +1128,7 @@ public void testBuildSetting_createsDefaultAttribute() { new RuleClass.Builder("label_flag", RuleClassType.NORMAL, false) .factory(DUMMY_CONFIGURED_TARGET_FACTORY) .add(attr("tags", STRING_LIST)) - .setBuildSetting(new BuildSetting(true, LABEL)) + .setBuildSetting(new BuildSetting(true, NODEP_LABEL)) .build(); RuleClass stringSetting = new RuleClass.Builder("string_setting", RuleClassType.NORMAL, false) @@ -1136,7 +1137,7 @@ public void testBuildSetting_createsDefaultAttribute() { .setBuildSetting(new BuildSetting(false, STRING)) .build(); - assertThat(labelFlag.hasAttr(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, LABEL)).isTrue(); + assertThat(labelFlag.hasAttr(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, NODEP_LABEL)).isTrue(); assertThat(stringSetting.hasAttr(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, STRING)).isTrue(); }