Allow passing a comma-separated list of strategies to the strategy flags.
Any strategy except the first one in a list is currently ignored. This
feature will be used in follow up changes to implement a new way of
specifying execution strategies and to improve the configurability of
execution fallback (e.g. remote to local, sandboxed to non-sandboxed).
RELNOTES: None.
PiperOrigin-RevId: 234849292
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java
index b5236a2..6b3712c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.common.options.OptionsBase;
+import java.util.List;
import java.util.Map;
/** Module which registers the strategy options for Bazel. */
@@ -47,27 +48,26 @@
ExecutionOptions options = env.getOptions().getOptions(ExecutionOptions.class);
// Default strategies for certain mnemonics - they can be overridden by --strategy= flags.
- builder.addStrategyByMnemonic("Javac", "worker");
- builder.addStrategyByMnemonic("Closure", "worker");
- builder.addStrategyByMnemonic("DexBuilder", "worker");
+ builder.addStrategyByMnemonic("Javac", ImmutableList.of("worker"));
+ builder.addStrategyByMnemonic("Closure", ImmutableList.of("worker"));
+ builder.addStrategyByMnemonic("DexBuilder", ImmutableList.of("worker"));
// Allow genrule_strategy to also be overridden by --strategy= flags.
builder.addStrategyByMnemonic("Genrule", options.genruleStrategy);
- for (Map.Entry<String, String> strategy : options.strategy) {
- String strategyName = strategy.getValue();
- // TODO(philwo) - remove this when the standalone / local mess is cleaned up.
- // Some flag expansions use "local" as the strategy name, but the strategy is now called
- // "standalone", so we'll translate it here.
- if (strategyName.equals("local")) {
- strategyName = "standalone";
- }
- builder.addStrategyByMnemonic(strategy.getKey(), strategyName);
+ for (Map.Entry<String, List<String>> strategy : options.strategy) {
+ builder.addStrategyByMnemonic(strategy.getKey(), strategy.getValue());
}
- builder.addStrategyByMnemonic("", options.spawnStrategy);
+ // The --spawn_strategy= flag is a bit special: If it's set to the empty string, we actually
+ // have to pass a literal empty string to the builder to trigger the "use the strategy that was
+ // registered last" behavior. Otherwise we would have no default strategy at all and Bazel would
+ // crash.
+ List<String> spawnStrategies =
+ options.spawnStrategy.isEmpty() ? ImmutableList.of("") : options.spawnStrategy;
+ builder.addStrategyByMnemonic("", spawnStrategies);
- for (Map.Entry<RegexFilter, String> entry : options.strategyByRegexp) {
+ for (Map.Entry<RegexFilter, List<String>> entry : options.strategyByRegexp) {
builder.addStrategyByRegexp(entry.getKey(), entry.getValue());
}
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 c8444fd..88e36d5 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
@@ -24,7 +24,9 @@
import com.google.devtools.build.lib.util.ResourceConverter;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.common.options.BoolOrEnumConverter;
-import com.google.devtools.common.options.Converters.AssignmentConverter;
+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;
@@ -58,6 +60,7 @@
@Option(
name = "spawn_strategy",
defaultValue = "",
+ converter = CommaSeparatedNonEmptyOptionListConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
@@ -65,23 +68,24 @@
+ "'standalone' means run all of them locally without any kind of sandboxing. "
+ "'sandboxed' means to run them in a sandboxed environment with limited privileges "
+ "(details depend on platform support).")
- public String spawnStrategy;
+ public List<String> spawnStrategy;
@Option(
name = "genrule_strategy",
defaultValue = "",
+ converter = CommaSeparatedOptionListConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Specify how to execute genrules. This flag will be phased out. Instead, use "
+ "--spawn_strategy=<value> to control all actions or --strategy=Genrule=<value> "
+ "to control genrules only.")
- public String genruleStrategy;
+ public List<String> genruleStrategy;
@Option(
name = "strategy",
allowMultiple = true,
- converter = AssignmentConverter.class,
+ converter = AssignmentToListOfValuesConverter.class,
defaultValue = "",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
@@ -89,7 +93,7 @@
"Specify how to distribute compilation of other spawn actions. "
+ "Example: 'Javac=local' means to spawn Java compilation locally. "
+ "'JavaIjar=sandboxed' means to spawn Java Ijar actions in a sandbox. ")
- public List<Map.Entry<String, String>> strategy;
+ public List<Map.Entry<String, List<String>>> strategy;
@Option(
name = "strategy_regexp",
@@ -109,7 +113,7 @@
+ "Example: --strategy_regexp='Compiling.*/bar=local "
+ " --strategy_regexp=Compiling=sandboxed will run 'Compiling //foo/bar/baz' with "
+ "the 'local' strategy, but reversing the order would run it with 'sandboxed'. ")
- public List<Map.Entry<RegexFilter, String>> strategyByRegexp;
+ public List<Map.Entry<RegexFilter, List<String>>> strategyByRegexp;
@Option(
name = "materialize_param_files",
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutorBuilder.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutorBuilder.java
index 73339de..e478088 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutorBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutorBuilder.java
@@ -63,21 +63,13 @@
}
/**
- * Sets the strategy name for a given action mnemonic.
+ * Sets the strategy names for a given action mnemonic.
*
- * <p>The calling module can either decide for itself which implementation is needed and make the
- * value associated with this key a constant or defer that decision to the user, for example, by
- * providing a command line option and setting the value in the map based on that.
- *
- * <p>Setting the strategy to the empty string "" redirects it to the value for the empty
- * mnemonic.
- *
- * <p>Example: a module requires {@code SpawnActionContext} to do its job, and it creates actions
- * with the mnemonic <code>C++</code>. The the module can call
- * <code>addStrategyByMnemonic("C++", strategy)</code>.
+ * <p>During execution, the {@link ProxySpawnActionContext} will ask each strategy whether it can
+ * execute a given Spawn. The first strategy in the list that says so will get the job.
*/
- public ExecutorBuilder addStrategyByMnemonic(String mnemonic, String strategy) {
- spawnActionContextMapsBuilder.strategyByMnemonicMap().put(mnemonic, strategy);
+ public ExecutorBuilder addStrategyByMnemonic(String mnemonic, List<String> strategies) {
+ spawnActionContextMapsBuilder.strategyByMnemonicMap().replaceValues(mnemonic, strategies);
return this;
}
@@ -106,7 +98,7 @@
* Similar to {@link #addStrategyByMnemonic}, but allows specifying a regex for the set of
* matching mnemonics, instead of an exact string.
*/
- public ExecutorBuilder addStrategyByRegexp(RegexFilter regexFilter, String strategy) {
+ public ExecutorBuilder addStrategyByRegexp(RegexFilter regexFilter, List<String> strategy) {
spawnActionContextMapsBuilder.addStrategyByRegexp(regexFilter, strategy);
return this;
}
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 143e605..7464e6e 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
@@ -20,6 +20,7 @@
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.UserExecException;
import com.google.devtools.build.lib.events.EventHandler;
import java.util.List;
@@ -41,26 +42,44 @@
@Override
public List<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
- return resolve(spawn, actionExecutionContext.getEventHandler())
- .exec(spawn, actionExecutionContext);
+ 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.
+ return strategies.get(0).exec(spawn, actionExecutionContext);
}
@Override
public FutureSpawn execMaybeAsync(Spawn spawn, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
- return resolve(spawn, actionExecutionContext.getEventHandler())
- .execMaybeAsync(spawn, actionExecutionContext);
+ 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.
+ return strategies.get(0).execMaybeAsync(spawn, actionExecutionContext);
}
/**
- * Returns the {@link SpawnActionContext} that should be used to execute the given spawn.
+ * Returns the list of {@link SpawnActionContext}s that should be used to execute the given spawn.
*
* @param spawn The spawn for which the correct {@link SpawnActionContext} should be determined.
* @param eventHandler An event handler that can be used to print messages while resolving the
* correct {@link SpawnActionContext} for the given spawn.
*/
@VisibleForTesting
- public SpawnActionContext resolve(Spawn spawn, EventHandler eventHandler) {
- return spawnActionContextMaps.getSpawnActionContext(spawn, eventHandler);
+ public List<SpawnActionContext> resolve(Spawn spawn, EventHandler eventHandler)
+ throws UserExecException {
+ List<SpawnActionContext> strategies =
+ spawnActionContextMaps.getSpawnActionContexts(spawn, eventHandler);
+
+ if (strategies.isEmpty()) {
+ throw new UserExecException(
+ String.format(
+ "No usable spawn strategy found for spawn with mnemonic %s. Are your --spawn_strategy"
+ + " or --strategy flags too strict?",
+ spawn.getMnemonic()));
+ }
+
+ return strategies;
}
}
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 65b60b4..96610bc 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
@@ -13,29 +13,35 @@
// limitations under the License.
package com.google.devtools.build.lib.exec;
+import com.google.common.base.Splitter;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.build.lib.util.RegexFilter.RegexFilterConverter;
import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.OptionsParsingException;
+import java.util.List;
import java.util.Map;
/** A converter for options of the form RegexFilter=String. */
-public class RegexFilterAssignmentConverter implements Converter<Map.Entry<RegexFilter, String>> {
+public class RegexFilterAssignmentConverter
+ implements Converter<Map.Entry<RegexFilter, List<String>>> {
+
+ private final Splitter splitter = Splitter.on(',');
@Override
- public Map.Entry<RegexFilter, String> convert(String input) throws OptionsParsingException {
+ public Map.Entry<RegexFilter, List<String>> convert(String input) throws OptionsParsingException {
int pos = input.indexOf('=');
if (pos <= 0) {
- throw new OptionsParsingException("Must be in the form of a 'regex=value' assignment");
+ throw new OptionsParsingException(
+ "Must be in the form of a 'regex=value[,value]' assignment");
}
- String value = input.substring(pos + 1);
+ List<String> value = splitter.splitToList(input.substring(pos + 1));
RegexFilter filter = new RegexFilterConverter().convert(input.substring(0, pos));
return Maps.immutableEntry(filter, value);
}
@Override
public String getTypeDescription() {
- return "a '<RegexFilter>=value' assignment";
+ return "a '<RegexFilter>=value[,value]' assignment";
}
}
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 21d1215..2d7918c 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
@@ -24,7 +24,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSortedMap;
-import com.google.common.collect.Iterables;
+import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Ordering;
import com.google.common.collect.Table;
import com.google.devtools.build.lib.actions.ActionContext;
@@ -45,8 +45,10 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
import java.util.TreeMap;
+import java.util.stream.Collectors;
/**
* Container for looking up the {@link ActionContext} to use for a given action.
@@ -65,51 +67,52 @@
public abstract static class RegexFilterSpawnActionContext {
public abstract RegexFilter regexFilter();
- public abstract SpawnActionContext spawnActionContext();
+ public abstract ImmutableList<SpawnActionContext> spawnActionContext();
}
- private final ImmutableSortedMap<String, SpawnActionContext> spawnStrategyMnemonicMap;
+ private final ImmutableSortedMap<String, List<SpawnActionContext>> mnemonicToSpawnStrategiesMap;
private final ImmutableList<ActionContext> strategies;
private final ImmutableList<RegexFilterSpawnActionContext> spawnStrategyRegexList;
private SpawnActionContextMaps(
- ImmutableSortedMap<String, SpawnActionContext> spawnStrategyMnemonicMap,
+ ImmutableSortedMap<String, List<SpawnActionContext>> mnemonicToSpawnStrategiesMap,
ImmutableList<ActionContext> strategies,
ImmutableList<RegexFilterSpawnActionContext> spawnStrategyRegexList) {
- this.spawnStrategyMnemonicMap = spawnStrategyMnemonicMap;
+ this.mnemonicToSpawnStrategiesMap = mnemonicToSpawnStrategiesMap;
this.strategies = strategies;
this.spawnStrategyRegexList = spawnStrategyRegexList;
}
/**
- * Returns the appropriate {@link ActionContext} to execute the given {@link Spawn} with.
+ * Returns a list of appropriate {@link ActionContext}s to execute the given {@link Spawn} with.
*
* <p>If the reason for selecting the context is worth mentioning to the user, logs a message
* using the given {@link Reporter}.
*/
- public SpawnActionContext getSpawnActionContext(Spawn spawn, EventHandler reporter) {
+ List<SpawnActionContext> getSpawnActionContexts(Spawn spawn, EventHandler reporter) {
Preconditions.checkNotNull(spawn);
if (!spawnStrategyRegexList.isEmpty() && spawn.getResourceOwner() != null) {
String description = spawn.getResourceOwner().getProgressMessage();
if (description != null) {
for (RegexFilterSpawnActionContext entry : spawnStrategyRegexList) {
if (entry.regexFilter().isIncluded(description) && entry.spawnActionContext() != null) {
- reporter.handle(Event.progress(
- description + " with context " + entry.spawnActionContext().toString()));
+ reporter.handle(
+ Event.progress(
+ description + " with context " + entry.spawnActionContext().toString()));
return entry.spawnActionContext();
}
}
}
}
- SpawnActionContext context = spawnStrategyMnemonicMap.get(spawn.getMnemonic());
+ List<SpawnActionContext> context = mnemonicToSpawnStrategiesMap.get(spawn.getMnemonic());
if (context != null) {
return context;
}
- return Preconditions.checkNotNull(spawnStrategyMnemonicMap.get(""));
+ return Preconditions.checkNotNull(mnemonicToSpawnStrategiesMap.get(""));
}
/** Returns a map from action context class to its instantiated context object. */
- public ImmutableMap<Class<? extends ActionContext>, ActionContext> contextMap() {
+ ImmutableMap<Class<? extends ActionContext>, ActionContext> contextMap() {
Map<Class<? extends ActionContext>, ActionContext> contextMap = new HashMap<>();
for (ActionContext context : strategies) {
ExecutionStrategy annotation = context.getClass().getAnnotation(ExecutionStrategy.class);
@@ -123,13 +126,13 @@
}
/** Returns a list of all referenced {@link ActionContext} instances. */
- public ImmutableList<ActionContext> allContexts() {
+ ImmutableList<ActionContext> allContexts() {
// We need to keep only the last occurrences of the entries in contextImplementations
// (so we respect insertion order but also instantiate them only once).
LinkedHashSet<ActionContext> allContexts = new LinkedHashSet<>();
allContexts.addAll(strategies);
- allContexts.addAll(spawnStrategyMnemonicMap.values());
- spawnStrategyRegexList.forEach(x -> allContexts.add(x.spawnActionContext()));
+ mnemonicToSpawnStrategiesMap.values().forEach(allContexts::addAll);
+ spawnStrategyRegexList.forEach(x -> allContexts.addAll(x.spawnActionContext()));
return ImmutableList.copyOf(allContexts);
}
@@ -138,13 +141,17 @@
*
* <p>Prints out debug information about the mappings.
*/
- public void debugPrintSpawnActionContextMaps(Reporter reporter) {
- for (Map.Entry<String, SpawnActionContext> entry : spawnStrategyMnemonicMap.entrySet()) {
+ void debugPrintSpawnActionContextMaps(Reporter reporter) {
+ for (Entry<String, List<SpawnActionContext>> entry : mnemonicToSpawnStrategiesMap.entrySet()) {
+ List<String> strategyNames =
+ entry.getValue().stream()
+ .map(spawnActionContext -> spawnActionContext.getClass().getSimpleName())
+ .collect(Collectors.toList());
reporter.handle(
Event.info(
String.format(
- "SpawnActionContextMap: \"%s\" = %s",
- entry.getKey(), entry.getValue().getClass().getSimpleName())));
+ "SpawnActionContextMap: \"%s\" = [%s]",
+ entry.getKey(), Joiner.on(", ").join(strategyNames))));
}
ImmutableMap<Class<? extends ActionContext>, ActionContext> contextMap = contextMap();
@@ -173,11 +180,10 @@
@VisibleForTesting
public static SpawnActionContextMaps createStub(
- List<ActionContext> strategies, Map<String, SpawnActionContext> spawnStrategyMnemonicMap) {
+ List<ActionContext> strategies,
+ Map<String, List<SpawnActionContext>> spawnStrategyMnemonicMap) {
return new SpawnActionContextMaps(
- ImmutableSortedMap.<String, SpawnActionContext>orderedBy(String.CASE_INSENSITIVE_ORDER)
- .putAll(spawnStrategyMnemonicMap)
- .build(),
+ ImmutableSortedMap.copyOf(spawnStrategyMnemonicMap, String.CASE_INSENSITIVE_ORDER),
ImmutableList.copyOf(strategies),
ImmutableList.of());
}
@@ -187,13 +193,13 @@
public abstract static class RegexFilterStrategy {
public abstract RegexFilter regexFilter();
- public abstract String strategy();
+ public abstract ImmutableList<String> strategy();
}
/** Builder for {@code SpawnActionContextMaps}. */
public static final class Builder {
- private ImmutableListMultimap.Builder<String, String> strategyByMnemonicMapBuilder =
- ImmutableListMultimap.builder();
+ private final LinkedHashMultimap<String, String> strategyByMnemonicMap =
+ LinkedHashMultimap.create();
private ImmutableListMultimap.Builder<Class<? extends ActionContext>, String>
strategyByContextMapBuilder = ImmutableListMultimap.builder();
@@ -206,17 +212,17 @@
* <p>If a spawn action is executed whose mnemonic maps to the empty string or is not present in
* the map at all, the choice of the implementation is left to Blaze.
*
- * <p>Matching on mnemonics is done case-insensitively so it is recommended that any
- * module makes sure that no two strategies refer to the same mnemonic. If they do, Blaze
- * will pick the last one added.
+ * <p>Matching on mnemonics is done case-insensitively so it is recommended that any module
+ * makes sure that no two strategies refer to the same mnemonic. If they do, Blaze will pick the
+ * last one added.
*/
- public ImmutableMultimap.Builder<String, String> strategyByMnemonicMap() {
- return strategyByMnemonicMapBuilder;
+ public LinkedHashMultimap<String, String> strategyByMnemonicMap() {
+ return strategyByMnemonicMap;
}
/**
- * Returns a builder modules can use to associate {@link ActionContext} classes with
- * strategy names.
+ * Returns a builder modules can use to associate {@link ActionContext} classes with strategy
+ * names.
*/
public ImmutableMultimap.Builder<Class<? extends ActionContext>, String>
strategyByContextMap() {
@@ -224,9 +230,10 @@
}
/** Adds a mapping from the given {@link RegexFilter} to a {@code strategy}. */
- public void addStrategyByRegexp(RegexFilter regexFilter, String strategy) {
+ public void addStrategyByRegexp(RegexFilter regexFilter, List<String> strategy) {
strategyByRegexpBuilder.add(
- new AutoValue_SpawnActionContextMaps_RegexFilterStrategy(regexFilter, strategy));
+ new AutoValue_SpawnActionContextMaps_RegexFilterStrategy(
+ regexFilter, ImmutableList.copyOf(strategy)));
}
/** Builds a {@link SpawnActionContextMaps} instance. */
@@ -235,32 +242,27 @@
throws ExecutorInitException {
StrategyConverter strategyConverter = new StrategyConverter(actionContextProviders);
- ImmutableSortedMap.Builder<String, SpawnActionContext> spawnStrategyMap =
+ ImmutableSortedMap.Builder<String, List<SpawnActionContext>> spawnStrategyMap =
ImmutableSortedMap.orderedBy(String.CASE_INSENSITIVE_ORDER);
ImmutableList.Builder<ActionContext> strategies = ImmutableList.builder();
ImmutableList.Builder<RegexFilterSpawnActionContext> spawnStrategyRegexList =
ImmutableList.builder();
- ImmutableListMultimap<String, String> multimap = strategyByMnemonicMapBuilder.build();
- for (String mnemonic : multimap.keySet()) {
- String strategy = Iterables.getLast(multimap.get(mnemonic));
- if (strategy.isEmpty() && !mnemonic.isEmpty()) {
- // If strategy is set to the empty value, and we're not looking at the empty mnemonic,
- // then don't create a per-mnemonic entry for this case at all. At runtime, we'll fall
- // back to the setting for the empty mnemonic, which may or may not be the setting for
- // the "" strategy (the default strategy), but may be overridden to a specific one.
- continue;
+ for (String mnemonic : strategyByMnemonicMap.keySet()) {
+ ImmutableList.Builder<SpawnActionContext> contexts = ImmutableList.builder();
+ for (String strategy : strategyByMnemonicMap.get(mnemonic)) {
+ SpawnActionContext context =
+ strategyConverter.getStrategy(SpawnActionContext.class, strategy);
+ if (context == null) {
+ String strategyOrNull = Strings.emptyToNull(strategy);
+ throw makeExceptionForInvalidStrategyValue(
+ strategy,
+ Joiner.on(' ').skipNulls().join(strategyOrNull, "spawn"),
+ strategyConverter.getValidValues(SpawnActionContext.class));
+ }
+ contexts.add(context);
}
- SpawnActionContext context =
- strategyConverter.getStrategy(SpawnActionContext.class, strategy);
- if (context == null) {
- String strategyOrNull = Strings.emptyToNull(strategy);
- throw makeExceptionForInvalidStrategyValue(
- strategy,
- Joiner.on(' ').skipNulls().join(strategyOrNull, "spawn"),
- strategyConverter.getValidValues(SpawnActionContext.class));
- }
- spawnStrategyMap.put(mnemonic, context);
+ spawnStrategyMap.put(mnemonic, contexts.build());
}
Set<ActionContext> seenContext = new HashSet<>();
@@ -281,19 +283,22 @@
}
for (RegexFilterStrategy entry : strategyByRegexpBuilder.build()) {
- SpawnActionContext context =
- strategyConverter.getStrategy(SpawnActionContext.class, entry.strategy());
- if (context == null) {
- String strategy = Strings.emptyToNull(entry.strategy().toString());
- throw makeExceptionForInvalidStrategyValue(
- entry.regexFilter().toString(),
- Joiner.on(' ').skipNulls().join(strategy, "spawn"),
- strategyConverter.getValidValues(SpawnActionContext.class));
+ ImmutableList.Builder<SpawnActionContext> contexts = ImmutableList.builder();
+ for (String strategy : entry.strategy()) {
+ SpawnActionContext context =
+ strategyConverter.getStrategy(SpawnActionContext.class, strategy);
+ if (context == null) {
+ strategy = Strings.emptyToNull(strategy);
+ throw makeExceptionForInvalidStrategyValue(
+ entry.regexFilter().toString(),
+ Joiner.on(' ').skipNulls().join(strategy, "spawn"),
+ strategyConverter.getValidValues(SpawnActionContext.class));
+ }
+ contexts.add(context);
}
-
spawnStrategyRegexList.add(
new AutoValue_SpawnActionContextMaps_RegexFilterSpawnActionContext(
- entry.regexFilter(), context));
+ entry.regexFilter(), contexts.build()));
}
ActionContext context =
@@ -355,5 +360,4 @@
return marker != null ? marker.name() : context.getSimpleName();
}
}
-
}
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 072e821..95ca264 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
@@ -238,7 +238,7 @@
// This makes the "sandboxed" strategy the default Spawn strategy, unless it is
// overridden by a later BlazeModule.
- builder.addStrategyByMnemonic("", "sandboxed");
+ builder.addStrategyByMnemonic("", ImmutableList.of("sandboxed"));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/BUILD b/src/main/java/com/google/devtools/build/lib/standalone/BUILD
index 69cc4a5..c1a479d 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/standalone/BUILD
@@ -26,6 +26,7 @@
"//src/main/java/com/google/devtools/build/lib/exec/local:options",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/lib/vfs",
+ "//third_party:guava",
"//third_party:jsr305",
],
)
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
index 1393c20..1c44b3a 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.standalone;
+import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.analysis.actions.LocalTemplateExpansionStrategy;
@@ -69,7 +70,7 @@
// This makes the "sandboxed" strategy the default Spawn strategy, unless it is overridden by a
// later BlazeModule.
- builder.addStrategyByMnemonic("", "standalone");
+ builder.addStrategyByMnemonic("", ImmutableList.of("standalone"));
// This makes the "standalone" strategy available via --spawn_strategy=standalone, but it is not
// necessarily the default.
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 fb3bbfa..a691bd4 100644
--- a/src/main/java/com/google/devtools/common/options/Converters.java
+++ b/src/main/java/com/google/devtools/common/options/Converters.java
@@ -254,15 +254,24 @@
private final String separatorDescription;
private final Splitter splitter;
+ private final boolean allowEmptyValues;
- protected SeparatedOptionListConverter(char separator, String separatorDescription) {
+ protected SeparatedOptionListConverter(
+ char separator, String separatorDescription, boolean allowEmptyValues) {
this.separatorDescription = separatorDescription;
this.splitter = Splitter.on(separator);
+ this.allowEmptyValues = allowEmptyValues;
}
@Override
- public List<String> convert(String input) {
- return input.isEmpty() ? ImmutableList.of() : ImmutableList.copyOf(splitter.split(input));
+ public List<String> convert(String input) throws OptionsParsingException {
+ List<String> result =
+ input.isEmpty() ? ImmutableList.of() : ImmutableList.copyOf(splitter.split(input));
+ if (!allowEmptyValues && result.contains("")) {
+ throw new OptionsParsingException(
+ "Empty values are not allowed as part of this " + getTypeDescription());
+ }
+ return result;
}
@Override
@@ -273,13 +282,20 @@
public static class CommaSeparatedOptionListConverter extends SeparatedOptionListConverter {
public CommaSeparatedOptionListConverter() {
- super(',', "comma");
+ super(',', "comma", true);
+ }
+ }
+
+ public static class CommaSeparatedNonEmptyOptionListConverter
+ extends SeparatedOptionListConverter {
+ public CommaSeparatedNonEmptyOptionListConverter() {
+ super(',', "comma", false);
}
}
public static class ColonSeparatedOptionListConverter extends SeparatedOptionListConverter {
public ColonSeparatedOptionListConverter() {
- super(':', "colon");
+ super(':', "colon", true);
}
}
@@ -444,6 +460,44 @@
/**
* A converter for variable assignments from the parameter list of a blaze command invocation.
+ * Assignments are expected to have the form {@code name=value1[,..,valueN]}, where names and
+ * values are defined to be as permissive as possible.
+ */
+ public static class AssignmentToListOfValuesConverter
+ implements Converter<Map.Entry<String, List<String>>> {
+
+ private final Splitter splitter = Splitter.on(',');
+
+ @Override
+ public Map.Entry<String, List<String>> convert(String input) throws OptionsParsingException {
+ int pos = input.indexOf("=");
+ if (pos <= 0) {
+ throw new OptionsParsingException(
+ "Variable definitions must be in the form of a 'name=value1[,..,valueN]' assignment");
+ }
+ String name = input.substring(0, pos);
+ 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(
+ "Variable definitions must not contain empty strings or leading / trailing commas");
+ }
+ }
+ return Maps.immutableEntry(name, value);
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "a 'name=value1[,..,valueN]' assignment";
+ }
+ }
+
+ /**
+ * A converter for variable assignments from the parameter list of a blaze command invocation.
* Assignments are expected to have the form "name[=value]", where names and values are defined to
* be as permissive as possible and value part can be optional (in which case it is considered to
* be null).
