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/actions/SpawnActionContext.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java index 421d8e4..a932a7a 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java
@@ -35,4 +35,7 @@ SpawnResult result = Iterables.getOnlyElement(exec(spawn, actionExecutionContext)); return FutureSpawn.immediate(result); } + + /** Returns whether this SpawnActionContext supports executing the given Spawn. */ + boolean canExec(Spawn spawn); }
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index 40b8887..05d8cb8 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -163,9 +163,14 @@ // independently from each other, for example, to run genrules locally and Java compile action // in prod. Thus, for SpawnActions, we decide the action context to use not only based on the // context class, but also the mnemonic of the action. + ExecutionOptions options = request.getOptions(ExecutionOptions.class); spawnActionContextMaps = - builder.getSpawnActionContextMapsBuilder().build( - actionContextProviders, request.getOptions(ExecutionOptions.class).testStrategy); + builder + .getSpawnActionContextMapsBuilder() + .build( + actionContextProviders, + options.testStrategy, + options.incompatibleListBasedExecutionStrategySelection); } Executor getExecutor() throws ExecutorInitException {
diff --git a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java index 1d6913d0..38802b3 100644 --- a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java
@@ -316,6 +316,13 @@ return dynamicExecutionResult.spawnResults(); } + @Override + public boolean canExec(Spawn spawn) { + return remoteStrategy.canExec(spawn) + || workerStrategy.canExec(spawn) + || localStrategy.canExec(spawn); + } + private void moveFileOutErr(ActionExecutionContext actionExecutionContext, FileOutErr outErr) throws IOException { if (outErr.getOutputPath().exists()) {
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(
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 85d7f08..55d8c58 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -279,6 +279,11 @@ } } + @Override + public boolean canExec(Spawn spawn) { + return Spawns.mayBeExecutedRemotely(spawn); + } + private void maybeWriteParamFilesLocally(Spawn spawn) throws IOException { if (!executionOptions.materializeParamFiles) { return;
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index 19dc642..6bccf16 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; +import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.exec.BinTools; import com.google.devtools.build.lib.exec.ExecutionOptions; @@ -78,6 +79,11 @@ } } + @Override + public boolean canExec(Spawn spawn) { + return Spawns.mayBeSandboxed(spawn); + } + // TODO(laszlocsomor): refactor this class to make `actuallyExec`'s contract clearer: the caller // of `actuallyExec` should not depend on `actuallyExec` calling `runSpawn` because it's easy to // forget to do so in `actuallyExec`'s implementations.
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java index 95ca264..7f48615 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
@@ -26,7 +26,6 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.SpawnResult; -import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent; import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent; @@ -307,12 +306,17 @@ @Override public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws InterruptedException, IOException, ExecException { - if (!Spawns.mayBeSandboxed(spawn)) { - return fallbackSpawnRunner.exec(spawn, context); - } else { + if (sandboxSpawnRunner.canExec(spawn)) { return sandboxSpawnRunner.exec(spawn, context); + } else { + return fallbackSpawnRunner.exec(spawn, context); } } + + @Override + public boolean canExec(Spawn spawn) { + return sandboxSpawnRunner.canExec(spawn) || fallbackSpawnRunner.canExec(spawn); + } } /**
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index 1c8a345..c3ed426 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
@@ -121,6 +121,11 @@ return actuallyExec(spawn, context); } + @Override + public boolean canExec(Spawn spawn) { + return Spawns.supportsWorkers(spawn); + } + private SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException { if (Iterables.isEmpty(spawn.getToolFiles())) {
diff --git a/src/main/java/com/google/devtools/common/options/Converters.java b/src/main/java/com/google/devtools/common/options/Converters.java index a691bd4..6874c7e 100644 --- a/src/main/java/com/google/devtools/common/options/Converters.java +++ b/src/main/java/com/google/devtools/common/options/Converters.java
@@ -268,6 +268,12 @@ List<String> result = input.isEmpty() ? ImmutableList.of() : ImmutableList.copyOf(splitter.split(input)); if (!allowEmptyValues && result.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 (result.size() == 1) { + return ImmutableList.of(); + } + throw new OptionsParsingException( "Empty values are not allowed as part of this " + getTypeDescription()); }
diff --git a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java index 1f3454b..cd24422 100644 --- a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
@@ -102,6 +102,11 @@ } @Override + public boolean canExec(Spawn spawn) { + return true; + } + + @Override public List<SpawnResult> exec( Spawn spawn, ActionExecutionContext actionExecutionContext,
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java index a676256..d757799 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
@@ -74,7 +74,7 @@ public void duplicateMnemonics_bothGetStored() throws Exception { builder.strategyByMnemonicMap().put("Spawn1", "ac1"); builder.strategyByMnemonicMap().put("Spawn1", "ac2"); - SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest"); + SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest", true); List<SpawnActionContext> result = maps.getSpawnActionContexts(mockSpawn("Spawn1", null), reporter); assertThat(result).containsExactly(ac1, ac2); @@ -84,7 +84,7 @@ public void emptyStrategyFallsBackToEmptyMnemonicNotToDefault() throws Exception { builder.strategyByMnemonicMap().put("Spawn1", ""); builder.strategyByMnemonicMap().put("", "ac2"); - SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest"); + SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest", false); List<SpawnActionContext> result = maps.getSpawnActionContexts(mockSpawn("Spawn1", null), reporter); assertThat(result).containsExactly(ac2); @@ -94,7 +94,7 @@ public void multipleRegexps_firstMatchWins() throws Exception { builder.addStrategyByRegexp(converter.convert("foo"), ImmutableList.of("ac1")); builder.addStrategyByRegexp(converter.convert("foo/bar"), ImmutableList.of("ac2")); - SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest"); + SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest", false); List<SpawnActionContext> result = maps.getSpawnActionContexts(mockSpawn(null, "Doing something with foo/bar/baz"), reporter); @@ -106,7 +106,7 @@ public void regexpAndMnemonic_regexpWins() throws Exception { builder.strategyByMnemonicMap().put("Spawn1", "ac1"); builder.addStrategyByRegexp(converter.convert("foo/bar"), ImmutableList.of("ac2")); - SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest"); + SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest", false); List<SpawnActionContext> result = maps.getSpawnActionContexts( @@ -138,6 +138,11 @@ throws ExecException, InterruptedException { throw new UnsupportedOperationException(); } + + @Override + public boolean canExec(Spawn spawn) { + return true; + } } @ExecutionStrategy(contextType = SpawnActionContext.class, name = "ac2") @@ -147,6 +152,11 @@ throws ExecException, InterruptedException { throw new UnsupportedOperationException(); } + + @Override + public boolean canExec(Spawn spawn) { + return true; + } } @ExecutionStrategy(contextType = TestActionContext.class, name = "actest")
diff --git a/src/test/shell/integration/execution_strategies_test.sh b/src/test/shell/integration/execution_strategies_test.sh index 934c098..6cf70ad 100755 --- a/src/test/shell/integration/execution_strategies_test.sh +++ b/src/test/shell/integration/execution_strategies_test.sh
@@ -44,27 +44,37 @@ source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ || { echo "integration_test_setup.sh not found!" >&2; exit 1; } +# Tests that you have to opt-in to list based strategy selection via an incompatible flag. +function test_incompatible_flag_required() { + bazel build --spawn_strategy=worker,local --debug_print_action_contexts &> $TEST_log || true + expect_log "incompatible_list_based_execution_strategy_selection was not enabled" +} + # Tests that you can set the spawn strategy flags to a list of strategies. function test_multiple_strategies() { - bazel build --spawn_strategy=worker,local --debug_print_action_contexts &> $TEST_log || fail + bazel build --incompatible_list_based_execution_strategy_selection \ + --spawn_strategy=worker,local --debug_print_action_contexts &> $TEST_log || fail # Can't test for exact strategy names here, because they differ between platforms and products. expect_log "\"\" = \[.*, .*\]" } # Tests that Bazel catches an invalid strategy list that has an empty string as an element. function test_empty_strategy_in_list_is_forbidden() { - bazel build --spawn_strategy=worker,,local --debug_print_action_contexts &> $TEST_log || true + bazel build --incompatible_list_based_execution_strategy_selection \ + --spawn_strategy=worker,,local --debug_print_action_contexts &> $TEST_log || true expect_log "--spawn_strategy=worker,,local: Empty values are not allowed as part of this comma-separated list of options" } # Test that when you set a strategy to the empty string, it gets removed from the map of strategies # and thus results in the default strategy being used (the one set via --spawn_strategy=). function test_empty_strategy_means_default() { - bazel build --spawn_strategy=worker,local --strategy=FooBar=local \ + bazel build --incompatible_list_based_execution_strategy_selection \ + --spawn_strategy=worker,local --strategy=FooBar=local \ --debug_print_action_contexts &> $TEST_log || fail expect_log "\"FooBar\" = " - bazel build --spawn_strategy=worker,local --strategy=FooBar=local --strategy=FooBar= \ + bazel build --incompatible_list_based_execution_strategy_selection \ + --spawn_strategy=worker,local --strategy=FooBar=local --strategy=FooBar= \ --debug_print_action_contexts &> $TEST_log || fail expect_not_log "\"FooBar\" = " }