De-experimentalize starlark defined *rule class transitions* that *only read native options* and *don't have access to attributes that use select()*.
This necessitates de-experimentalizing the entire transition() function.
Still experimental:
- applying starlark-defined transitions on attributes
- applying starlark-defined transitions on starlark build settings anywhere
Still forbidden:
- reading configured attributes on rule class transitions
Newly forbidden:
- transitioning on native flags that are --experimental_ or --incomptaible_
RELNOTES: None.
PiperOrigin-RevId: 234602410
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
index 5117376..b6768ac 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
@@ -160,12 +160,13 @@
}
}
- private static class RegularTransition extends StarlarkDefinedConfigTransition {
+ /** A transition with a user-defined implementation function. */
+ public static class RegularTransition extends StarlarkDefinedConfigTransition {
private final BaseFunction impl;
private final StarlarkSemantics semantics;
private final StarlarkContext starlarkContext;
- public RegularTransition(
+ RegularTransition(
BaseFunction impl,
List<String> inputs,
List<String> outputs,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
index b50310c..919ea2a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
@@ -278,6 +278,13 @@
if (starlarkDefinedTransition.isForAnalysisTesting()) {
builder.hasAnalysisTestTransition();
} else {
+ if (!env.getSemantics().experimentalStarlarkConfigTransitions()) {
+ throw new EvalException(
+ ast.getLocation(),
+ "Starlark-defined transitions on rule attributes is experimental and disabled by "
+ + "default. This API is in development and subject to change at any time. Use "
+ + "--experimental_starlark_config_transitions to use this experimental API.");
+ }
builder.hasStarlarkDefinedTransition();
}
builder.cfg(new StarlarkAttributeTransitionProvider(starlarkDefinedTransition));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java
index d0a7ab0..6355db8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java
@@ -49,15 +49,10 @@
StarlarkContext context)
throws EvalException {
StarlarkSemantics semantics = env.getSemantics();
- if (!semantics.experimentalStarlarkConfigTransitions()) {
- throw new EvalException(
- location,
- "transition() is experimental and disabled by default. "
- + "This API is in development and subject to change at any time. Use "
- + "--experimental_starlark_config_transitions to use this experimental API.");
- }
- validateBuildSettingKeys(inputs, "input", location);
- validateBuildSettingKeys(outputs, "output", location);
+ validateBuildSettingKeys(
+ inputs, "input", location, semantics.experimentalStarlarkConfigTransitions());
+ validateBuildSettingKeys(
+ outputs, "output", location, semantics.experimentalStarlarkConfigTransitions());
return StarlarkDefinedConfigTransition.newRegularTransition(
implementation, inputs, outputs, semantics, context);
}
@@ -68,18 +63,29 @@
throws EvalException {
Map<String, Object> changedSettingsMap =
changedSettings.getContents(String.class, Object.class, "changed_settings dict");
- validateBuildSettingKeys(changedSettingsMap.keySet(), "output", location);
+ validateBuildSettingKeys(changedSettingsMap.keySet(), "output", location, true);
return StarlarkDefinedConfigTransition.newAnalysisTestTransition(changedSettingsMap, location);
}
private void validateBuildSettingKeys(
- Iterable<String> optionKeys, String keyErrorDescriptor, Location location)
+ Iterable<String> optionKeys,
+ String keyErrorDescriptor,
+ Location location,
+ boolean starlarkTransitionsEnabled)
throws EvalException {
HashSet<String> processedOptions = Sets.newHashSet();
for (String optionKey : optionKeys) {
if (!optionKey.startsWith(COMMAND_LINE_OPTION_PREFIX)) {
+ if (!starlarkTransitionsEnabled) {
+ throw new EvalException(
+ location,
+ "transitions on Starlark-defined build settings is experimental and "
+ + "disabled by default. This API is in development and subject to change at any"
+ + "time. Use --experimental_starlark_config_transitions to use this experimental "
+ + "API.");
+ }
try {
Label.parseAbsoluteUnchecked(optionKey);
} catch (IllegalArgumentException e) {
@@ -91,6 +97,16 @@
keyErrorDescriptor, optionKey),
e);
}
+ } else {
+ String optionName = optionKey.substring(COMMAND_LINE_OPTION_PREFIX.length());
+ if (optionName.startsWith("experimental_") || optionName.startsWith("incompatible_")) {
+ throw new EvalException(
+ location,
+ String.format(
+ "Invalid transition %s '%s'. Cannot transition on --experimental_* or "
+ + "--incompatible_* options",
+ keyErrorDescriptor, optionKey));
+ }
}
if (!processedOptions.add(optionKey)) {
throw new EvalException(location,
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
index aac3675..c2b82d1 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
@@ -337,8 +337,6 @@
defaultValue = "None",
named = true,
positional = false,
- enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_STARLARK_CONFIG_TRANSITION,
- valueWhenDisabled = "None",
doc =
"If set, points to the configuration transition the rule will "
+ "apply to its own configuration before analysis.")
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java
index 5707c85..77b82a1 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java
@@ -348,7 +348,7 @@
"my_transition = transition(implementation = transition_func,",
" inputs = [],",
" outputs = ['//command_line_option:cpu',",
- " '//command_line_option:experimental_strict_java_deps'])",
+ " '//command_line_option:host_cpu'])",
"def impl(ctx): ",
" return []",
"my_rule = rule(",
@@ -369,7 +369,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test/skylark:test");
assertContainsEvent(
- "transition outputs [//command_line_option:experimental_strict_java_deps] were not "
+ "transition outputs [//command_line_option:host_cpu] were not "
+ "defined by transition function");
}
@@ -382,12 +382,12 @@
"test/skylark/my_rule.bzl",
"def transition_func(settings, attr):",
" if (len(settings) != 2",
- " or (not settings['//command_line_option:experimental_strict_java_deps'])",
+ " or (not settings['//command_line_option:host_cpu'])",
" or (not settings['//command_line_option:cpu'])):",
" fail()",
" return {'//command_line_option:cpu': 'k8'}",
"my_transition = transition(implementation = transition_func,",
- " inputs = ['//command_line_option:experimental_strict_java_deps',",
+ " inputs = ['//command_line_option:host_cpu',",
" '//command_line_option:cpu'],",
" outputs = ['//command_line_option:cpu'])",
"def impl(ctx): ",
@@ -747,6 +747,17 @@
}
@Test
+ public void testCannotTransitionWithoutFlag() throws Exception {
+ writeBasicTestFiles();
+ setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=false");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test/skylark:test");
+ assertContainsEvent(
+ "Starlark-defined transitions on rule attributes is experimental and disabled by default");
+ }
+
+ @Test
public void testOptionConversionDynamicMode() throws Exception {
// TODO(waltl): check that dynamic_mode is parsed properly.
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
index dec9ebf..ba35ef9 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
@@ -30,7 +30,6 @@
@Test
public void testOutputOnlyTransition() throws Exception {
- setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=true");
scratch.file(
"test/transitions.bzl",
"def _impl(settings, attr):",
@@ -54,7 +53,6 @@
@Test
public void testInputAndOutputTransition() throws Exception {
- setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=true");
scratch.file(
"test/transitions.bzl",
"def _impl(settings, attr):",
@@ -84,8 +82,7 @@
@Test
public void testBuildSettingCannotTransition() throws Exception {
- setSkylarkSemanticsOptions(
- "--experimental_starlark_config_transitions=true", "--experimental_build_setting_api=true");
+ setSkylarkSemanticsOptions("--experimental_build_setting_api=true");
scratch.file(
"test/transitions.bzl",
"def _impl(settings, attr):",
@@ -112,7 +109,6 @@
@Test
public void testBadCfgInput() throws Exception {
- setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=true");
scratch.file(
"test/rules.bzl",
"def _impl(ctx):",
@@ -131,7 +127,6 @@
@Test
public void testMultipleReturnConfigs() throws Exception {
- setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=true");
scratch.file(
"test/transitions.bzl",
"def _impl(settings, attr):",
@@ -157,7 +152,6 @@
@Test
public void testCanDoBadStuffWithParameterizedTransitionsAndSelects() throws Exception {
- setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=true");
scratch.file(
"test/transitions.bzl",
"def _impl(settings, attr):",
@@ -202,7 +196,6 @@
@Test
public void testLabelTypedAttrReturnsLabelNotDep() throws Exception {
- setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=true");
scratch.file(
"test/transitions.bzl",
"def _impl(settings, attr):",
@@ -240,28 +233,23 @@
}
@Test
- public void testCannotTransitionWithoutFlag() throws Exception {
- setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=false");
+ public void testCannotTransitionOnBuildSettingWithoutFlag() throws Exception {
+ setSkylarkSemanticsOptions(
+ "--experimental_starlark_config_transitions=false", "--experimental_build_setting_api");
scratch.file(
"test/transitions.bzl",
- "def _impl(settings, attr):",
- " return {'//command_line_option:test_arg': ['post-transition']}",
- "my_transition = transition(implementation = _impl, inputs = [],",
- " outputs = ['//command_line_option:test_arg'])");
- scratch.file(
- "test/rules.bzl",
- "load('//test:transitions.bzl', 'my_transition')",
- "def _impl(ctx):",
- " return []",
- "my_rule = rule(implementation = _impl, cfg = my_transition)");
- scratch.file("test/BUILD", "load('//test:rules.bzl', 'my_rule')", "my_rule(name = 'test')");
+ "def _transition_impl(settings, attr):",
+ " return {'//test:cute-animal-fact': 'puffins mate for life'}",
+ "my_transition = transition(",
+ " implementation = _transition_impl,",
+ " inputs = [],",
+ " outputs = ['//test:cute-animal-fact']",
+ ")");
+ writeRulesBuildSettingsAndBUILDforBuildSettingTransitionTests();
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test");
- assertContainsEvent(
- "transition() is experimental and disabled by default. This API is in "
- + "development and subject to change at any time. Use "
- + "--experimental_starlark_config_transitions to use this experimental API.");
+ assertContainsEvent("transitions on Starlark-defined build settings is experimental");
}
private void writeRulesBuildSettingsAndBUILDforBuildSettingTransitionTests() throws Exception {
@@ -414,4 +402,26 @@
getConfiguredTarget("//test");
assertContainsEvent("too many (2) positional arguments in call to _impl(settings)");
}
+
+ @Test
+ public void testCannotTransitionOnExperimentalFlag() throws Exception {
+ scratch.file(
+ "test/transitions.bzl",
+ "def _impl(settings, attr):",
+ " return {'//command_line_option:experimental_build_setting_api': True}",
+ "my_transition = transition(implementation = _impl, inputs = [],",
+ " outputs = ['//command_line_option:experimental_build_setting_api'])");
+ scratch.file(
+ "test/rules.bzl",
+ "load('//test:transitions.bzl', 'my_transition')",
+ "def _impl(ctx):",
+ " return []",
+ "my_rule = rule(implementation = _impl, cfg = my_transition)");
+ scratch.file("test/BUILD", "load('//test:rules.bzl', 'my_rule')", "my_rule(name = 'test')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test");
+ assertContainsEvent("Cannot transition on --experimental_* or --incompatible_* options");
+ }
}
+
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 90c7efa..576f055 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
@@ -1154,31 +1154,6 @@
}
@Test
- public void testTransitionGuardedByExperimentalFlag() throws Exception {
- setSkylarkSemanticsOptions("--experimental_starlark_config_transitions=false");
-
- scratch.file(
- "test/extension.bzl",
- "def custom_rule_impl(ctx):",
- " return []",
- "def transition_func(settings):",
- " return settings",
- "my_transition = transition(implementation = transition_func, inputs = [], outputs = [])",
- "",
- "custom_rule = rule(implementation = custom_rule_impl)");
-
- scratch.file(
- "test/BUILD",
- "load('//test:extension.bzl', 'custom_rule')",
- "",
- "custom_rule(name = 'r')");
-
- reporter.removeHandler(failFastHandler);
- getConfiguredTarget("//test:r");
- assertContainsEvent("transition() is experimental and disabled by default");
- }
-
- @Test
public void testNoOutputListAttrDefault() throws Exception {
setSkylarkSemanticsOptions("--incompatible_no_output_attr_default=true");
@@ -2152,7 +2127,7 @@
@Test
public void testAnalysisTestTransitionOnAnalysisTest() throws Exception {
- useConfiguration("--experimental_strict_java_deps=OFF");
+ useConfiguration("--copt=yeehaw");
scratch.file(
"test/extension.bzl",
@@ -2160,21 +2135,21 @@
"MyDep = provider()",
"",
"def outer_rule_impl(ctx):",
- " return [MyInfo(strict_java_deps = ctx.fragments.java.strict_java_deps),",
+ " return [MyInfo(copts = ctx.fragments.cpp.copts),",
" MyDep(info = ctx.attr.dep[0][MyInfo]),",
" AnalysisTestResultInfo(success = True, message = 'message contents')]",
"def inner_rule_impl(ctx):",
- " return [MyInfo(strict_java_deps = ctx.fragments.java.strict_java_deps)]",
+ " return [MyInfo(copts = ctx.fragments.cpp.copts)]",
"",
"my_transition = analysis_test_transition(",
" settings = {",
- " '//command_line_option:experimental_strict_java_deps' : 'WARN' }",
+ " '//command_line_option:copt' : ['cowabunga'] }",
")",
"inner_rule = rule(implementation = inner_rule_impl,",
- " fragments = ['java'])",
+ " fragments = ['cpp'])",
"outer_rule_test = rule(",
" implementation = outer_rule_impl,",
- " fragments = ['java'],",
+ " fragments = ['cpp'],",
" analysis_test = True,",
" attrs = {",
" 'dep': attr.label(cfg = my_transition),",
@@ -2197,8 +2172,8 @@
StructImpl outerDepInfo = (StructImpl) outerTarget.get(myDepKey);
StructImpl innerInfo = (StructImpl) outerDepInfo.getValue("info");
- assertThat(outerInfo.getValue("strict_java_deps")).isEqualTo("off");
- assertThat(innerInfo.getValue("strict_java_deps")).isEqualTo("warn");
+ assertThat((SkylarkList) outerInfo.getValue("copts")).containsExactly("yeehaw");
+ assertThat((SkylarkList) innerInfo.getValue("copts")).containsExactly("cowabunga");
}
@Test
@@ -2209,7 +2184,7 @@
" return []",
"my_transition = analysis_test_transition(",
" settings = {",
- " '//command_line_option:experimental_strict_java_deps' : 'WARN' }",
+ " '//command_line_option:test_arg' : ['yeehaw'] }",
")",
"",
"custom_rule = rule(",
@@ -2347,7 +2322,7 @@
"",
"my_transition = analysis_test_transition(",
" settings = {",
- " '//command_line_option:experimental_strict_java_deps' : 'WARN' }",
+ " '//command_line_option:test_arg' : ['yeehaw'] }",
")",
"",
"inner_rule_test = rule(",
@@ -2410,7 +2385,7 @@
"",
"my_transition = analysis_test_transition(",
" settings = {",
- " '//command_line_option:experimental_strict_java_deps' : 'WARN' }",
+ " '//command_line_option:test_arg' : ['yeehaw'] }",
")",
"dep_rule = rule(",
" implementation = dep_rule_impl,",
@@ -2506,7 +2481,7 @@
"",
"my_transition = analysis_test_transition(",
" settings = {",
- " '//command_line_option:experimental_strict_java_deps' : 'WARN' }",
+ " '//command_line_option:test_arg' : ['yeehaw'] }",
")",
"dep_rule = rule(",
" implementation = dep_rule_impl,",
