Add a "canExec" method to SpawnActionContext and SpawnRunner.
This is used by the ProxySpawnActionContext to determine at runtime
whether a spawn strategy can execute a given spawn.
Adds the flag --incompatible_list_based_execution_strategy_selection,
which is used to make this an opt-in change that will be managed by the
incompatible flags process.
The flag's GitHub issue is here:
https://github.com/bazelbuild/bazel/issues/7480
RELNOTES[INC]: The flag --incompatible_list_based_execution_strategy_selection
was added and is used to ease the migration to the new style of specifying
execution strategy selection and fallback behavior. The documentation for
this flag is here: https://github.com/bazelbuild/bazel/issues/7480
PiperOrigin-RevId: 234877574
diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
index e95413b..9759999 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
@@ -77,6 +77,11 @@
}
@Override
+ public boolean canExec(Spawn spawn) {
+ return spawnRunner.canExec(spawn);
+ }
+
+ @Override
public List<SpawnResult> exec(
Spawn spawn,
ActionExecutionContext actionExecutionContext,
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
index 88e36d5..e3099fc 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
@@ -26,10 +26,10 @@
import com.google.devtools.common.options.BoolOrEnumConverter;
import com.google.devtools.common.options.Converters.AssignmentToListOfValuesConverter;
import com.google.devtools.common.options.Converters.CommaSeparatedNonEmptyOptionListConverter;
-import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
+import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.Options;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParsingException;
@@ -58,6 +58,18 @@
public static final ExecutionOptions DEFAULTS = Options.getDefaults(ExecutionOptions.class);
@Option(
+ name = "incompatible_list_based_execution_strategy_selection",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
+ effectTags = {OptionEffectTag.EXECUTION},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help = "See https://github.com/bazelbuild/bazel/issues/7480")
+ public boolean incompatibleListBasedExecutionStrategySelection;
+
+ @Option(
name = "spawn_strategy",
defaultValue = "",
converter = CommaSeparatedNonEmptyOptionListConverter.class,
@@ -73,7 +85,7 @@
@Option(
name = "genrule_strategy",
defaultValue = "",
- converter = CommaSeparatedOptionListConverter.class,
+ converter = CommaSeparatedNonEmptyOptionListConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java b/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java
index 7464e6e..f337b12 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java
@@ -22,21 +22,27 @@
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.events.NullEventHandler;
import java.util.List;
+import java.util.stream.Collectors;
/** Proxy that looks up the right SpawnActionContext for a spawn during {@link #exec}. */
public final class ProxySpawnActionContext implements SpawnActionContext {
private final SpawnActionContextMaps spawnActionContextMaps;
+ private final boolean listBasedExecutionStrategySelection;
/**
* Creates a new {@link ProxySpawnActionContext}.
*
* @param spawnActionContextMaps The {@link SpawnActionContextMaps} to use to decide which {@link
* SpawnActionContext} should execute a given {@link Spawn} during {@link #exec}.
+ * @param listBasedExecutionStrategySelection
*/
- public ProxySpawnActionContext(SpawnActionContextMaps spawnActionContextMaps) {
+ public ProxySpawnActionContext(
+ SpawnActionContextMaps spawnActionContextMaps, boolean listBasedExecutionStrategySelection) {
this.spawnActionContextMaps = spawnActionContextMaps;
+ this.listBasedExecutionStrategySelection = listBasedExecutionStrategySelection;
}
@Override
@@ -44,8 +50,9 @@
throws ExecException, InterruptedException {
List<SpawnActionContext> strategies = resolve(spawn, actionExecutionContext.getEventHandler());
- // For now, we only support executing with the first strategy in the list. Later versions of
- // this code will add some smartness to pick the best out of the list.
+ // Because the strategies are ordered by preference, we can execute the spawn with the best
+ // possible one by simply filtering out the ones that can't execute it and then picking the
+ // first one from the remaining strategies in the list.
return strategies.get(0).exec(spawn, actionExecutionContext);
}
@@ -54,8 +61,9 @@
throws ExecException, InterruptedException {
List<SpawnActionContext> strategies = resolve(spawn, actionExecutionContext.getEventHandler());
- // For now, we only support executing with the first strategy in the list. Later versions of
- // this code will add some smartness to pick the best out of the list.
+ // Because the strategies are ordered by preference, we can execute the spawn with the best
+ // possible one by simply filtering out the ones that can't execute it and then picking the
+ // first one from the remaining strategies in the list.
return strategies.get(0).execMaybeAsync(spawn, actionExecutionContext);
}
@@ -72,6 +80,13 @@
List<SpawnActionContext> strategies =
spawnActionContextMaps.getSpawnActionContexts(spawn, eventHandler);
+ if (listBasedExecutionStrategySelection) {
+ strategies =
+ strategies.stream()
+ .filter(spawnActionContext -> spawnActionContext.canExec(spawn))
+ .collect(Collectors.toList());
+ }
+
if (strategies.isEmpty()) {
throw new UserExecException(
String.format(
@@ -82,4 +97,10 @@
return strategies;
}
+
+ @Override
+ public boolean canExec(Spawn spawn) {
+ return spawnActionContextMaps.getSpawnActionContexts(spawn, NullEventHandler.INSTANCE).stream()
+ .anyMatch(spawnActionContext -> spawnActionContext.canExec(spawn));
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/RegexFilterAssignmentConverter.java b/src/main/java/com/google/devtools/build/lib/exec/RegexFilterAssignmentConverter.java
index 96610bc..2824104 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/RegexFilterAssignmentConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/RegexFilterAssignmentConverter.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.exec;
import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.build.lib.util.RegexFilter.RegexFilterConverter;
@@ -36,6 +37,16 @@
"Must be in the form of a 'regex=value[,value]' assignment");
}
List<String> value = splitter.splitToList(input.substring(pos + 1));
+ if (value.contains("")) {
+ // If the list contains exactly the empty string, it means an empty value was passed and we
+ // should instead return an empty list.
+ if (value.size() == 1) {
+ value = ImmutableList.of();
+ } else {
+ throw new OptionsParsingException(
+ "Values must not contain empty strings or leading / trailing commas");
+ }
+ }
RegexFilter filter = new RegexFilterConverter().convert(input.substring(0, pos));
return Maps.immutableEntry(filter, value);
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
index 2d7918c..b0fd288 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
@@ -73,14 +73,17 @@
private final ImmutableSortedMap<String, List<SpawnActionContext>> mnemonicToSpawnStrategiesMap;
private final ImmutableList<ActionContext> strategies;
private final ImmutableList<RegexFilterSpawnActionContext> spawnStrategyRegexList;
+ private final boolean listBasedExecutionStrategySelection;
private SpawnActionContextMaps(
ImmutableSortedMap<String, List<SpawnActionContext>> mnemonicToSpawnStrategiesMap,
ImmutableList<ActionContext> strategies,
- ImmutableList<RegexFilterSpawnActionContext> spawnStrategyRegexList) {
+ ImmutableList<RegexFilterSpawnActionContext> spawnStrategyRegexList,
+ boolean listBasedExecutionStrategySelection) {
this.mnemonicToSpawnStrategiesMap = mnemonicToSpawnStrategiesMap;
this.strategies = strategies;
this.spawnStrategyRegexList = spawnStrategyRegexList;
+ this.listBasedExecutionStrategySelection = listBasedExecutionStrategySelection;
}
/**
@@ -121,7 +124,9 @@
}
contextMap.put(context.getClass(), context);
}
- contextMap.put(SpawnActionContext.class, new ProxySpawnActionContext(this));
+ contextMap.put(
+ SpawnActionContext.class,
+ new ProxySpawnActionContext(this, listBasedExecutionStrategySelection));
return ImmutableMap.copyOf(contextMap);
}
@@ -185,7 +190,8 @@
return new SpawnActionContextMaps(
ImmutableSortedMap.copyOf(spawnStrategyMnemonicMap, String.CASE_INSENSITIVE_ORDER),
ImmutableList.copyOf(strategies),
- ImmutableList.of());
+ ImmutableList.of(),
+ false);
}
/** A stored entry for a {@link RegexFilter} to {@code strategy} mapping. */
@@ -238,7 +244,9 @@
/** Builds a {@link SpawnActionContextMaps} instance. */
public SpawnActionContextMaps build(
- ImmutableList<ActionContextProvider> actionContextProviders, String testStrategyValue)
+ ImmutableList<ActionContextProvider> actionContextProviders,
+ String testStrategyValue,
+ boolean listBasedExecutionStrategySelection)
throws ExecutorInitException {
StrategyConverter strategyConverter = new StrategyConverter(actionContextProviders);
@@ -250,7 +258,19 @@
for (String mnemonic : strategyByMnemonicMap.keySet()) {
ImmutableList.Builder<SpawnActionContext> contexts = ImmutableList.builder();
- for (String strategy : strategyByMnemonicMap.get(mnemonic)) {
+ Set<String> strategiesForMnemonic = strategyByMnemonicMap.get(mnemonic);
+ if (strategiesForMnemonic.size() > 1 && !listBasedExecutionStrategySelection) {
+ String flagName =
+ mnemonic.isEmpty() ? "--spawn_strategy=" : ("--strategy=" + mnemonic + "=");
+ throw new ExecutorInitException(
+ "Flag "
+ + flagName
+ + Joiner.on(',').join(strategiesForMnemonic)
+ + " contains a list of strategies to use, but "
+ + "--incompatible_list_based_execution_strategy_selection was not enabled.",
+ ExitCode.COMMAND_LINE_ERROR);
+ }
+ for (String strategy : strategiesForMnemonic) {
SpawnActionContext context =
strategyConverter.getStrategy(SpawnActionContext.class, strategy);
if (context == null) {
@@ -284,7 +304,18 @@
for (RegexFilterStrategy entry : strategyByRegexpBuilder.build()) {
ImmutableList.Builder<SpawnActionContext> contexts = ImmutableList.builder();
- for (String strategy : entry.strategy()) {
+ List<String> strategiesForRegex = entry.strategy();
+ if (strategiesForRegex.size() > 1 && !listBasedExecutionStrategySelection) {
+ throw new ExecutorInitException(
+ "Flag --strategy_regexp="
+ + entry.regexFilter().toString()
+ + "="
+ + Joiner.on(',').join(strategiesForRegex)
+ + " contains a list of strategies to use, but "
+ + "--incompatible_list_based_execution_strategy_selection was not enabled.",
+ ExitCode.COMMAND_LINE_ERROR);
+ }
+ for (String strategy : strategiesForRegex) {
SpawnActionContext context =
strategyConverter.getStrategy(SpawnActionContext.class, strategy);
if (context == null) {
@@ -310,7 +341,10 @@
strategies.add(context);
return new SpawnActionContextMaps(
- spawnStrategyMap.build(), strategies.build(), spawnStrategyRegexList.build());
+ spawnStrategyMap.build(),
+ strategies.build(),
+ spawnStrategyRegexList.build(),
+ listBasedExecutionStrategySelection);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
index 0d70563..131915e 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
@@ -232,6 +232,9 @@
SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
throws InterruptedException, IOException, ExecException;
+ /** Returns whether this SpawnRunner supports executing the given Spawn. */
+ boolean canExec(Spawn spawn);
+
/* Name of the SpawnRunner. */
String getName();
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
index 80770b2..04f975d 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
@@ -154,6 +154,11 @@
}
}
+ @Override
+ public boolean canExec(Spawn spawn) {
+ return true;
+ }
+
protected Path createActionTemp(Path execRoot) throws IOException {
return execRoot.getRelative(
java.nio.file.Files.createTempDirectory(
