Allow starlark rules to be build settings.
For skylark rules that are declared build settings, we auto-add a nonconfigurable build_setting_default attribute which must be set on a per target basis. We also add access to the build setting value via the rule context object. All of this functionality is hidden behind the --experimental_build_setting_api flag which by default is flipped off.
This includes:
- Add a build_setting parameter to starlark rule construction.
- If a starlark rule sets the build_setting parameter, auto-add a mandatory 'build_setting_default' attribute of the same type.
- Allow the value of a build setting skylark rules to be accessed via ctx.build_setting_value.
- Get rid of BuildSettingDescriptor since we don't need both it and BuildSetting.
- Add the --experimental_build_setting_api flag to SkylarkSemantics flags.
Work towards issue #5577
PiperOrigin-RevId: 219537101
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index ef657dd..1e87ef3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -18,6 +18,7 @@
import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith;
import static org.junit.Assert.fail;
+import com.google.common.base.Ascii;
import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
@@ -341,8 +342,8 @@
return ResourceSet.createWithRamCpu(Double.MAX_VALUE, Double.MAX_VALUE);
}
- protected final BuildConfigurationCollection createConfigurations(String... args)
- throws Exception {
+ private BuildConfigurationCollection createConfigurations(
+ ImmutableMap<String, Object> skylarkOptions, String... args) throws Exception {
optionsParser =
OptionsParser.newOptionsParser(
Iterables.concat(
@@ -358,6 +359,9 @@
optionsParser.parse(allArgs);
optionsParser.parse(args);
+ // TODO(juliexxia): when the skylark options parsing work goes in, add type verification here.
+ optionsParser.setSkylarkOptionsForTesting(skylarkOptions);
+
InvocationPolicyEnforcer optionsPolicyEnforcer =
getAnalysisMock().getInvocationPolicyEnforcer();
optionsPolicyEnforcer.enforce(optionsParser);
@@ -504,20 +508,32 @@
* Sets host and target configuration using the specified options, falling back to the default
* options for unspecified ones, and recreates the build view.
*
+ * TODO(juliexxia): when skylark option parsing exists, find a way to combine these parameters
+ * into a single parameter so skylark/native options don't have to be specified separately.
+ *
+ * @param skylarkOptions map of skylark-defined options where the keys are option names (in the
+ * form of label-like strings) and the values are option values
+ * @param args native option name/pair descriptions in command line form (e.g. "--cpu=k8")
+ *
* @throws IllegalArgumentException
*/
- protected void useConfiguration(String... args) throws Exception {
+ protected void useConfiguration(ImmutableMap<String, Object> skylarkOptions, String... args)
+ throws Exception {
String[] actualArgs;
actualArgs = Arrays.copyOf(args, args.length + 1);
- actualArgs[args.length] = "--experimental_dynamic_configs="
- + configsMode.toString().toLowerCase();
- masterConfig = createConfigurations(actualArgs);
+ actualArgs[args.length] =
+ "--experimental_dynamic_configs=" + Ascii.toLowerCase(configsMode.toString());
+ masterConfig = createConfigurations(skylarkOptions, actualArgs);
targetConfig = getTargetConfiguration();
targetConfigKey = BuildConfigurationValue.key(targetConfig);
configurationArgs = Arrays.asList(actualArgs);
createBuildView();
}
+ protected void useConfiguration(String... args) throws Exception {
+ useConfiguration(ImmutableMap.of(), args);
+ }
+
/**
* Makes subsequent {@link #useConfiguration} calls automatically use the specified style for
* configurations.
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 a83f34a..eb3544b 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
@@ -20,6 +20,7 @@
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
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.SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME;
import static com.google.devtools.build.lib.packages.RuleClass.NO_EXTERNAL_BINDINGS;
import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
import static com.google.devtools.build.lib.syntax.Type.INTEGER;
@@ -911,7 +912,8 @@
ExecutionPlatformConstraintsAllowed.PER_RULE,
/* executionPlatformConstraints= */ ImmutableSet.of(),
OutputFile.Kind.FILE,
- ImmutableList.copyOf(attributes));
+ ImmutableList.copyOf(attributes),
+ /* buildSetting= */ null);
}
private static RuleClass createParentRuleClass() {
@@ -1233,4 +1235,34 @@
.isEqualTo(ExecutionPlatformConstraintsAllowed.PER_TARGET);
assertThat(childRuleClass.hasAttr("exec_compatible_with", LABEL_LIST)).isTrue();
}
+
+ @Test
+ public void testBuildSetting_createsDefaultAttribute() {
+ RuleClass labelFlag =
+ new RuleClass.Builder("label_flag", RuleClassType.NORMAL, false)
+ .factory(DUMMY_CONFIGURED_TARGET_FACTORY)
+ .add(attr("tags", STRING_LIST))
+ .setBuildSetting(new BuildSetting(true, LABEL))
+ .build();
+ RuleClass stringSetting =
+ new RuleClass.Builder("string_setting", RuleClassType.NORMAL, false)
+ .factory(DUMMY_CONFIGURED_TARGET_FACTORY)
+ .add(attr("tags", STRING_LIST))
+ .setBuildSetting(new BuildSetting(false, STRING))
+ .build();
+
+ assertThat(labelFlag.hasAttr(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, LABEL)).isTrue();
+ assertThat(stringSetting.hasAttr(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, STRING)).isTrue();
+ }
+
+ @Test
+ public void testBuildSetting_doesNotCreateDefaultAttributeIfNotBuildSetting() {
+ RuleClass stringSetting =
+ new RuleClass.Builder("non_build_setting", RuleClassType.NORMAL, false)
+ .factory(DUMMY_CONFIGURED_TARGET_FACTORY)
+ .add(attr("tags", STRING_LIST))
+ .build();
+
+ assertThat(stringSetting.hasAttr(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, LABEL)).isFalse();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 7fcda27..8996b5a 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -119,6 +119,7 @@
return parseOptions(
// <== Add new options here in alphabetic order ==>
"--experimental_analysis_testing_improvements=" + rand.nextBoolean(),
+ "--experimental_build_setting_api=" + rand.nextBoolean(),
"--experimental_cc_skylark_api_enabled_packages="
+ rand.nextDouble()
+ ","
@@ -166,6 +167,7 @@
return SkylarkSemantics.builder()
// <== Add new options here in alphabetic order ==>
.experimentalAnalysisTestingImprovements(rand.nextBoolean())
+ .experimentalBuildSettingApi(rand.nextBoolean())
.experimentalCcSkylarkApiEnabledPackages(
ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble())))
.experimentalEnableAndroidMigrationApis(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index abc9c75..56c118e 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -38,6 +38,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.AttributeContainer;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
+import com.google.devtools.build.lib.packages.BuildSetting;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.SkylarkProvider;
import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey;
@@ -50,6 +51,7 @@
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
+import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -2280,6 +2282,110 @@
+ "with for_analysis_testing=True");
}
+
+ @Test
+ public void testBuildSettingRule_flag() throws Exception {
+ setSkylarkSemanticsOptions("--experimental_build_setting_api=true");
+
+ scratch.file("test/rules.bzl",
+ "def _impl(ctx): return None",
+ "build_setting_rule = rule(_impl, build_setting = config.string(flag=True))");
+ scratch.file("test/BUILD",
+ "load('//test:rules.bzl', 'build_setting_rule')",
+ "build_setting_rule(name = 'my_build_setting', build_setting_default = 'default')");
+
+ BuildSetting buildSetting =
+ getTarget("//test:my_build_setting")
+ .getAssociatedRule()
+ .getRuleClassObject()
+ .getBuildSetting();
+
+ assertThat(buildSetting.getType()).isEqualTo(Type.STRING);
+ assertThat(buildSetting.isFlag()).isTrue();
+ }
+
+ @Test
+ public void testBuildSettingRule_settingByDefault() throws Exception {
+ setSkylarkSemanticsOptions("--experimental_build_setting_api=true");
+
+ scratch.file("test/rules.bzl",
+ "def _impl(ctx): return None",
+ "build_setting_rule = rule(_impl, build_setting = config.string())");
+ scratch.file("test/BUILD",
+ "load('//test:rules.bzl', 'build_setting_rule')",
+ "build_setting_rule(name = 'my_build_setting', build_setting_default = 'default')");
+
+ BuildSetting buildSetting =
+ getTarget("//test:my_build_setting")
+ .getAssociatedRule()
+ .getRuleClassObject()
+ .getBuildSetting();
+
+ assertThat(buildSetting.getType()).isEqualTo(Type.STRING);
+ assertThat(buildSetting.isFlag()).isFalse();
+ }
+
+ @Test
+ public void testBuildSettingRule_settingByFlagParameter() throws Exception {
+ setSkylarkSemanticsOptions("--experimental_build_setting_api=true");
+
+ scratch.file("test/rules.bzl",
+ "def _impl(ctx): return None",
+ "build_setting_rule = rule(_impl, build_setting = config.string(flag=False))");
+ scratch.file("test/BUILD",
+ "load('//test:rules.bzl', 'build_setting_rule')",
+ "build_setting_rule(name = 'my_build_setting', build_setting_default = 'default')");
+
+ BuildSetting buildSetting =
+ getTarget("//test:my_build_setting")
+ .getAssociatedRule()
+ .getRuleClassObject()
+ .getBuildSetting();
+
+ assertThat(buildSetting.getType()).isEqualTo(Type.STRING);
+ assertThat(buildSetting.isFlag()).isFalse();
+ }
+
+
+ @Test
+ public void testBuildSettingRule_noDefault() throws Exception {
+ setSkylarkSemanticsOptions("--experimental_build_setting_api=true");
+
+ scratch.file("test/rules.bzl",
+ "def _impl(ctx): return None",
+ "build_setting_rule = rule(_impl, build_setting = config.string())");
+ scratch.file("test/BUILD",
+ "load('//test:rules.bzl', 'build_setting_rule')",
+ "build_setting_rule(name = 'my_build_setting')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:my_build_setting");
+ assertContainsEvent("missing value for mandatory attribute "
+ + "'build_setting_default' in 'build_setting_rule' rule");
+
+ }
+
+ @Test
+ public void testBuildSettingRule_errorsWithoutExperimentalFlag() throws Exception {
+ setSkylarkSemanticsOptions("--experimental_build_setting_api=false");
+
+ scratch.file(
+ "test/rules.bzl",
+ "def _impl(ctx): return None",
+ "build_setting_rule = rule(_impl, build_setting = config.string())");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:rules.bzl', 'build_setting_rule')",
+ "build_setting_rule(name = 'my_build_setting', build_setting_default = 'default')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:my_build_setting");
+ assertContainsEvent(
+ "build_setting parameter is experimental and not "
+ + "available for general use. It is subject to change at any time. It may be enabled "
+ + "by specifying --experimental_build_setting_api");
+ }
+
/**
* Skylark integration test that forces inlining.
*/
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 5521b9c..3e8c104 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -36,7 +36,9 @@
import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext;
import com.google.devtools.build.lib.analysis.util.MockRule;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.SkylarkInfo;
+import com.google.devtools.build.lib.packages.SkylarkProvider;
import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey;
import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
import com.google.devtools.build.lib.packages.StructImpl;
@@ -2206,4 +2208,71 @@
SkylarkList<String> keys = (SkylarkList<String>) keyInfo.getValue("keys");
assertThat(keys).containsExactly("c", "b", "a", "f", "e", "d").inOrder();
}
+
+ private void writeIntFlagBuildSettingFiles() throws Exception {
+ setSkylarkSemanticsOptions("--experimental_build_setting_api=True");
+ scratch.file(
+ "test/build_setting.bzl",
+ "BuildSettingInfo = provider(fields = ['name', 'value'])",
+ "def _impl(ctx):",
+ " return [BuildSettingInfo(name = ctx.attr.name, value = ctx.build_setting_value)]",
+ "",
+ "int_flag = rule(",
+ " implementation = _impl,",
+ " build_setting = config.int(flag = True),",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:build_setting.bzl', 'int_flag')",
+ "int_flag(name = 'int_flag', build_setting_default = 42)");
+ }
+
+ @Test
+ public void testBuildSettingValue_explicitlySet() throws Exception {
+ writeIntFlagBuildSettingFiles();
+ useConfiguration(ImmutableMap.of("//test:int_flag", 24));
+
+ ConfiguredTarget buildSetting = getConfiguredTarget("//test:int_flag");
+ Provider.Key key =
+ new SkylarkProvider.SkylarkKey(
+ Label.create(buildSetting.getLabel().getPackageIdentifier(), "build_setting.bzl"),
+ "BuildSettingInfo");
+ StructImpl buildSettingInfo = (StructImpl) buildSetting.get(key);
+
+ assertThat(buildSettingInfo.getValue("value")).isEqualTo(24);
+ }
+
+ @Test
+ public void testBuildSettingValue_defaultFallback() throws Exception {
+ writeIntFlagBuildSettingFiles();
+
+ ConfiguredTarget buildSetting = getConfiguredTarget("//test:int_flag");
+ Provider.Key key =
+ new SkylarkProvider.SkylarkKey(
+ Label.create(buildSetting.getLabel().getPackageIdentifier(), "build_setting.bzl"),
+ "BuildSettingInfo");
+ StructImpl buildSettingInfo = (StructImpl) buildSetting.get(key);
+
+ assertThat(buildSettingInfo.getValue("value")).isEqualTo(42);
+ }
+
+ @Test
+ public void testBuildSettingValue_nonBuildSettingRule() throws Exception {
+ setSkylarkSemanticsOptions("--experimental_build_setting_api=True");
+ scratch.file(
+ "test/rule.bzl",
+ "def _impl(ctx):",
+ " foo = ctx.build_setting_value",
+ " return []",
+ "non_build_setting = rule(implementation = _impl)");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:rule.bzl', 'non_build_setting')",
+ "non_build_setting(name = 'my_non_build_setting')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:my_non_build_setting");
+ assertContainsEvent("attempting to access 'build_setting_value' of non-build setting "
+ + "//test:my_non_build_setting");
+ }
}
