Simplified ActionContextConsumer by having it operate on a new class which holds a variety of strategy/context maps.
PiperOrigin-RevId: 190491357
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
index 8a96476..a0ea5698 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
@@ -183,11 +183,9 @@
return executor.getContext(type);
}
- /**
- * Returns the action context implementation for spawn actions with a given mnemonic.
- */
- public SpawnActionContext getSpawnActionContext(String mnemonic) {
- return executor.getSpawnActionContext(mnemonic);
+ /** Returns the action context implementation for spawn actions with a given mnemonic. */
+ public SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn) {
+ return executor.getSpawnActionContext(mnemonic, spawn);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Executor.java b/src/main/java/com/google/devtools/build/lib/actions/Executor.java
index 292e78f..bb1f6b4 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Executor.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Executor.java
@@ -87,8 +87,6 @@
*/
<T extends ActionContext> T getContext(Class<? extends T> type);
- /**
- * Returns the action context implementation for spawn actions with a given mnemonic.
- */
- SpawnActionContext getSpawnActionContext(String mnemonic);
+ /** Returns the action context implementation for spawn actions with a given mnemonic. */
+ SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java
index ae6d210..ec46996 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java
@@ -194,7 +194,7 @@
// Execute the spawn.
List<SpawnResult> spawnResults;
try {
- spawnResults = getContext(actionExecutionContext).exec(spawn, actionExecutionContext);
+ spawnResults = getContext(spawn, actionExecutionContext).exec(spawn, actionExecutionContext);
} catch (ExecException e) {
throw e.toActionExecutionException(
getMnemonic() + " action failed for target: " + getOwner().getLabel(),
@@ -233,8 +233,9 @@
return true;
}
- private SpawnActionContext getContext(ActionExecutionContext actionExecutionContext) {
- return actionExecutionContext.getSpawnActionContext(getMnemonic());
+ private SpawnActionContext getContext(
+ Spawn spawn, ActionExecutionContext actionExecutionContext) {
+ return actionExecutionContext.getSpawnActionContext(getMnemonic(), spawn);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index 3a5b225..fc068bc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -257,8 +257,8 @@
*/
protected List<SpawnResult> internalExecute(ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException, CommandLineExpansionException {
- return getContext(actionExecutionContext)
- .exec(getSpawn(actionExecutionContext.getClientEnv()), actionExecutionContext);
+ Spawn spawn = getSpawn(actionExecutionContext.getClientEnv());
+ return getContext(actionExecutionContext, spawn).exec(spawn, actionExecutionContext);
}
@Override
@@ -449,8 +449,9 @@
return executionInfo;
}
- protected SpawnActionContext getContext(ActionExecutionContext actionExecutionContext) {
- return actionExecutionContext.getSpawnActionContext(getMnemonic());
+ protected SpawnActionContext getContext(
+ ActionExecutionContext actionExecutionContext, Spawn spawn) {
+ return actionExecutionContext.getSpawnActionContext(getMnemonic(), spawn);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelActionContextConsumer.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelActionContextConsumer.java
index 9b3e370..009d039 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelActionContextConsumer.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelActionContextConsumer.java
@@ -14,20 +14,17 @@
package com.google.devtools.build.lib.bazel.rules;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableMultimap;
-import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext;
import com.google.devtools.build.lib.bazel.rules.BazelStrategyModule.BazelExecutionOptions;
import com.google.devtools.build.lib.exec.ActionContextConsumer;
+import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
import com.google.devtools.build.lib.exec.SpawnCache;
import com.google.devtools.build.lib.rules.android.WriteAdbArgsActionContext;
import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext;
import com.google.devtools.build.lib.rules.cpp.CppIncludeExtractionContext;
import com.google.devtools.build.lib.rules.cpp.CppIncludeScanningContext;
import java.util.Map;
-import java.util.TreeMap;
/**
* An object describing the {@link ActionContext} implementation that some actions require in Bazel.
@@ -40,12 +37,10 @@
}
@Override
- public ImmutableMap<String, String> getSpawnActionContexts() {
- Map<String, String> contexts = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
-
+ public void populate(SpawnActionContextMaps.Builder builder) {
// Default strategies for certain mnemonics - they can be overridden by --strategy= flags.
- contexts.put("Javac", "worker");
- contexts.put("Closure", "worker");
+ builder.strategyByMnemonicMap().put("Javac", "worker");
+ builder.strategyByMnemonicMap().put("Closure", "worker");
for (Map.Entry<String, String> strategy : options.strategy) {
String strategyName = strategy.getValue();
@@ -55,30 +50,25 @@
if (strategyName.equals("local")) {
strategyName = "standalone";
}
- contexts.put(strategy.getKey(), strategyName);
+ builder.strategyByMnemonicMap().put(strategy.getKey(), strategyName);
}
if (!options.genruleStrategy.isEmpty()) {
- contexts.put("Genrule", options.genruleStrategy);
+ builder.strategyByMnemonicMap().put("Genrule", options.genruleStrategy);
}
// TODO(bazel-team): put this in getActionContexts (key=SpawnActionContext.class) instead
if (!options.spawnStrategy.isEmpty()) {
- contexts.put("", options.spawnStrategy);
+ builder.strategyByMnemonicMap().put("", options.spawnStrategy);
}
- return ImmutableMap.copyOf(contexts);
- }
-
- @Override
- public Multimap<Class<? extends ActionContext>, String> getActionContexts() {
- return ImmutableMultimap.<Class<? extends ActionContext>, String>builder()
+ builder
+ .strategyByContextMap()
.put(CppCompileActionContext.class, "")
.put(CppIncludeExtractionContext.class, "")
.put(CppIncludeScanningContext.class, "")
.put(FileWriteActionContext.class, "")
.put(WriteAdbArgsActionContext.class, "")
- .put(SpawnCache.class, "")
- .build();
+ .put(SpawnCache.class, "");
}
}
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 3b238d9..3323f58 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
@@ -16,24 +16,15 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
-import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.base.Stopwatch;
-import com.google.common.base.Strings;
-import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Multimap;
-import com.google.common.collect.Ordering;
-import com.google.common.collect.Table;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionCacheChecker;
-import com.google.devtools.build.lib.actions.ActionContext;
-import com.google.devtools.build.lib.actions.ActionContextMarker;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionInputFileCache;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
@@ -41,14 +32,12 @@
import com.google.devtools.build.lib.actions.ArtifactFactory;
import com.google.devtools.build.lib.actions.BuildFailedException;
import com.google.devtools.build.lib.actions.ExecException;
-import com.google.devtools.build.lib.actions.ExecutionStrategy;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.actions.LocalHostCapacity;
import com.google.devtools.build.lib.actions.PackageRoots;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.ResourceSet;
-import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.actions.TestExecException;
import com.google.devtools.build.lib.actions.cache.ActionCache;
import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics;
@@ -60,7 +49,6 @@
import com.google.devtools.build.lib.analysis.actions.SymlinkTreeActionContext;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
-import com.google.devtools.build.lib.analysis.test.TestActionContext;
import com.google.devtools.build.lib.buildtool.buildevent.ExecutionPhaseCompleteEvent;
import com.google.devtools.build.lib.buildtool.buildevent.ExecutionStartingEvent;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -76,6 +64,7 @@
import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.FilesetActionContextImpl;
import com.google.devtools.build.lib.exec.SingleBuildFileCache;
+import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
import com.google.devtools.build.lib.exec.SymlinkTreeStrategy;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.ProfilePhase;
@@ -90,7 +79,6 @@
import com.google.devtools.build.lib.skyframe.OutputService;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.util.AbruptExitException;
-import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
@@ -102,16 +90,11 @@
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
import java.util.Collection;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import java.util.TreeMap;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
@@ -129,50 +112,6 @@
* @see com.google.devtools.build.lib.analysis.BuildView
*/
public class ExecutionTool {
- private static class StrategyConverter {
- private Table<Class<? extends ActionContext>, String, ActionContext> classMap =
- HashBasedTable.create();
- private Map<Class<? extends ActionContext>, ActionContext> defaultClassMap =
- new HashMap<>();
-
- /**
- * Aggregates all {@link ActionContext}s that are in {@code contextProviders}.
- */
- @SuppressWarnings("unchecked")
- private StrategyConverter(Iterable<ActionContextProvider> contextProviders) {
- for (ActionContextProvider provider : contextProviders) {
- for (ActionContext strategy : provider.getActionContexts()) {
- ExecutionStrategy annotation =
- strategy.getClass().getAnnotation(ExecutionStrategy.class);
- // TODO(ulfjack): Don't silently ignore action contexts without annotation.
- if (annotation != null) {
- defaultClassMap.put(annotation.contextType(), strategy);
-
- for (String name : annotation.name()) {
- classMap.put(annotation.contextType(), name, strategy);
- }
- }
- }
- }
- }
-
- @SuppressWarnings("unchecked")
- private <T extends ActionContext> T getStrategy(Class<T> clazz, String name) {
- return (T) (name.isEmpty() ? defaultClassMap.get(clazz) : classMap.get(clazz, name));
- }
-
- private String getValidValues(Class<? extends ActionContext> context) {
- return Joiner.on(", ").join(Ordering.natural().sortedCopy(classMap.row(context).keySet()));
- }
-
- private String getUserFriendlyName(Class<? extends ActionContext> context) {
- ActionContextMarker marker = context.getAnnotation(ActionContextMarker.class);
- return marker != null
- ? marker.name()
- : context.getSimpleName();
- }
- }
-
static final Logger logger = Logger.getLogger(ExecutionTool.class.getName());
private final CommandEnvironment env;
@@ -182,10 +121,7 @@
private final ActionInputFileCache fileCache;
private final ActionInputPrefetcher prefetcher;
private final ImmutableList<ActionContextProvider> actionContextProviders;
-
- private Map<String, SpawnActionContext> spawnStrategyMap =
- new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
- private List<ActionContext> strategies = new ArrayList<>();
+ private SpawnActionContextMaps spawnActionContextMaps;
ExecutionTool(CommandEnvironment env, BuildRequest request) throws ExecutorInitException {
this.env = env;
@@ -208,20 +144,11 @@
// these dependencies should be added to the ActionContextConsumer of the module that actually
// depends on them.
builder.addActionContextConsumer(
- new ActionContextConsumer() {
- @Override
- public ImmutableMap<String, String> getSpawnActionContexts() {
- return ImmutableMap.of();
- }
-
- @Override
- public Multimap<Class<? extends ActionContext>, String> getActionContexts() {
- return ImmutableMultimap.<Class<? extends ActionContext>, String>builder()
- .put(FilesetActionContext.class, "")
- .put(WorkspaceStatusAction.Context.class, "")
- .put(SymlinkTreeActionContext.class, "")
- .build();
- }
+ b -> {
+ b.strategyByContextMap()
+ .put(FilesetActionContext.class, "")
+ .put(WorkspaceStatusAction.Context.class, "")
+ .put(SymlinkTreeActionContext.class, "");
});
// Unfortunately, the exec root cache is not shared with caches in the remote execution client.
@@ -229,60 +156,24 @@
new SingleBuildFileCache(
env.getExecRoot().getPathString(), env.getRuntime().getFileSystem());
this.prefetcher = builder.getActionInputPrefetcher();
-
+
this.actionContextProviders = builder.getActionContextProviders();
for (ActionContextProvider provider : actionContextProviders) {
provider.init(fileCache);
}
- StrategyConverter strategyConverter = new StrategyConverter(actionContextProviders);
-
+ // There are many different SpawnActions, and we want to control the action context they use
+ // 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.
+ SpawnActionContextMaps.Builder spawnActionContextMapsBuilder =
+ new SpawnActionContextMaps.Builder();
for (ActionContextConsumer consumer : builder.getActionContextConsumers()) {
- // There are many different SpawnActions, and we want to control the action context they use
- // 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.
- for (Map.Entry<String, String> entry : consumer.getSpawnActionContexts().entrySet()) {
- SpawnActionContext context =
- strategyConverter.getStrategy(SpawnActionContext.class, entry.getValue());
- if (context == null) {
- String strategy = Strings.emptyToNull(entry.getKey());
- throw makeExceptionForInvalidStrategyValue(
- entry.getValue(),
- Joiner.on(' ').skipNulls().join(strategy, "spawn"),
- strategyConverter.getValidValues(SpawnActionContext.class));
- }
- spawnStrategyMap.put(entry.getKey(), context);
- }
-
- for (Map.Entry<Class<? extends ActionContext>, String> entry :
- consumer.getActionContexts().entries()) {
- ActionContext context = strategyConverter.getStrategy(entry.getKey(), entry.getValue());
- if (context == null) {
- throw makeExceptionForInvalidStrategyValue(
- entry.getValue(),
- strategyConverter.getUserFriendlyName(entry.getKey()),
- strategyConverter.getValidValues(entry.getKey()));
- }
- strategies.add(context);
- }
+ consumer.populate(spawnActionContextMapsBuilder);
}
-
- String testStrategyValue = request.getOptions(ExecutionOptions.class).testStrategy;
- ActionContext context = strategyConverter.getStrategy(TestActionContext.class,
- testStrategyValue);
- if (context == null) {
- throw makeExceptionForInvalidStrategyValue(testStrategyValue, "test",
- strategyConverter.getValidValues(TestActionContext.class));
- }
- strategies.add(context);
- }
-
- private static ExecutorInitException makeExceptionForInvalidStrategyValue(String value,
- String strategy, String validValues) {
- return new ExecutorInitException(String.format(
- "'%s' is an invalid value for %s strategy. Valid values are: %s", value, strategy,
- validValues), ExitCode.COMMAND_LINE_ERROR);
+ spawnActionContextMaps =
+ spawnActionContextMapsBuilder.build(
+ actionContextProviders, request.getOptions(ExecutionOptions.class).testStrategy);
}
Executor getExecutor() throws ExecutorInitException {
@@ -304,8 +195,7 @@
env.getEventBus(),
runtime.getClock(),
request,
- strategies,
- spawnStrategyMap,
+ spawnActionContextMaps,
actionContextProviders);
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ActionContextConsumer.java b/src/main/java/com/google/devtools/build/lib/exec/ActionContextConsumer.java
index 660ba4b..a63176b 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ActionContextConsumer.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ActionContextConsumer.java
@@ -13,51 +13,43 @@
// limitations under the License.
package com.google.devtools.build.lib.exec;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.actions.ActionContext;
+import com.google.devtools.build.lib.exec.SpawnActionContextMaps.Builder;
/**
- * An object describing that actions require a particular implementation of an
- * {@link ActionContext}.
+ * An object describing that actions require a particular implementation of an {@link
+ * ActionContext}.
*
* <p>This is expected to be implemented by modules that also implement actions which need these
* contexts. Other modules will provide implementations for various action contexts by implementing
* {@link ActionContextProvider}.
*
- * <p>Example: a module requires {@code SpawnActionContext} to do its job, and it creates
- * actions with the mnemonic <code>C++</code>. Then the {@link #getSpawnActionContexts} method of
- * this module would return a map with the key <code>"C++"</code> in it.
+ * <p>Example: a module requires {@code SpawnActionContext} to do its job, and it creates actions
+ * with the mnemonic <code>C++</code>. Then the {@link #populate(Builder)} method of this module
+ * would put <code>("C++", strategy)</code> in the map returned by {@link
+ * Builder#strategyByMnemonicMap()}.
*
* <p>The 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.
+ * 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>Other modules are free to provide different implementations of {@code SpawnActionContext}.
- * This can be used, for example, to implement sandboxed or distributed execution of
- * {@code SpawnAction}s in different ways, while giving the user control over how exactly they
- * are executed.
+ * This can be used, for example, to implement sandboxed or distributed execution of {@code
+ * SpawnAction}s in different ways, while giving the user control over how exactly they are
+ * executed.
+ *
+ * <p>Example: if a module requires {@code MyCustomActionContext} to be available, but doesn't
+ * associate it with any strategy, its {@link #populate(Builder)} should add {@code
+ * (MyCustomActionContext.class, "")} to the builder's {@link Builder#strategyByContextMap}.
+ *
+ * <p>Example: if a module requires {@code MyLocalCustomActionContext} to be available, and wants it
+ * to always use the "local" strategy, its {@link #populate(Builder)} should add {@code
+ * (MyCustomActionContext.class, "local")} to the builder's {@link Builder#strategyByContextMap}. .
*/
public interface ActionContextConsumer {
/**
- * Returns a map from spawn action mnemonics created by this module to the name of the
- * implementation of {@code SpawnActionContext} that the module wants to use for executing
- * it.
- *
- * <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
- * implementation of this method makes sure that no two keys that refer to the same mnemonic are
- * present in the returned map. The easiest way to assure this is to use a map created using
- * {@code new TreeMap<>(String.CASE_INSENSITIVE_ORDER)}.
+ * Provides a {@link SpawnActionContextMaps.Builder} instance which modules may use to configure
+ * the {@link ActionContext} instances the module requires for particular actions.
*/
- ImmutableMap<String, String> getSpawnActionContexts();
-
- /**
- * Returns a map from action context class to the implementation required by the module.
- *
- * <p>If the implementation name is the empty string, the choice is left to Blaze.
- */
- Multimap<Class<? extends ActionContext>, String> getActionContexts();
+ void populate(SpawnActionContextMaps.Builder builder);
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java b/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
index 89c22af..fe05762 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
@@ -14,12 +14,11 @@
package com.google.devtools.build.lib.exec;
import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableMap;
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionContext;
-import com.google.devtools.build.lib.actions.ExecutionStrategy;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.ExecutorInitException;
+import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -32,12 +31,7 @@
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.common.options.OptionsClassProvider;
-import java.util.HashMap;
-import java.util.LinkedHashSet;
-import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
-import java.util.TreeMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -63,9 +57,8 @@
private final OptionsClassProvider options;
private AtomicBoolean inExecutionPhase;
- private final Map<String, SpawnActionContext> spawnActionContextMap;
- private final Map<Class<? extends ActionContext>, ActionContext> contextMap =
- new HashMap<>();
+ private final Map<Class<? extends ActionContext>, ActionContext> contextMap;
+ private final SpawnActionContextMaps spawnActionContextMaps;
/**
* Constructs an Executor, bound to a specified output base path, and which will use the specified
@@ -85,8 +78,7 @@
EventBus eventBus,
Clock clock,
OptionsClassProvider options,
- List<ActionContext> contextImplementations,
- Map<String, SpawnActionContext> spawnActionContextMap,
+ SpawnActionContextMaps spawnActionContextMaps,
Iterable<ActionContextProvider> contextProviders)
throws ExecutorInitException {
ExecutionOptions executionOptions = options.getOptions(ExecutionOptions.class);
@@ -98,49 +90,16 @@
this.eventBus = eventBus;
this.clock = clock;
this.options = options;
+ this.spawnActionContextMaps = spawnActionContextMaps;
this.inExecutionPhase = new AtomicBoolean(false);
+ this.contextMap = spawnActionContextMaps.contextMap();
- // 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(contextImplementations);
- allContexts.addAll(spawnActionContextMap.values());
- this.spawnActionContextMap = ImmutableMap.copyOf(spawnActionContextMap);
- for (ActionContext context : contextImplementations) {
- ExecutionStrategy annotation = context.getClass().getAnnotation(ExecutionStrategy.class);
- if (annotation != null) {
- contextMap.put(annotation.contextType(), context);
- }
- contextMap.put(context.getClass(), context);
- }
-
- // Print a sorted list of our (Spawn)ActionContext maps.
if (executionOptions.debugPrintActionContexts) {
- for (Entry<String, SpawnActionContext> entry :
- new TreeMap<>(spawnActionContextMap).entrySet()) {
- reporter.handle(
- Event.info(
- String.format(
- "SpawnActionContextMap: \"%s\" = %s",
- entry.getKey(), entry.getValue().getClass().getSimpleName())));
- }
-
- TreeMap<String, String> sortedContextMapWithSimpleNames = new TreeMap<>();
- for (Entry<Class<? extends ActionContext>, ActionContext> entry : contextMap.entrySet()) {
- sortedContextMapWithSimpleNames.put(
- entry.getKey().getSimpleName(), entry.getValue().getClass().getSimpleName());
- }
- for (Entry<String, String> entry : sortedContextMapWithSimpleNames.entrySet()) {
- // Skip uninteresting identity mappings of contexts.
- if (!entry.getKey().equals(entry.getValue())) {
- reporter.handle(
- Event.info(String.format("ContextMap: %s = %s", entry.getKey(), entry.getValue())));
- }
- }
+ spawnActionContextMaps.debugPrintSpawnActionContextMaps(reporter);
}
for (ActionContextProvider factory : contextProviders) {
- factory.executorCreated(allContexts);
+ factory.executorCreated(spawnActionContextMaps.allContexts());
}
}
@@ -231,10 +190,9 @@
* set, then it returns the default strategy for spawn actions.
*/
@Override
- public SpawnActionContext getSpawnActionContext(String mnemonic) {
- SpawnActionContext context = spawnActionContextMap.get(mnemonic);
- return context == null ? spawnActionContextMap.get("") : context;
- }
+ public SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn) {
+ return spawnActionContextMaps.getSpawnActionContext(mnemonic, spawn, reporter);
+ }
/** Returns true iff the --verbose_failures option was enabled. */
@Override
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
new file mode 100644
index 0000000..fd017d0f
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
@@ -0,0 +1,349 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.exec;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Joiner;
+import com.google.common.base.Strings;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+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.Ordering;
+import com.google.common.collect.Table;
+import com.google.devtools.build.lib.actions.ActionContext;
+import com.google.devtools.build.lib.actions.ActionContextMarker;
+import com.google.devtools.build.lib.actions.ExecutionStrategy;
+import com.google.devtools.build.lib.actions.ExecutorInitException;
+import com.google.devtools.build.lib.actions.Spawn;
+import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.analysis.test.TestActionContext;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.util.ExitCode;
+import com.google.devtools.build.lib.util.RegexFilter;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+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;
+
+/**
+ * Container for looking up the {@link ActionContext} to use for a given action.
+ *
+ * <p>Holds {@link ActionContext} mappings populated by {@link ActionContextConsumer} modules. These
+ * include mappings from mnemonics and from description patterns.
+ *
+ * <p>At startup time, the application provides {@link Builder} to each module to register its
+ * contexts and mappings. At runtime, the {@link BlazeExecutor} uses the constructed object to find
+ * the context for each action.
+ */
+public final class SpawnActionContextMaps {
+
+ /** A stored entry for a {@link RegexFilter} to {@link SpawnActionContext} mapping. */
+ @AutoValue
+ public abstract static class RegexFilterSpawnActionContext {
+ public abstract RegexFilter regexFilter();
+
+ public abstract SpawnActionContext spawnActionContext();
+ }
+
+ private final ImmutableSortedMap<String, SpawnActionContext> spawnStrategyMnemonicMap;
+ private final ImmutableList<ActionContext> strategies;
+ private final ImmutableList<RegexFilterSpawnActionContext> spawnStrategyRegexList;
+
+ private SpawnActionContextMaps(
+ ImmutableSortedMap<String, SpawnActionContext> spawnStrategyMnemonicMap,
+ ImmutableList<ActionContext> strategies,
+ ImmutableList<RegexFilterSpawnActionContext> spawnStrategyRegexList) {
+ this.spawnStrategyMnemonicMap = spawnStrategyMnemonicMap;
+ this.strategies = strategies;
+ this.spawnStrategyRegexList = spawnStrategyRegexList;
+ }
+
+ /**
+ * Returns the appropriate {@link ActionContext} to execute an action with the given {@code
+ * mnemonic} and {@link Spawn}.
+ *
+ * <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(String mnemonic, Spawn spawn, Reporter reporter) {
+ if (!spawnStrategyRegexList.isEmpty() && spawn != null && 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.info(description + " with context " + entry.spawnActionContext().toString()));
+ return entry.spawnActionContext();
+ }
+ }
+ }
+ }
+ SpawnActionContext context = spawnStrategyMnemonicMap.get(mnemonic);
+ if (context != null) {
+ return context;
+ }
+ return spawnStrategyMnemonicMap.get("");
+ }
+
+ /** Returns a map from action context class to its instantiated context object. */
+ public 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);
+ if (annotation != null) {
+ contextMap.put(annotation.contextType(), context);
+ }
+ contextMap.put(context.getClass(), context);
+ }
+ return ImmutableMap.copyOf(contextMap);
+ }
+
+ /** Returns a list of all referenced {@link ActionContext} instances. */
+ public 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()));
+ return ImmutableList.copyOf(allContexts);
+ }
+
+ /**
+ * Print a sorted list of our (Spawn)ActionContext maps.
+ *
+ * <p>Prints out debug information about the mappings.
+ */
+ public void debugPrintSpawnActionContextMaps(Reporter reporter) {
+ for (Entry<String, SpawnActionContext> entry : spawnStrategyMnemonicMap.entrySet()) {
+ reporter.handle(
+ Event.info(
+ String.format(
+ "SpawnActionContextMap: \"%s\" = %s",
+ entry.getKey(), entry.getValue().getClass().getSimpleName())));
+ }
+
+ ImmutableMap<Class<? extends ActionContext>, ActionContext> contextMap = contextMap();
+ TreeMap<String, String> sortedContextMapWithSimpleNames = new TreeMap<>();
+ for (Entry<Class<? extends ActionContext>, ActionContext> entry : contextMap.entrySet()) {
+ sortedContextMapWithSimpleNames.put(
+ entry.getKey().getSimpleName(), entry.getValue().getClass().getSimpleName());
+ }
+ for (Entry<String, String> entry : sortedContextMapWithSimpleNames.entrySet()) {
+ // Skip uninteresting identity mappings of contexts.
+ if (!entry.getKey().equals(entry.getValue())) {
+ reporter.handle(
+ Event.info(String.format("ContextMap: %s = %s", entry.getKey(), entry.getValue())));
+ }
+ }
+
+ for (RegexFilterSpawnActionContext entry : spawnStrategyRegexList) {
+ reporter.handle(
+ Event.info(
+ String.format(
+ "SpawnActionContextMap: \"%s\" = %s",
+ entry.regexFilter().toString(),
+ entry.spawnActionContext().getClass().getSimpleName())));
+ }
+ }
+
+ @VisibleForTesting
+ public static SpawnActionContextMaps createStub(
+ List<ActionContext> strategies, Map<String, SpawnActionContext> spawnStrategyMnemonicMap) {
+ return new SpawnActionContextMaps(
+ ImmutableSortedMap.<String, SpawnActionContext>orderedBy(String.CASE_INSENSITIVE_ORDER)
+ .putAll(spawnStrategyMnemonicMap)
+ .build(),
+ ImmutableList.copyOf(strategies),
+ ImmutableList.of());
+ }
+
+ /** A stored entry for a {@link RegexFilter} to {@code strategy} mapping. */
+ @AutoValue
+ public abstract static class RegexFilterStrategy {
+ public abstract RegexFilter regexFilter();
+
+ public abstract String strategy();
+ }
+
+ /** Builder for {@code SpawnActionContextMaps}. */
+ public static final class Builder {
+ private ImmutableListMultimap.Builder<String, String> strategyByMnemonicMapBuilder =
+ ImmutableListMultimap.builder();
+ private ImmutableListMultimap.Builder<Class<? extends ActionContext>, String>
+ strategyByContextMapBuilder = ImmutableListMultimap.builder();
+
+ private final ImmutableList.Builder<RegexFilterStrategy> strategyByRegexpBuilder =
+ new ImmutableList.Builder();
+
+ /*
+ * Returns a builder modules can use to add mappings from mnemonics to strategy names.
+ *
+ * <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.
+ */
+ public ImmutableMultimap.Builder<String, String> strategyByMnemonicMap() {
+ return strategyByMnemonicMapBuilder;
+ }
+
+ /*
+ * Returns a builder modules can use to associate {@link ActionContext} classes with
+ * strategy names.
+ */
+ public ImmutableMultimap.Builder<Class<? extends ActionContext>, String>
+ strategyByContextMap() {
+ return strategyByContextMapBuilder;
+ }
+
+ /** Adds a mapping from the given {@link RegexFilter} to a {@code strategy}. */
+ public void addStrategyByRegexp(RegexFilter regexFilter, String strategy) {
+ strategyByRegexpBuilder.add(
+ new AutoValue_SpawnActionContextMaps_RegexFilterStrategy(regexFilter, strategy));
+ }
+
+ /** Builds a {@link SpawnActionContextMaps} instance. */
+ public SpawnActionContextMaps build(
+ ImmutableList<ActionContextProvider> actionContextProviders, String testStrategyValue)
+ throws ExecutorInitException {
+ StrategyConverter strategyConverter = new StrategyConverter(actionContextProviders);
+
+ ImmutableSortedMap.Builder<String, 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));
+ 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);
+ }
+
+ Set<ActionContext> seenContext = new HashSet<>();
+ for (Map.Entry<Class<? extends ActionContext>, String> entry :
+ strategyByContextMapBuilder.orderValuesBy(Collections.reverseOrder()).build().entries()) {
+ ActionContext context = strategyConverter.getStrategy(entry.getKey(), entry.getValue());
+ if (context == null) {
+ throw makeExceptionForInvalidStrategyValue(
+ entry.getValue(),
+ strategyConverter.getUserFriendlyName(entry.getKey()),
+ strategyConverter.getValidValues(entry.getKey()));
+ }
+ if (seenContext.contains(context)) {
+ continue;
+ }
+ seenContext.add(context);
+ strategies.add(context);
+ }
+
+ 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));
+ }
+
+ spawnStrategyRegexList.add(
+ new AutoValue_SpawnActionContextMaps_RegexFilterSpawnActionContext(
+ entry.regexFilter(), context));
+ }
+
+ ActionContext context =
+ strategyConverter.getStrategy(TestActionContext.class, testStrategyValue);
+ if (context == null) {
+ throw makeExceptionForInvalidStrategyValue(
+ testStrategyValue, "test", strategyConverter.getValidValues(TestActionContext.class));
+ }
+ strategies.add(context);
+
+ return new SpawnActionContextMaps(
+ spawnStrategyMap.build(), strategies.build(), spawnStrategyRegexList.build());
+ }
+ }
+
+ private static ExecutorInitException makeExceptionForInvalidStrategyValue(
+ String value, String strategy, String validValues) {
+ return new ExecutorInitException(
+ String.format(
+ "'%s' is an invalid value for %s strategy. Valid values are: %s",
+ value, strategy, validValues),
+ ExitCode.COMMAND_LINE_ERROR);
+ }
+
+ private static class StrategyConverter {
+ private Table<Class<? extends ActionContext>, String, ActionContext> classMap =
+ HashBasedTable.create();
+ private Map<Class<? extends ActionContext>, ActionContext> defaultClassMap = new HashMap<>();
+
+ /** Aggregates all {@link ActionContext}s that are in {@code contextProviders}. */
+ @SuppressWarnings("unchecked")
+ private StrategyConverter(Iterable<ActionContextProvider> contextProviders) {
+ for (ActionContextProvider provider : contextProviders) {
+ for (ActionContext strategy : provider.getActionContexts()) {
+ ExecutionStrategy annotation = strategy.getClass().getAnnotation(ExecutionStrategy.class);
+ // TODO(ulfjack): Don't silently ignore action contexts without annotation.
+ if (annotation != null) {
+ defaultClassMap.put(annotation.contextType(), strategy);
+
+ for (String name : annotation.name()) {
+ classMap.put(annotation.contextType(), name, strategy);
+ }
+ }
+ }
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private <T extends ActionContext> T getStrategy(Class<T> clazz, String name) {
+ return (T) (name.isEmpty() ? defaultClassMap.get(clazz) : classMap.get(clazz, name));
+ }
+
+ private String getValidValues(Class<? extends ActionContext> context) {
+ return Joiner.on(", ").join(Ordering.natural().sortedCopy(classMap.row(context).keySet()));
+ }
+
+ private String getUserFriendlyName(Class<? extends ActionContext> context) {
+ ActionContextMarker marker = context.getAnnotation(ActionContextMarker.class);
+ return marker != null ? marker.name() : context.getSimpleName();
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
index 93134e8..91ff96f 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
@@ -348,7 +348,7 @@
long startTime = actionExecutionContext.getClock().currentTimeMillis();
SpawnActionContext spawnActionContext =
- actionExecutionContext.getSpawnActionContext(action.getMnemonic());
+ actionExecutionContext.getSpawnActionContext(action.getMnemonic(), spawn);
List<SpawnResult> spawnResults = ImmutableList.of();
try {
try {
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
index 70b0ddd..612b926 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
@@ -116,16 +116,16 @@
Preconditions.checkState(
actionExecutionContext.getInputPath(inputManifestArtifact).equals(inputManifest));
if (enableRunfiles) {
+ Spawn spawn =
+ createSpawn(
+ owner,
+ actionExecutionContext.getExecRoot(),
+ binTools,
+ shellEnvironment,
+ inputManifestArtifact);
return actionExecutionContext
- .getSpawnActionContext(owner.getMnemonic())
- .exec(
- createSpawn(
- owner,
- actionExecutionContext.getExecRoot(),
- binTools,
- shellEnvironment,
- inputManifestArtifact),
- actionExecutionContext);
+ .getSpawnActionContext(owner.getMnemonic(), spawn)
+ .exec(spawn, actionExecutionContext);
} else {
// Pretend we created the runfiles tree by copying the manifest
try {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
index a0e6c84..2b214ee 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -317,7 +317,7 @@
estimateResourceConsumptionLocal());
return ActionResult.create(
actionExecutionContext
- .getSpawnActionContext(getMnemonic())
+ .getSpawnActionContext(getMnemonic(), spawn)
.exec(spawn, actionExecutionContext));
} catch (ExecException e) {
throw e.toActionExecutionException(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
index 2f40917..3f40a10 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
@@ -63,7 +63,7 @@
List<SpawnResult> spawnResults =
actionExecutionContext
- .getSpawnActionContext(action.getMnemonic())
+ .getSpawnActionContext(action.getMnemonic(), spawn)
.exec(spawn, actionExecutionContext);
return CppCompileActionResult.builder().setSpawnResults(spawnResults).build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
index fca4ffc..51f91c2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
@@ -140,9 +140,10 @@
@Override
protected List<SpawnResult> internalExecute(ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
- SpawnActionContext context = getContext(actionExecutionContext);
+ Spawn spawn = getDirectSpawn();
+ SpawnActionContext context = getContext(actionExecutionContext, spawn);
try {
- return context.exec(getDirectSpawn(), actionExecutionContext);
+ return context.exec(spawn, actionExecutionContext);
} catch (ExecException e) {
// if the direct input spawn failed, try again with transitive inputs to produce better
// better messages
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java
index 64aaf90..08547c30 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java
@@ -16,10 +16,10 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
-import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.exec.ActionContextConsumer;
+import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
/**
@@ -53,12 +53,8 @@
}
@Override
- public ImmutableMap<String, String> getSpawnActionContexts() {
- return spawnContexts;
- }
-
- @Override
- public Multimap<Class<? extends ActionContext>, String> getActionContexts() {
- return contexts;
+ public void populate(SpawnActionContextMaps.Builder builder) {
+ builder.strategyByMnemonicMap().putAll(spawnContexts.entrySet());
+ builder.strategyByContextMap().putAll(contexts.entries());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextConsumer.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextConsumer.java
index 168c596..225b3a3 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextConsumer.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextConsumer.java
@@ -14,12 +14,9 @@
package com.google.devtools.build.lib.standalone;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableMultimap;
-import com.google.common.collect.Multimap;
-import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.exec.ActionContextConsumer;
+import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
/**
* {@link ActionContextConsumer} that requests the action contexts necessary for standalone
@@ -28,17 +25,13 @@
public class StandaloneActionContextConsumer implements ActionContextConsumer {
@Override
- public ImmutableMap<String, String> getSpawnActionContexts() {
+ public void populate(SpawnActionContextMaps.Builder builder) {
// This makes the "sandboxed" strategy the default Spawn strategy, unless it is overridden by a
// later BlazeModule.
- return ImmutableMap.of("", "standalone");
- }
+ builder.strategyByMnemonicMap().put("", "standalone");
- @Override
- public Multimap<Class<? extends ActionContext>, String> getActionContexts() {
// This makes the "standalone" strategy available via --spawn_strategy=standalone, but it is not
// necessarily the default.
- return ImmutableMultimap.<Class<? extends ActionContext>, String>of(
- SpawnActionContext.class, "standalone");
+ builder.strategyByContextMap().put(SpawnActionContext.class, "standalone");
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextConsumer.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextConsumer.java
index 8ff769e..d3c083d 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextConsumer.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextConsumer.java
@@ -13,13 +13,9 @@
// limitations under the License.
package com.google.devtools.build.lib.worker;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableMultimap;
-import com.google.common.collect.ImmutableMultimap.Builder;
-import com.google.common.collect.Multimap;
-import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.exec.ActionContextConsumer;
+import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
/**
* {@link ActionContextConsumer} that requests the action contexts necessary for worker process
@@ -28,15 +24,8 @@
public class WorkerActionContextConsumer implements ActionContextConsumer {
@Override
- public ImmutableMap<String, String> getSpawnActionContexts() {
- return ImmutableMap.of();
- }
-
- @Override
- public Multimap<Class<? extends ActionContext>, String> getActionContexts() {
- Builder<Class<? extends ActionContext>, String> contexts = ImmutableMultimap.builder();
- contexts.put(SpawnActionContext.class, "standalone");
- contexts.put(SpawnActionContext.class, "worker");
- return contexts.build();
+ public void populate(SpawnActionContextMaps.Builder builder) {
+ builder.strategyByContextMap().put(SpawnActionContext.class, "standalone");
+ builder.strategyByContextMap().put(SpawnActionContext.class, "worker");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 3f55bd0..330c8d7 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1385,6 +1385,7 @@
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
"//src/main/java/com/google/devtools/common/options",
+ "//src/main/protobuf:test_status_java_proto",
"//third_party:mockito",
"//third_party/protobuf:protobuf_java",
],
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java b/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
index 34f6283..d9d78cd 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
@@ -16,6 +16,7 @@
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.Executor;
+import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.clock.Clock;
@@ -83,7 +84,7 @@
}
@Override
- public SpawnActionContext getSpawnActionContext(String mnemonic) {
+ public SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn) {
throw new UnsupportedOperationException();
}
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
new file mode 100644
index 0000000..8e9f610
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
@@ -0,0 +1,151 @@
+// Copyright 2018 The Bazel Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.exec;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.eventbus.EventBus;
+import com.google.devtools.build.lib.actions.ActionContext;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
+import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ExecutionStrategy;
+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.analysis.test.TestActionContext;
+import com.google.devtools.build.lib.analysis.test.TestResult;
+import com.google.devtools.build.lib.analysis.test.TestRunnerAction;
+import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.testutil.Suite;
+import com.google.devtools.build.lib.testutil.TestSpec;
+import com.google.devtools.build.lib.util.RegexFilter.RegexFilterConverter;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
+import java.io.IOException;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mockito;
+
+/** Tests of {@link SpawnActionContextMaps}. */
+@RunWith(JUnit4.class)
+@TestSpec(size = Suite.SMALL_TESTS)
+public class SpawnActionContextMapsTest {
+
+ private SpawnActionContextMaps.Builder builder;
+ private final RegexFilterConverter converter = new RegexFilterConverter();
+ private final EventBus bus = new EventBus();
+ private final Reporter reporter = new Reporter(bus);
+
+ private static final ImmutableList<ActionContextProvider> PROVIDERS =
+ ImmutableList.of(
+ new ActionContextProvider() {
+ @Override
+ public Iterable<? extends ActionContext> getActionContexts() {
+ return ImmutableList.of(new AC1(), new AC2(), new ACTest());
+ }
+ });
+
+ @Before
+ public void setUp() {
+ builder = new SpawnActionContextMaps.Builder();
+ }
+
+ @Test
+ public void duplicateMnemonics_lastOneWins() throws Exception {
+ builder.strategyByMnemonicMap().put("Spawn1", "ac1").put("Spawn1", "ac2");
+ SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest");
+ SpawnActionContext result = maps.getSpawnActionContext("Spawn1", null, reporter);
+ assertThat(result).isInstanceOf(AC2.class);
+ }
+
+ @Test
+ public void multipleRegexps_firstMatchWins() throws Exception {
+ builder.addStrategyByRegexp(converter.convert("foo"), "ac1");
+ builder.addStrategyByRegexp(converter.convert("foo/bar"), "ac2");
+ SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest");
+
+ SpawnActionContext result =
+ maps.getSpawnActionContext("", mockSpawn("Doing something with foo/bar/baz"), reporter);
+
+ assertThat(result).isInstanceOf(AC1.class);
+ }
+
+ @Test
+ public void regexpAndMnemonic_regexpWins() throws Exception {
+ builder.strategyByMnemonicMap().put("Spawn1", "ac1");
+ builder.addStrategyByRegexp(converter.convert("foo/bar"), "ac2");
+ SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest");
+
+ SpawnActionContext result =
+ maps.getSpawnActionContext(
+ "Spawn1", mockSpawn("Doing something with foo/bar/baz"), reporter);
+
+ assertThat(result).isInstanceOf(AC2.class);
+ }
+
+ @Test
+ public void duplicateContext_noException() throws Exception {
+ builder.strategyByContextMap().put(AC1.class, "one");
+ builder.strategyByContextMap().put(AC1.class, "two");
+ builder.strategyByContextMap().put(AC1.class, "");
+ }
+
+ private Spawn mockSpawn(String message) {
+ Spawn mockSpawn = Mockito.mock(Spawn.class);
+ ActionExecutionMetadata mockOwner = Mockito.mock(ActionExecutionMetadata.class);
+ when(mockOwner.getProgressMessage()).thenReturn(message);
+ when(mockSpawn.getResourceOwner()).thenReturn(mockOwner);
+ return mockSpawn;
+ }
+
+ @ExecutionStrategy(contextType = SpawnActionContext.class, name = "ac1")
+ private static class AC1 implements SpawnActionContext {
+ @Override
+ public List<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException {
+ throw new UnsupportedOperationException();
+ }
+ }
+
+ @ExecutionStrategy(contextType = SpawnActionContext.class, name = "ac2")
+ private static class AC2 implements SpawnActionContext {
+ @Override
+ public List<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException {
+ throw new UnsupportedOperationException();
+ }
+ }
+
+ @ExecutionStrategy(contextType = TestActionContext.class, name = "actest")
+ private static class ACTest implements TestActionContext {
+ @Override
+ public List<SpawnResult> exec(
+ TestRunnerAction action, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public TestResult newCachedTestResult(
+ Path execRoot, TestRunnerAction action, TestResultData cached) throws IOException {
+ throw new UnsupportedOperationException();
+ }
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
index 57ad243..9ebe4aa 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
@@ -128,7 +128,7 @@
.build();
when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult));
- when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext);
+ when(actionExecutionContext.getSpawnActionContext(any(), any())).thenReturn(spawnActionContext);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -208,7 +208,7 @@
expectedSpawnResult,
/*forciblyRunRemotely=*/ false,
/*catastrophe=*/ false));
- when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext);
+ when(actionExecutionContext.getSpawnActionContext(any(), any())).thenReturn(spawnActionContext);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -285,7 +285,7 @@
SpawnResult expectedSpawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).build();
when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult));
- when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext);
+ when(actionExecutionContext.getSpawnActionContext(any(), any())).thenReturn(spawnActionContext);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java b/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
index 70bd8e6..db3214f 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.exec.util;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionContext;
@@ -28,6 +27,7 @@
import com.google.devtools.build.lib.exec.BlazeExecutor;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.FileWriteStrategy;
+import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
import com.google.devtools.build.lib.exec.SymlinkTreeStrategy;
import com.google.devtools.build.lib.runtime.CommonCommandOptions;
import com.google.devtools.build.lib.testutil.TestConstants;
@@ -107,8 +107,7 @@
bus,
BlazeClock.instance(),
optionsParser,
- strategies,
- ImmutableMap.copyOf(spawnStrategyMap),
+ SpawnActionContextMaps.createStub(strategies, spawnStrategyMap),
ImmutableList.<ActionContextProvider>of());
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index 37fd188..0e93ff2 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Sets;
import com.google.common.eventbus.EventBus;
-import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
@@ -44,6 +43,7 @@
import com.google.devtools.build.lib.exec.BlazeExecutor;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.SingleBuildFileCache;
+import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
import com.google.devtools.build.lib.exec.local.LocalSpawnRunner;
@@ -134,16 +134,17 @@
bus,
BlazeClock.instance(),
optionsParser,
- ImmutableList.<ActionContext>of(),
- ImmutableMap.<String, SpawnActionContext>of(
- "",
- new StandaloneSpawnStrategy(
- execRoot,
- new LocalSpawnRunner(
+ SpawnActionContextMaps.createStub(
+ ImmutableList.of(),
+ ImmutableMap.<String, SpawnActionContext>of(
+ "",
+ new StandaloneSpawnStrategy(
execRoot,
- localExecutionOptions,
- resourceManager,
- LocalEnvProvider.UNMODIFIED))),
+ new LocalSpawnRunner(
+ execRoot,
+ localExecutionOptions,
+ resourceManager,
+ LocalEnvProvider.UNMODIFIED)))),
ImmutableList.<ActionContextProvider>of());
executor.getExecRoot().createDirectoryAndParents();
@@ -170,14 +171,14 @@
@Test
public void testBinTrueExecutesFine() throws Exception {
Spawn spawn = createSpawn(getTrueCommand());
- executor.getSpawnActionContext(spawn.getMnemonic()).exec(spawn, createContext());
+ executor.getSpawnActionContext(spawn.getMnemonic(), spawn).exec(spawn, createContext());
assertThat(out()).isEmpty();
assertThat(err()).isEmpty();
}
private List<SpawnResult> run(Spawn spawn) throws Exception {
- return executor.getSpawnActionContext(spawn.getMnemonic()).exec(spawn, createContext());
+ return executor.getSpawnActionContext(spawn.getMnemonic(), spawn).exec(spawn, createContext());
}
private ActionExecutionContext createContext() {
