Define a new Starlark API to create exec transitions.
Currently this is only usable within the builtin rules.
This is intended to reduce the complexity of the StarlarkDefinedConfigTransition, by making it easier to tell if a transition has the extra capabilities of the exec transition.
Work towards composable starlark transitions: #22248. Also part of fixing #22996.
PiperOrigin-RevId: 654886740
Change-Id: I1d5292bc2e5e8646e5563f0a2cd3afd9b2157659
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 1b9dafc..b2ab7c7 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
@@ -122,11 +122,8 @@
return packageContext;
}
- /** Is this transition the same one specified by --experimental_exec_config? */
- public boolean matchesExecConfigFlag(String starlarkExecConfig) {
- return starlarkExecConfig.contains(parentLabel.getPackageName())
- && starlarkExecConfig.contains(parentLabel.getName());
- }
+ /** Is this transition an exec transition? */
+ public abstract boolean isExecTransition();
/**
* Returns a build settings in canonicalized form taking into account repository remappings.
@@ -279,6 +276,18 @@
impl, inputs, outputs, semantics, parentLabel, location, repoMapping);
}
+ public static StarlarkDefinedConfigTransition newExecTransition(
+ StarlarkCallable impl,
+ List<String> inputs,
+ List<String> outputs,
+ StarlarkSemantics semantics,
+ Label parentLabel,
+ Location location,
+ RepositoryMapping repoMapping)
+ throws EvalException {
+ return new ExecTransition(impl, inputs, outputs, semantics, parentLabel, location, repoMapping);
+ }
+
public static StarlarkDefinedConfigTransition newAnalysisTestTransition(
Map<String, Object> changedSettings,
RepositoryMapping repoMapping,
@@ -312,6 +321,11 @@
}
@Override
+ public boolean isExecTransition() {
+ return false;
+ }
+
+ @Override
public ImmutableMap<String, Map<String, Object>> evaluate(
Map<String, Object> previousSettings,
StructImpl attributeMapper,
@@ -370,6 +384,11 @@
return false;
}
+ @Override
+ public boolean isExecTransition() {
+ return false;
+ }
+
/** An exception for validating that a transition is properly constructed */
private static final class UnreadableInputSettingException extends Exception {
UnreadableInputSettingException(String unreadableSetting, Class<?> unreadableClass) {
@@ -740,6 +759,26 @@
}
}
+ /** A transition implementation used only for Starlark-defined exec transitions. */
+ private static class ExecTransition extends RegularTransition {
+ private ExecTransition(
+ StarlarkCallable impl,
+ List<String> inputs,
+ List<String> outputs,
+ StarlarkSemantics semantics,
+ Label parentLabel,
+ Location location,
+ RepositoryMapping repoMapping)
+ throws EvalException {
+ super(impl, inputs, outputs, semantics, parentLabel, location, repoMapping);
+ }
+
+ @Override
+ public boolean isExecTransition() {
+ return true;
+ }
+ }
+
/** An exception for validating that a transition is properly constructed */
public static final class ValidationException extends Exception {
public ValidationException(String message) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java
index 5b530be..753b516 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java
@@ -163,10 +163,7 @@
*/
private static BuildOptions maybeGetExecDefaults(
BuildOptions fromOptions, StarlarkDefinedConfigTransition starlarkTransition) {
- if (starlarkTransition == null
- || fromOptions.get(CoreOptions.class).starlarkExecConfig == null
- || !starlarkTransition.matchesExecConfigFlag(
- fromOptions.get(CoreOptions.class).starlarkExecConfig)) {
+ if (starlarkTransition == null || !starlarkTransition.isExecTransition()) {
// Not an exec transition: the baseline options are just the input options.
return fromOptions;
}
@@ -587,10 +584,7 @@
}
CoreOptions coreOptions = toOptions.get(CoreOptions.class);
- boolean isExecTransition =
- coreOptions.starlarkExecConfig != null
- && starlarkTransition != null
- && starlarkTransition.matchesExecConfigFlag(coreOptions.starlarkExecConfig);
+ boolean isExecTransition = starlarkTransition != null && starlarkTransition.isExecTransition();
if (!isExecTransition
&& coreOptions.outputDirectoryNamingScheme.equals(
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 bedbbe6..3c5742a 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
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.cmdline.BazelModuleContext;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.packages.BuiltinRestriction;
import com.google.devtools.build.lib.starlarkbuildapi.config.ConfigGlobalLibraryApi;
import com.google.devtools.build.lib.starlarkbuildapi.config.ConfigurationTransitionApi;
import java.util.HashSet;
@@ -51,18 +52,17 @@
StarlarkThread thread)
throws EvalException {
StarlarkSemantics semantics = thread.getSemantics();
+ BazelModuleContext moduleContext = BazelModuleContext.ofInnermostBzlOrThrow(thread);
+
List<String> inputsList = Sequence.cast(inputs, String.class, "inputs");
List<String> outputsList = Sequence.cast(outputs, String.class, "outputs");
- // TODO(b/288258583): use a more sustainable way of determining if this is an exec transition.
- // Either match the transition name with the value of --experimental_exec_config (maybe passing
- // that info through StarlarkSemantics) or add an "exec = True" parameter to Starlark's
- // transition() function.
- boolean isExecTransition = implementation.getLocation().file().endsWith("_exec_platforms.bzl");
- BazelModuleContext moduleContext = BazelModuleContext.ofInnermostBzlOrThrow(thread);
validateBuildSettingKeys(
- inputsList, Settings.INPUTS, isExecTransition, moduleContext.packageContext());
+ inputsList, Settings.INPUTS, /* isExecTransition= */ false, moduleContext.packageContext());
validateBuildSettingKeys(
- outputsList, Settings.OUTPUTS, isExecTransition, moduleContext.packageContext());
+ outputsList,
+ Settings.OUTPUTS,
+ /* isExecTransition= */ false,
+ moduleContext.packageContext());
Location location = thread.getCallerLocation();
return StarlarkDefinedConfigTransition.newRegularTransition(
implementation,
@@ -74,15 +74,49 @@
moduleContext.repoMapping());
}
+ @Override
+ public ConfigurationTransitionApi execTransition(
+ StarlarkCallable implementation,
+ Sequence<?> inputs, // <String> expected
+ Sequence<?> outputs, // <String> expected
+ StarlarkThread thread)
+ throws EvalException {
+ // TODO: blaze-configurability-team - When we relax this, add checks that regular usages of
+ // transitions (ie, rule and attribute) cannot use exec_transition.
+ BuiltinRestriction.failIfCalledOutsideBuiltins(thread);
+
+ StarlarkSemantics semantics = thread.getSemantics();
+ BazelModuleContext moduleContext = BazelModuleContext.ofInnermostBzlOrThrow(thread);
+
+ List<String> inputsList = Sequence.cast(inputs, String.class, "inputs");
+ List<String> outputsList = Sequence.cast(outputs, String.class, "outputs");
+ validateBuildSettingKeys(
+ inputsList, Settings.INPUTS, /* isExecTransition= */ true, moduleContext.packageContext());
+ validateBuildSettingKeys(
+ outputsList,
+ Settings.OUTPUTS,
+ /* isExecTransition= */ true,
+ moduleContext.packageContext());
+ Location location = thread.getCallerLocation();
+ return StarlarkDefinedConfigTransition.newExecTransition(
+ implementation,
+ inputsList,
+ outputsList,
+ semantics,
+ moduleContext.label(),
+ location,
+ moduleContext.repoMapping());
+ }
+
// TODO(b/237422931): move into testing module
@Override
public ConfigurationTransitionApi analysisTestTransition(
Dict<?, ?> changedSettings, // <String, String> expected
StarlarkThread thread)
throws EvalException {
+ BazelModuleContext moduleContext = BazelModuleContext.ofInnermostBzlOrThrow(thread);
Map<String, Object> changedSettingsMap =
Dict.cast(changedSettings, String.class, Object.class, "changed_settings dict");
- BazelModuleContext moduleContext = BazelModuleContext.ofInnermostBzlOrThrow(thread);
validateBuildSettingKeys(
changedSettingsMap.keySet(),
Settings.OUTPUTS,
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/config/ConfigGlobalLibraryApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/config/ConfigGlobalLibraryApi.java
index df710d5..ca303e3 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/config/ConfigGlobalLibraryApi.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/config/ConfigGlobalLibraryApi.java
@@ -128,4 +128,32 @@
Dict<?, ?> changedSettings, // <String, String> expected
StarlarkThread thread)
throws EvalException;
+
+ @StarlarkMethod(
+ name = "exec_transition",
+ doc =
+ "A specialized version of <a"
+ + " href=\"../builtins/transition.html\"><code>transition()</code></a> used to define"
+ + " the exec transition. See its documentation (or its implementation) for best"
+ + " practices. Only usable from the Bazel builtins.",
+ parameters = {
+ @Param(name = "implementation", positional = false, named = true),
+ @Param(
+ name = "inputs",
+ allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)},
+ positional = false,
+ named = true),
+ @Param(
+ name = "outputs",
+ allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)},
+ positional = false,
+ named = true),
+ },
+ useStarlarkThread = true)
+ ConfigurationTransitionApi execTransition(
+ StarlarkCallable implementation,
+ Sequence<?> inputs, // <String> expected
+ Sequence<?> outputs, // <String> expected
+ StarlarkThread thread)
+ throws EvalException;
}
diff --git a/src/main/starlark/builtins_bzl/common/builtin_exec_platforms.bzl b/src/main/starlark/builtins_bzl/common/builtin_exec_platforms.bzl
index 5cfb234..bd4d8c1 100644
--- a/src/main/starlark/builtins_bzl/common/builtin_exec_platforms.bzl
+++ b/src/main/starlark/builtins_bzl/common/builtin_exec_platforms.bzl
@@ -471,7 +471,7 @@
# Bazel's exec transition.
_transition_data = exec_transition(bazel_fragments)
-bazel_exec_transition = _builtins.toplevel.transition(
+bazel_exec_transition = _builtins.toplevel.exec_transition(
implementation = _transition_data.implementation,
inputs = _transition_data.inputs,
outputs = _transition_data.outputs,
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BuiltinsInjectionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/BuiltinsInjectionTest.java
index 0913e57..2281cfa 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/BuiltinsInjectionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/BuiltinsInjectionTest.java
@@ -173,7 +173,7 @@
"--platforms=//minimal_buildenv/platforms:default",
// Since this file tests builtins injection, replace the standard exec transition (which is
// in builtins) with a no-op to avoid interference.
- "--experimental_exec_config=//pkg2:dummy_exec_platforms.bzl%noop_transition");
+ "--experimental_exec_config=//pkg2:dummy_exec_platforms.bzl%noop_exec_transition");
}
@Override
@@ -200,11 +200,18 @@
scratch.overwriteFile("pkg2/BUILD", "");
scratch.file(
"pkg2/dummy_exec_platforms.bzl",
- "noop_transition = transition(",
- " implementation = lambda settings, attr: { '//command_line_option:is exec configuration':"
- + " True },",
- " inputs = [],",
- " outputs = ['//command_line_option:is exec configuration'])");
+ """
+ # Since this isn't in builtins, use `transition`, not `exec_transition`
+ # This is fine, since this is a no-op and doesn't use any of the features that exec
+ # transitions are allowed to use.
+ noop_exec_transition = transition(
+ implementation = lambda settings, attr: {
+ '//command_line_option:is exec configuration': True,
+ },
+ inputs = [],
+ outputs = ['//command_line_option:is exec configuration'],
+ )
+ """);
}
@Override
