Split SpawnStrategy from ActionContext.
This change removes SpawnStrategy from the ActionContext hierarchy. Spawn strategies thus are officially no longer queryable from any place in the execution phase (they already hadn't been but it was not obvious at all). Instead all access to them must be mediated by the SpawnStrategyResolver (formerly ProxySpawnStrategy) which uses the SpawnStrategyRegistry (formerly SpawnActionContextMaps) to do its work.
Other kinds of action contexts are accessible through the ModuleActionContextRegistry. Blaze modules now also register action contexts and strategies separately, in newly introduced methods for this purpose.
This change makes the many distinct properties of spawn strategies (as opposed to other contexts) more obvious, whether in registration (where there is a complex matching logic for strategies to spawns) or in usage (execution flows and selection of strategy). It also paves the way for some information of strategies to be available in the analysis phase rather than only for the execution phase (action contexts will remain in the latter).
RELNOTES: None.
PiperOrigin-RevId: 291728633
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnStrategy.java
index d0ef44c..33d0454 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnStrategy.java
@@ -15,9 +15,17 @@
import com.google.common.collect.ImmutableList;
-/** A context that allows execution of {@link Spawn} instances. */
-@ActionContextMarker(name = "spawn")
-public interface SpawnStrategy extends ActionContext {
+/**
+ * An implementation of how to {@linkplain #exec execute} a {@link Spawn} instance.
+ *
+ * <p>Strategies are used during the execution phase based on how they were {@linkplain
+ * com.google.devtools.build.lib.runtime.BlazeModule#registerSpawnStrategies registered by a module}
+ * and whether they {@linkplain #canExec match a given spawn based on their abilities}.
+ */
+// TODO(schmitt): If possible, merge this with AbstractSpawnStrategy and SpawnRunner. The former
+// because (almost?) all implementations of this interface extend it; the latter because it forms
+// a shadow hierarchy graph that looks like that of the corresponding strategies.
+public interface SpawnStrategy {
/**
* Executes the given spawn and returns metadata about the execution. Implementations must
@@ -43,5 +51,15 @@
}
/** Returns whether this SpawnActionContext supports executing the given Spawn. */
- boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry);
+ boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry);
+
+ /**
+ * Performs any actions conditional on this strategy not only being registered but triggered as
+ * used because its identifier was requested and it was not overridden.
+ *
+ * @param actionContextRegistry a registry containing all available contexts
+ */
+ // TODO(schmitt): Remove once strategies are only instantiated if used, the callback can then be
+ // done upon construction.
+ default void usedContext(ActionContext.ActionContextRegistry actionContextRegistry) {}
}
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 31955b7..b8b7254 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
@@ -56,7 +56,6 @@
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnContinuation;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.actions.extra.EnvironmentVariable;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.actions.extra.SpawnInfo;
@@ -67,6 +66,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skylarkbuildapi.CommandLineArgsApi;
@@ -326,7 +326,7 @@
}
SpawnContinuation spawnContinuation =
actionExecutionContext
- .getContext(SpawnStrategy.class)
+ .getContext(SpawnStrategyResolver.class)
.beginExecution(spawn, actionExecutionContext);
return new SpawnActionContinuation(actionExecutionContext, spawnContinuation);
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
index 71a150e..c4b0650 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
@@ -36,7 +36,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import com.google.devtools.build.lib.runtime.Command;
@@ -351,9 +351,11 @@
}
@Override
- public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
- builder.addActionContext(
+ public void registerActionContexts(
+ ModuleActionContextRegistry.Builder registryBuilder,
+ CommandEnvironment env,
+ BuildRequest buildRequest) {
+ registryBuilder.register(
WorkspaceStatusAction.Context.class, new BazelWorkspaceStatusActionContext(env));
}
-
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java b/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
index fa621a3..701380a 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
@@ -18,6 +18,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.SpawnLogContext;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.runtime.BlazeModule;
@@ -117,6 +118,18 @@
}
@Override
+ public void registerActionContexts(
+ ModuleActionContextRegistry.Builder registryBuilder,
+ CommandEnvironment env,
+ BuildRequest buildRequest) {
+ if (spawnLogContext != null) {
+ // TODO(schmitt): Pretty sure the "spawn-log" commandline identifier is never used as there is
+ // no other SpawnLogContext to distinguish from.
+ registryBuilder.register(SpawnLogContext.class, spawnLogContext, "spawn-log");
+ }
+ }
+
+ @Override
public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
env.getEventBus().register(this);
@@ -129,13 +142,6 @@
new AbruptExitException(
"Error initializing execution log", ExitCode.COMMAND_LINE_ERROR, e));
}
-
- if (spawnLogContext != null) {
- // TODO(schmitt): Pretty sure the "spawn-log" commandline identifier is never used as there is
- // no other SpawnLogContext to distinguish from.
- builder.addActionContext(SpawnLogContext.class, spawnLogContext, "spawn-log");
- builder.addStrategyByContext(SpawnLogContext.class, "");
- }
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java
index 3992a75..ecab4b4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java
@@ -36,7 +36,6 @@
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
@@ -51,6 +50,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
@@ -132,7 +132,7 @@
LOCAL_RESOURCES);
List<SpawnResult> spawnResults =
actionExecutionContext
- .getContext(SpawnStrategy.class)
+ .getContext(SpawnStrategyResolver.class)
.exec(spawn, actionExecutionContext);
actionExecutionContext.getEventHandler().handle(Event.info(locationMessage));
return ActionResult.create(spawnResults);
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 744b514..044bbce 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
@@ -15,16 +15,10 @@
package com.google.devtools.build.lib.bazel.rules;
import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext;
-import com.google.devtools.build.lib.analysis.actions.TemplateExpansionContext;
-import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
-import com.google.devtools.build.lib.exec.SpawnCache;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.remote.RemoteModule;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
-import com.google.devtools.build.lib.rules.cpp.CppIncludeExtractionContext;
-import com.google.devtools.build.lib.rules.cpp.CppIncludeScanningContext;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -45,7 +39,8 @@
}
@Override
- public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
+ public void registerSpawnStrategies(
+ SpawnStrategyRegistry.Builder registryBuilder, CommandEnvironment env) {
ExecutionOptions options = env.getOptions().getOptions(ExecutionOptions.class);
RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class);
@@ -62,25 +57,18 @@
}
spawnStrategies.add("local");
}
+ registryBuilder.setDefaultStrategies(spawnStrategies);
- // Allow genrule_strategy to also be overridden by --strategy= flags.
- builder.addStrategyByMnemonic("Genrule", options.genruleStrategy);
+ // By adding this filter before the ones derived from --strategy the latter can override the
+ // former.
+ registryBuilder.addMnemonicFilter("Genrule", options.genruleStrategy);
for (Map.Entry<String, List<String>> strategy : options.strategy) {
- builder.addStrategyByMnemonic(strategy.getKey(), strategy.getValue());
+ registryBuilder.addMnemonicFilter(strategy.getKey(), strategy.getValue());
}
- builder.addStrategyByMnemonic("", spawnStrategies);
-
for (Map.Entry<RegexFilter, List<String>> entry : options.strategyByRegexp) {
- builder.addStrategyByRegexp(entry.getKey(), entry.getValue());
+ registryBuilder.addDescriptionFilter(entry.getKey(), entry.getValue());
}
-
- builder
- .addStrategyByContext(CppIncludeExtractionContext.class, "")
- .addStrategyByContext(CppIncludeScanningContext.class, "")
- .addStrategyByContext(FileWriteActionContext.class, "")
- .addStrategyByContext(TemplateExpansionContext.class, "")
- .addStrategyByContext(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 fc343e4..a1bb69a 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
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ArtifactFactory;
import com.google.devtools.build.lib.actions.BuildFailedException;
+import com.google.devtools.build.lib.actions.DynamicStrategyRegistry;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.actions.LocalHostCapacity;
@@ -42,7 +43,6 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
-import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
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.BuildOptions;
@@ -64,7 +64,10 @@
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
-import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
+import com.google.devtools.build.lib.exec.RemoteLocalFallbackRegistry;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.exec.SymlinkTreeStrategy;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.ProfilePhase;
@@ -122,8 +125,9 @@
private final BuildRequest request;
private BlazeExecutor executor;
private final ActionInputPrefetcher prefetcher;
+ private final SpawnStrategyRegistry spawnStrategyRegistry;
+ private final ModuleActionContextRegistry actionContextRegistry;
private final ImmutableSet<ExecutorLifecycleListener> executorLifecycleListeners;
- private final SpawnActionContextMaps spawnActionContextMaps;
ExecutionTool(CommandEnvironment env, BuildRequest request) throws ExecutorInitException {
this.env = env;
@@ -136,33 +140,47 @@
throw new ExecutorInitException("Execroot creation failed", e);
}
+ ExecutionOptions options = request.getOptions(ExecutionOptions.class);
ExecutorBuilder builder = new ExecutorBuilder();
+ ModuleActionContextRegistry.Builder actionContextRegistryBuilder =
+ new ModuleActionContextRegistry.Builder()
+ // TODO(philwo): ExecutionTool should not modify action contexts on its own, instead
+ // these should be added and restricted in modules.
+ .restrictTo(TestActionContext.class, options.testStrategy)
+ .register(
+ SymlinkTreeActionContext.class,
+ new SymlinkTreeStrategy(
+ env.getOutputService(), env.getBlazeWorkspace().getBinTools()))
+ .register(SpawnStrategyResolver.class, new SpawnStrategyResolver());
+ SpawnStrategyRegistry.Builder spawnStrategyRegistryBuilder =
+ new SpawnStrategyRegistry.Builder()
+ // TODO: Wrap in incompatible flag.
+ .useLegacyDescriptionFilterPrecedence();
for (BlazeModule module : runtime.getBlazeModules()) {
- try (SilentCloseable closeable = Profiler.instance().profile(module + ".executorInit")) {
+ try (SilentCloseable ignored = Profiler.instance().profile(module + ".executorInit")) {
module.executorInit(env, request, builder);
}
+
+ try (SilentCloseable ignored =
+ Profiler.instance().profile(module + ".registerActionContexts")) {
+ module.registerActionContexts(actionContextRegistryBuilder, env, request);
+ }
+
+ try (SilentCloseable ignored =
+ Profiler.instance().profile(module + ".registerSpawnStrategies")) {
+ module.registerSpawnStrategies(spawnStrategyRegistryBuilder, env);
+ }
}
- builder.addActionContext(
- SymlinkTreeActionContext.class,
- new SymlinkTreeStrategy(env.getOutputService(), env.getBlazeWorkspace().getBinTools()));
- // TODO(philwo) - the ExecutionTool should not add arbitrary dependencies on its own, instead
- // these dependencies should be added to the ActionContextConsumer of the module that actually
- // depends on them.
- builder
- .addStrategyByContext(WorkspaceStatusAction.Context.class, "")
- .addStrategyByContext(SymlinkTreeActionContext.class, "");
+
+ spawnStrategyRegistry = spawnStrategyRegistryBuilder.build();
+ actionContextRegistryBuilder.register(SpawnStrategyRegistry.class, spawnStrategyRegistry);
+ actionContextRegistryBuilder.register(DynamicStrategyRegistry.class, spawnStrategyRegistry);
+ actionContextRegistryBuilder.register(RemoteLocalFallbackRegistry.class, spawnStrategyRegistry);
+
+ actionContextRegistry = actionContextRegistryBuilder.build();
+ executorLifecycleListeners = builder.getExecutorLifecycleListeners();
this.prefetcher = builder.getActionInputPrefetcher();
- this.executorLifecycleListeners = builder.getExecutorLifecycleListeners();
-
- // 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.
- ExecutionOptions options = request.getOptions(ExecutionOptions.class);
- // TODO(jmmv): This should live in some testing-related Blaze module, not here.
- builder.addStrategyByContext(TestActionContext.class, options.testStrategy);
- spawnActionContextMaps = builder.getSpawnActionContextMaps();
if (options.availableResources != null && options.removeLocalResources) {
throw new ExecutorInitException(
@@ -189,7 +207,8 @@
getReporter(),
runtime.getClock(),
request,
- spawnActionContextMaps);
+ actionContextRegistry,
+ spawnStrategyRegistry);
}
void init() throws ExecutorInitException {
diff --git a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicExecutionModule.java b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicExecutionModule.java
index d7126c8..0c06cc3 100644
--- a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicExecutionModule.java
+++ b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicExecutionModule.java
@@ -24,10 +24,9 @@
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.actions.Spawns;
-import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.concurrent.ExecutorUtil;
import com.google.devtools.build.lib.exec.ExecutionPolicy;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -42,9 +41,6 @@
*/
public class DynamicExecutionModule extends BlazeModule {
- /** Strings that can be used to select this strategy in flags. */
- private static final String[] COMMANDLINE_IDENTIFIERS = {"dynamic", "dynamic_worker"};
-
private ExecutorService executorService;
public DynamicExecutionModule() {}
@@ -105,47 +101,45 @@
: options.dynamicRemoteStrategy;
}
- @VisibleForTesting
- void initStrategies(ExecutorBuilder builder, DynamicExecutionOptions options)
+ @Override
+ public void registerSpawnStrategies(
+ SpawnStrategyRegistry.Builder registryBuilder, CommandEnvironment env)
+ throws ExecutorInitException {
+ registerSpawnStrategies(
+ registryBuilder, env.getOptions().getOptions(DynamicExecutionOptions.class));
+ }
+
+ @VisibleForTesting // CommandEnvironment is difficult to access in tests.
+ void registerSpawnStrategies(
+ SpawnStrategyRegistry.Builder registryBuilder, DynamicExecutionOptions options)
throws ExecutorInitException {
if (!options.internalSpawnScheduler) {
return;
}
+ SpawnStrategy strategy;
if (options.legacySpawnScheduler) {
- builder.addActionContext(
- SpawnStrategy.class,
- new LegacyDynamicSpawnStrategy(executorService, options, this::getExecutionPolicy),
- COMMANDLINE_IDENTIFIERS);
+ strategy = new LegacyDynamicSpawnStrategy(executorService, options, this::getExecutionPolicy);
} else {
- builder.addActionContext(
- SpawnStrategy.class,
- new DynamicSpawnStrategy(executorService, options, this::getExecutionPolicy),
- COMMANDLINE_IDENTIFIERS);
+ strategy = new DynamicSpawnStrategy(executorService, options, this::getExecutionPolicy);
}
- builder.addStrategyByContext(SpawnStrategy.class, "dynamic");
+ registryBuilder.registerStrategy(strategy, "dynamic", "dynamic_worker");
for (Map.Entry<String, List<String>> mnemonicToStrategies : getLocalStrategies(options)) {
throwIfContainsDynamic(mnemonicToStrategies.getValue(), "--dynamic_local_strategy");
- builder.addDynamicLocalStrategiesByMnemonic(
+ registryBuilder.addDynamicLocalStrategiesByMnemonic(
mnemonicToStrategies.getKey(), mnemonicToStrategies.getValue());
}
for (Map.Entry<String, List<String>> mnemonicToStrategies : getRemoteStrategies(options)) {
throwIfContainsDynamic(mnemonicToStrategies.getValue(), "--dynamic_remote_strategy");
- builder.addDynamicRemoteStrategiesByMnemonic(
+ registryBuilder.addDynamicRemoteStrategiesByMnemonic(
mnemonicToStrategies.getKey(), mnemonicToStrategies.getValue());
}
}
- @Override
- public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder)
- throws ExecutorInitException {
- initStrategies(builder, env.getOptions().getOptions(DynamicExecutionOptions.class));
- }
-
private void throwIfContainsDynamic(List<String> strategies, String flagName)
throws ExecutorInitException {
- ImmutableSet<String> identifiers = ImmutableSet.copyOf(COMMANDLINE_IDENTIFIERS);
+ ImmutableSet<String> identifiers = ImmutableSet.of("dynamic", "dynamic_worker");
if (!Sets.intersection(identifiers, ImmutableSet.copyOf(strategies)).isEmpty()) {
throw new ExecutorInitException(
"Cannot use strategy "
@@ -158,6 +152,7 @@
/**
* Use the {@link Spawn} metadata to determine if it can be executed locally, remotely, or both.
+ *
* @param spawn the {@link Spawn} action
* @return the {@link ExecutionPolicy} containing local/remote execution policies
*/
diff --git a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java
index 6b9c3c9..f6b0599 100644
--- a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java
@@ -23,6 +23,7 @@
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;
+import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.DynamicStrategyRegistry;
import com.google.devtools.build.lib.actions.ExecException;
@@ -327,7 +328,7 @@
}
@Override
- public boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry) {
+ public boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry) {
DynamicStrategyRegistry dynamicStrategyRegistry =
actionContextRegistry.getContext(DynamicStrategyRegistry.class);
for (SandboxedSpawnStrategy strategy :
@@ -348,10 +349,10 @@
}
@Override
- public void usedContext(ActionContextRegistry actionExecutionContext) {
- actionExecutionContext
+ public void usedContext(ActionContext.ActionContextRegistry actionContextRegistry) {
+ actionContextRegistry
.getContext(DynamicStrategyRegistry.class)
- .notifyUsedDynamic(actionExecutionContext);
+ .notifyUsedDynamic(actionContextRegistry);
}
private static FileOutErr getSuffixedFileOutErr(FileOutErr fileOutErr, String suffix) {
@@ -366,7 +367,7 @@
private static ImmutableList<SpawnResult> runLocally(
Spawn spawn,
ActionExecutionContext actionExecutionContext,
- @Nullable StopConcurrentSpawns stopConcurrentSpawns)
+ @Nullable SandboxedSpawnStrategy.StopConcurrentSpawns stopConcurrentSpawns)
throws ExecException, InterruptedException {
DynamicStrategyRegistry dynamicStrategyRegistry =
actionExecutionContext.getContext(DynamicStrategyRegistry.class);
@@ -383,7 +384,7 @@
private static ImmutableList<SpawnResult> runRemotely(
Spawn spawn,
ActionExecutionContext actionExecutionContext,
- @Nullable StopConcurrentSpawns stopConcurrentSpawns)
+ @Nullable SandboxedSpawnStrategy.StopConcurrentSpawns stopConcurrentSpawns)
throws ExecException, InterruptedException {
DynamicStrategyRegistry dynamicStrategyRegistry =
actionExecutionContext.getContext(DynamicStrategyRegistry.class);
diff --git a/src/main/java/com/google/devtools/build/lib/dynamic/LegacyDynamicSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/dynamic/LegacyDynamicSpawnStrategy.java
index e459369..c4d395c 100644
--- a/src/main/java/com/google/devtools/build/lib/dynamic/LegacyDynamicSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/dynamic/LegacyDynamicSpawnStrategy.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.Files;
+import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.DynamicStrategyRegistry;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
@@ -303,7 +304,7 @@
}
@Override
- public boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry) {
+ public boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry) {
DynamicStrategyRegistry dynamicStrategyRegistry =
actionContextRegistry.getContext(DynamicStrategyRegistry.class);
diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
index c7ddf47..c29f00b 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
@@ -84,7 +84,7 @@
}
@Override
- public boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry) {
+ public boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry) {
return spawnRunner.canExec(spawn);
}
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 96e24fa..96bba6f 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
@@ -60,7 +60,8 @@
Reporter reporter,
Clock clock,
OptionsProvider options,
- SpawnActionContextMaps spawnActionContextMaps) {
+ ModuleActionContextRegistry actionContextRegistry,
+ SpawnStrategyRegistry spawnStrategyRegistry) {
ExecutionOptions executionOptions = options.getOptions(ExecutionOptions.class);
this.verboseFailures = executionOptions.verboseFailures;
this.showSubcommands = executionOptions.showSubcommands;
@@ -68,13 +69,15 @@
this.execRoot = execRoot;
this.clock = clock;
this.options = options;
- this.actionContextRegistry = spawnActionContextMaps;
+ this.actionContextRegistry = actionContextRegistry;
if (executionOptions.debugPrintActionContexts) {
- spawnActionContextMaps.debugPrintSpawnActionContextMaps(reporter);
+ spawnStrategyRegistry.writeSpawnStrategiesTo(reporter);
+ actionContextRegistry.writeActionContextsTo(reporter);
}
- spawnActionContextMaps.notifyUsed();
+ actionContextRegistry.notifyUsed();
+ spawnStrategyRegistry.notifyUsed(actionContextRegistry);
}
@Override
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 73da12989..b0d1986 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
@@ -15,15 +15,9 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
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.SpawnStrategy;
-import com.google.devtools.build.lib.util.RegexFilter;
import java.util.LinkedHashSet;
-import java.util.List;
import java.util.Set;
/**
@@ -31,15 +25,9 @@
* which allows modules to affect how the executor is initialized.
*/
public class ExecutorBuilder {
- private final SpawnActionContextMaps.Builder spawnActionContextMapsBuilder =
- new SpawnActionContextMaps.Builder();
private final Set<ExecutorLifecycleListener> executorLifecycleListeners = new LinkedHashSet<>();
private ActionInputPrefetcher prefetcher;
- public SpawnActionContextMaps getSpawnActionContextMaps() throws ExecutorInitException {
- return spawnActionContextMapsBuilder.build();
- }
-
/** Returns all executor lifecycle listeners registered with this builder so far. */
public ImmutableSet<ExecutorLifecycleListener> getExecutorLifecycleListeners() {
return ImmutableSet.copyOf(executorLifecycleListeners);
@@ -50,98 +38,6 @@
}
/**
- * Adds the specified action context to the executor, by wrapping it in a simple action context
- * provider implementation.
- *
- * <p>If two action contexts are registered that share an identifying type and commandline
- * identifier the last registered will take precedence.
- */
- public <T extends ActionContext> ExecutorBuilder addActionContext(
- Class<T> identifyingType, T context, String... commandlineIdentifiers) {
- spawnActionContextMapsBuilder.addContext(identifyingType, context, commandlineIdentifiers);
- return this;
- }
-
- /**
- * Sets the strategy names for a given action mnemonic.
- *
- * <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, List<String> strategies) {
- spawnActionContextMapsBuilder.strategyByMnemonicMap().replaceValues(mnemonic, strategies);
- return this;
- }
-
- /**
- * Sets the strategy names to use in the remote branch of dynamic execution for a given action
- * mnemonic.
- *
- * <p>During execution, each strategy is {@linkplain SpawnStrategy#canExec(Spawn,
- * ActionContext.ActionContextRegistry) asked} whether it can execute a given Spawn. The first
- * strategy in the list that says so will get the job.
- */
- public ExecutorBuilder addDynamicRemoteStrategiesByMnemonic(
- String mnemonic, List<String> strategies) {
- spawnActionContextMapsBuilder
- .remoteDynamicStrategyByMnemonicMap()
- .replaceValues(mnemonic, strategies);
- return this;
- }
-
- /**
- * Sets the strategy names to use in the local branch of dynamic execution for a given action
- * mnemonic.
- *
- * <p>During execution, each strategy is {@linkplain SpawnStrategy#canExec(Spawn,
- * ActionContext.ActionContextRegistry) asked} whether it can execute a given Spawn. The first
- * strategy in the list that says so will get the job.
- */
- public ExecutorBuilder addDynamicLocalStrategiesByMnemonic(
- String mnemonic, List<String> strategies) {
- spawnActionContextMapsBuilder
- .localDynamicStrategyByMnemonicMap()
- .replaceValues(mnemonic, strategies);
- return this;
- }
-
- /** Sets the strategy name to use if remote execution is not possible. */
- public ExecutorBuilder setRemoteFallbackStrategy(String remoteLocalFallbackStrategy) {
- spawnActionContextMapsBuilder.setRemoteFallbackStrategy(remoteLocalFallbackStrategy);
- return this;
- }
-
- /**
- * Adds an implementation with a specific strategy name.
- *
- * <p>Modules are free to provide different implementations of {@code ActionContext}. 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: a module requires {@code MyCustomActionContext} to be available, but doesn't
- * associate it with any strategy. Call <code>
- * addStrategyByContext(MyCustomActionContext.class, "")</code>.
- *
- * <p>Example: a module requires {@code MyLocalCustomActionContext} to be available, and wants it
- * to always use the "local" strategy. Call <code>
- * addStrategyByContext(MyCustomActionContext.class, "local")</code>.
- */
- public ExecutorBuilder addStrategyByContext(
- Class<? extends ActionContext> actionContext, String strategy) {
- spawnActionContextMapsBuilder.strategyByContextMap().put(actionContext, strategy);
- return this;
- }
-
- /**
- * 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, List<String> strategy) {
- spawnActionContextMapsBuilder.addStrategyByRegexp(regexFilter, strategy);
- return this;
- }
-
- /**
* Sets the action input prefetcher. Only one module may set the prefetcher. If multiple modules
* set it, this method will throw an {@link IllegalStateException}.
*/
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
deleted file mode 100644
index f28cdda..0000000
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
+++ /dev/null
@@ -1,540 +0,0 @@
-// 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.Preconditions;
-import com.google.common.base.Strings;
-import com.google.common.collect.HashBasedTable;
-import com.google.common.collect.ImmutableClassToInstanceMap;
-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.ImmutableSet;
-import com.google.common.collect.ImmutableSortedMap;
-import com.google.common.collect.LinkedHashMultimap;
-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.DynamicStrategyRegistry;
-import com.google.devtools.build.lib.actions.ExecutorInitException;
-import com.google.devtools.build.lib.actions.SandboxedSpawnStrategy;
-import com.google.devtools.build.lib.actions.Spawn;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
-import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.events.EventHandler;
-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.ArrayList;
-import java.util.Collection;
-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;
-import java.util.stream.Collectors;
-import javax.annotation.Nullable;
-
-/**
- * Container for looking up the {@link ActionContext} to use for a given action.
- *
- * <p>Holds {@link ActionContext} mappings populated by 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
- implements DynamicStrategyRegistry,
- RemoteLocalFallbackRegistry,
- ActionContext.ActionContextRegistry {
-
- /** A stored entry for a {@link RegexFilter} to {@link SpawnStrategy} mapping. */
- @AutoValue
- public abstract static class RegexFilterSpawnStrategy {
- public abstract RegexFilter regexFilter();
-
- public abstract ImmutableList<SpawnStrategy> strategies();
- }
-
- private final ImmutableSortedMap<String, List<SpawnStrategy>> mnemonicToSpawnStrategiesMap;
- private final ImmutableClassToInstanceMap<ActionContext> strategies;
- private final ImmutableList<RegexFilterSpawnStrategy> spawnStrategyRegexList;
- private final ImmutableMultimap<String, SandboxedSpawnStrategy> mnemonicToRemoteDynamicStrategies;
- private final ImmutableMultimap<String, SandboxedSpawnStrategy> mnemonicToLocalDynamicStrategies;
- private final ImmutableMap<Class<? extends ActionContext>, ActionContext> contextMap;
- @Nullable private final AbstractSpawnStrategy remoteLocalFallbackStrategy;
-
- private SpawnActionContextMaps(
- ImmutableSortedMap<String, List<SpawnStrategy>> mnemonicToSpawnStrategiesMap,
- ImmutableClassToInstanceMap<ActionContext> strategies,
- ImmutableList<RegexFilterSpawnStrategy> spawnStrategyRegexList,
- ImmutableMultimap<String, SandboxedSpawnStrategy> mnemonicToRemoteDynamicStrategies,
- ImmutableMultimap<String, SandboxedSpawnStrategy> mnemonicToLocalDynamicStrategies,
- AbstractSpawnStrategy remoteLocalFallbackStrategy) {
- this.mnemonicToSpawnStrategiesMap = mnemonicToSpawnStrategiesMap;
- this.strategies = strategies;
- this.spawnStrategyRegexList = spawnStrategyRegexList;
- this.mnemonicToRemoteDynamicStrategies = mnemonicToRemoteDynamicStrategies;
- this.mnemonicToLocalDynamicStrategies = mnemonicToLocalDynamicStrategies;
- this.remoteLocalFallbackStrategy = remoteLocalFallbackStrategy;
- contextMap = createContextMap();
- }
-
- /**
- * 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}.
- */
- List<SpawnStrategy> getSpawnActionContexts(Spawn spawn, EventHandler reporter) {
- Preconditions.checkNotNull(spawn);
- if (!spawnStrategyRegexList.isEmpty() && spawn.getResourceOwner() != null
- // Don't override test strategies by --strategy_regexp for backwards compatibility.
- && !"TestRunner".equals(spawn.getMnemonic())) {
- String description = spawn.getResourceOwner().getProgressMessage();
- if (description != null) {
- for (RegexFilterSpawnStrategy entry : spawnStrategyRegexList) {
- if (entry.regexFilter().isIncluded(description) && entry.strategies() != null) {
- reporter.handle(
- Event.progress(description + " with context " + entry.strategies().toString()));
- return entry.strategies();
- }
- }
- }
- }
- List<SpawnStrategy> strategies = mnemonicToSpawnStrategiesMap.get(spawn.getMnemonic());
- if (strategies != null) {
- return strategies;
- }
- return Preconditions.checkNotNull(mnemonicToSpawnStrategiesMap.get(""));
- }
-
- @Override
- public List<SandboxedSpawnStrategy> getDynamicSpawnActionContexts(
- Spawn spawn, DynamicMode dynamicMode) {
- ImmutableMultimap<String, SandboxedSpawnStrategy> mnemonicToDynamicStrategies =
- dynamicMode == DynamicStrategyRegistry.DynamicMode.REMOTE
- ? mnemonicToRemoteDynamicStrategies
- : mnemonicToLocalDynamicStrategies;
- return ImmutableList.<SandboxedSpawnStrategy>builder()
- .addAll(mnemonicToDynamicStrategies.get(spawn.getMnemonic()))
- .addAll(mnemonicToDynamicStrategies.get(""))
- .build();
- }
-
- @Nullable
- @Override
- public AbstractSpawnStrategy getRemoteLocalFallbackStrategy() {
- return remoteLocalFallbackStrategy;
- }
-
- private ImmutableMap<Class<? extends ActionContext>, ActionContext> createContextMap() {
- Map<Class<? extends ActionContext>, ActionContext> contextMap = new HashMap<>();
- for (Map.Entry<Class<? extends ActionContext>, ActionContext> typeToStrategy :
- strategies.entrySet()) {
- ActionContext strategy = typeToStrategy.getValue();
- contextMap.put(typeToStrategy.getKey(), strategy);
- contextMap.put(strategy.getClass(), strategy);
- }
- contextMap.put(SpawnStrategy.class, new ProxySpawnActionContext(this));
- contextMap.put(DynamicStrategyRegistry.class, this);
- contextMap.put(RemoteLocalFallbackRegistry.class, this);
- return ImmutableMap.copyOf(contextMap);
- }
-
- @Nullable
- @Override
- public <T extends ActionContext> T getContext(Class<T> identifyingType) {
- return identifyingType.cast(contextMap.get(identifyingType));
- }
-
- /** Returns a list of all referenced {@link ActionContext} instances. */
- @VisibleForTesting
- 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<>(strategies.values());
- mnemonicToSpawnStrategiesMap.values().forEach(allContexts::addAll);
- spawnStrategyRegexList.forEach(x -> allContexts.addAll(x.strategies()));
- return ImmutableList.copyOf(allContexts);
- }
-
- /**
- * Notifies all (non-dynamic) contexts stored in this context map that they are {@link
- * ActionContext#usedContext used}.
- */
- public void notifyUsed() {
- for (ActionContext context : allContexts()) {
- context.usedContext(this);
- }
- }
-
- @Override
- public void notifyUsedDynamic(ActionContextRegistry actionContextRegistry) {
- for (SandboxedSpawnStrategy context : mnemonicToRemoteDynamicStrategies.values()) {
- context.usedContext(actionContextRegistry);
- }
-
- for (SandboxedSpawnStrategy context : mnemonicToLocalDynamicStrategies.values()) {
- context.usedContext(actionContextRegistry);
- }
- }
-
- /**
- * Print a sorted list of our (Spawn)ActionContext maps.
- *
- * <p>Prints out debug information about the mappings.
- */
- void debugPrintSpawnActionContextMaps(Reporter reporter) {
- for (Entry<String, List<SpawnStrategy>> 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(), Joiner.on(", ").join(strategyNames))));
- }
-
- ImmutableMap<Class<? extends ActionContext>, ActionContext> contextMap = createContextMap();
- TreeMap<String, String> sortedContextMapWithSimpleNames = new TreeMap<>();
- for (Map.Entry<Class<? extends ActionContext>, ActionContext> entry : contextMap.entrySet()) {
- sortedContextMapWithSimpleNames.put(
- entry.getKey().getSimpleName(), entry.getValue().getClass().getSimpleName());
- }
- for (Map.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 (RegexFilterSpawnStrategy entry : spawnStrategyRegexList) {
- reporter.handle(
- Event.info(
- String.format(
- "SpawnActionContextMap: \"%s\" = %s",
- entry.regexFilter().toString(), entry.strategies().getClass().getSimpleName())));
- }
- }
-
- @VisibleForTesting
- public static SpawnActionContextMaps createStub(
- Map<Class<? extends ActionContext>, ActionContext> strategies,
- Map<String, List<SpawnStrategy>> spawnStrategyMnemonicMap) {
- return new SpawnActionContextMaps(
- ImmutableSortedMap.copyOf(spawnStrategyMnemonicMap, String.CASE_INSENSITIVE_ORDER),
- ImmutableClassToInstanceMap.copyOf(strategies),
- ImmutableList.of(),
- ImmutableMultimap.of(),
- ImmutableMultimap.of(),
- /* remoteLocalFallbackStrategy=*/ null);
- }
-
- /** A stored entry for a {@link RegexFilter} to {@code strategy} mapping. */
- @AutoValue
- public abstract static class RegexFilterStrategy {
- public abstract RegexFilter regexFilter();
-
- public abstract ImmutableList<String> strategy();
- }
-
- /** Builder for {@code SpawnActionContextMaps}. */
- public static final class Builder {
- private final LinkedHashMultimap<String, String> strategyByMnemonicMap =
- LinkedHashMultimap.create();
- private ImmutableListMultimap.Builder<Class<? extends ActionContext>, String>
- strategyByContextMapBuilder = ImmutableListMultimap.builder();
- private final ImmutableList.Builder<RegexFilterStrategy> strategyByRegexpBuilder =
- ImmutableList.builder();
- private final LinkedHashMultimap<String, String> remoteDynamicStrategyByMnemonicMap =
- LinkedHashMultimap.create();
- private final LinkedHashMultimap<String, String> localDynamicStrategyByMnemonicMap =
- LinkedHashMultimap.create();
- private final List<ActionContextInformation<?>> actionContexts = new ArrayList<>();
- @Nullable private String remoteLocalFallbackStrategyName;
-
- /**
- * 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 LinkedHashMultimap<String, String> strategyByMnemonicMap() {
- return strategyByMnemonicMap;
- }
-
- /**
- * Returns a builder modules can use to add mappings from mnemonics to strategy names for use in
- * the remote branch of dynamic execution.
- *
- * <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 LinkedHashMultimap<String, String> remoteDynamicStrategyByMnemonicMap() {
- return remoteDynamicStrategyByMnemonicMap;
- }
-
- /**
- * Returns a builder modules can use to add mappings from mnemonics to strategy names for use in
- * the local branch of dynamic execution.
- *
- * <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 LinkedHashMultimap<String, String> localDynamicStrategyByMnemonicMap() {
- return localDynamicStrategyByMnemonicMap;
- }
-
- /**
- * Sets the command-line identifier of the strategy to be used when falling back from remote to
- * local execution.
- *
- * <p>Note that this is an optional setting, if not provided {@link
- * SpawnActionContextMaps#getRemoteLocalFallbackStrategy()} will return {@code null}. If the
- * value <b>is</b> provided it must match the commandline identifier of a registered strategy
- * (at {@linkplain #build build} time).
- */
- public void setRemoteFallbackStrategy(String remoteLocalFallbackStrategy) {
- this.remoteLocalFallbackStrategyName = remoteLocalFallbackStrategy;
- }
-
- /**
- * 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, List<String> strategy) {
- strategyByRegexpBuilder.add(
- new AutoValue_SpawnActionContextMaps_RegexFilterStrategy(
- regexFilter, ImmutableList.copyOf(strategy)));
- }
-
- /**
- * Adds a context implementation to this map with the given identifying type and command-line
- * identifiers.
- *
- * <p>If two contexts are added for the same identifying type and they are not distinguished by
- * a restriction to a different command-line identifier then the last registered implementation
- * is used.
- */
- public <T extends ActionContext> Builder addContext(
- Class<T> identifyingType, T context, String... commandLineIdentifiers) {
- actionContexts.add(
- new AutoValue_SpawnActionContextMaps_ActionContextInformation<>(
- context, identifyingType, ImmutableList.copyOf(commandLineIdentifiers)));
- return this;
- }
-
- /** Builds a {@link SpawnActionContextMaps} instance. */
- public SpawnActionContextMaps build() throws ExecutorInitException {
- StrategyConverter strategyConverter = new StrategyConverter(actionContexts);
-
- ImmutableSortedMap.Builder<String, List<SpawnStrategy>> spawnStrategyMap =
- ImmutableSortedMap.orderedBy(String.CASE_INSENSITIVE_ORDER);
- HashMap<Class<? extends ActionContext>, ActionContext> strategies = new HashMap<>();
- ImmutableList.Builder<RegexFilterSpawnStrategy> spawnStrategyRegexList =
- ImmutableList.builder();
-
- for (String mnemonic : strategyByMnemonicMap.keySet()) {
- ImmutableList.Builder<SpawnStrategy> spawnStrategies = ImmutableList.builder();
- Set<String> strategiesForMnemonic = strategyByMnemonicMap.get(mnemonic);
- for (String strategy : strategiesForMnemonic) {
- SpawnStrategy spawnStrategy =
- strategyConverter.getStrategy(SpawnStrategy.class, strategy);
- if (spawnStrategy == null) {
- String strategyOrNull = Strings.emptyToNull(strategy);
- throw makeExceptionForInvalidStrategyValue(
- strategy,
- Joiner.on(' ').skipNulls().join(strategyOrNull, "spawn"),
- strategyConverter.getValidValues(SpawnStrategy.class));
- }
- spawnStrategies.add(spawnStrategy);
- }
- spawnStrategyMap.put(mnemonic, spawnStrategies.build());
- }
-
- 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.put(entry.getKey(), context);
- }
-
- for (RegexFilterStrategy entry : strategyByRegexpBuilder.build()) {
- ImmutableList.Builder<SpawnStrategy> spawnStrategies = ImmutableList.builder();
- List<String> strategiesForRegex = entry.strategy();
- for (String strategy : strategiesForRegex) {
- SpawnStrategy spawnStrategy =
- strategyConverter.getStrategy(SpawnStrategy.class, strategy);
- if (spawnStrategy == null) {
- strategy = Strings.emptyToNull(strategy);
- throw makeExceptionForInvalidStrategyValue(
- entry.regexFilter().toString(),
- Joiner.on(' ').skipNulls().join(strategy, "spawn"),
- strategyConverter.getValidValues(SpawnStrategy.class));
- }
- spawnStrategies.add(spawnStrategy);
- }
- spawnStrategyRegexList.add(
- new AutoValue_SpawnActionContextMaps_RegexFilterSpawnStrategy(
- entry.regexFilter(), spawnStrategies.build()));
- }
-
- AbstractSpawnStrategy remoteLocalFallbackStrategy = null;
- if (remoteLocalFallbackStrategyName != null) {
- SpawnStrategy strategy =
- strategyConverter.getStrategy(SpawnStrategy.class, remoteLocalFallbackStrategyName);
- if (!(strategy instanceof AbstractSpawnStrategy)) {
- throw makeExceptionForInvalidStrategyValue(
- remoteLocalFallbackStrategyName,
- "remote local fallback",
- strategyConverter.getValidValues(SpawnStrategy.class, "remote"));
- }
- remoteLocalFallbackStrategy = (AbstractSpawnStrategy) strategy;
- }
-
- return new SpawnActionContextMaps(
- spawnStrategyMap.build(),
- ImmutableClassToInstanceMap.copyOf(strategies),
- spawnStrategyRegexList.build(),
- toActionContexts(strategyConverter, remoteDynamicStrategyByMnemonicMap),
- toActionContexts(strategyConverter, localDynamicStrategyByMnemonicMap),
- remoteLocalFallbackStrategy);
- }
-
- private ImmutableMultimap<String, SandboxedSpawnStrategy> toActionContexts(
- StrategyConverter strategyConverter,
- LinkedHashMultimap<String, String> dynamicStrategyByMnemonicMap)
- throws ExecutorInitException {
- ImmutableMultimap.Builder<String, SandboxedSpawnStrategy> mnemonicToStrategies =
- ImmutableMultimap.builder();
- for (Entry<String, Collection<String>> mnemonicToIdentifiers :
- dynamicStrategyByMnemonicMap.asMap().entrySet()) {
- for (String identifier : mnemonicToIdentifiers.getValue()) {
- if (identifier.isEmpty()) {
- continue;
- }
- SpawnStrategy strategy = strategyConverter.getStrategy(SpawnStrategy.class, identifier);
- if (strategy == null) {
- throw makeExceptionForInvalidStrategyValue(
- identifier,
- Joiner.on(' ').skipNulls().join(Strings.emptyToNull(identifier), "spawn"),
- strategyConverter.getValidValues(SpawnStrategy.class));
- }
- if (!(strategy instanceof SandboxedSpawnStrategy)) {
- throw new ExecutorInitException(
- "Requested strategy " + identifier + " exists but does not support sandboxing");
- }
- mnemonicToStrategies.put(
- mnemonicToIdentifiers.getKey(), (SandboxedSpawnStrategy) strategy);
- }
- }
- return mnemonicToStrategies.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}. */
- private StrategyConverter(List<ActionContextInformation<?>> actionContexts) {
- for (ActionContextInformation<?> contextInformation : actionContexts) {
- defaultClassMap.put(contextInformation.identifyingType(), contextInformation.context());
-
- for (String name : contextInformation.commandLineIdentifiers()) {
- classMap.put(contextInformation.identifyingType(), name, contextInformation.context());
- }
- }
- }
-
- @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, String... excludes) {
- ImmutableSet<String> excludedNames = ImmutableSet.copyOf(excludes);
- return classMap.row(context).keySet().stream()
- .filter(s -> !excludedNames.contains(s))
- .sorted()
- .collect(Collectors.joining(", "));
- }
-
- private String getUserFriendlyName(Class<? extends ActionContext> context) {
- ActionContextMarker marker = context.getAnnotation(ActionContextMarker.class);
- return marker != null ? marker.name() : context.getSimpleName();
- }
- }
-
- @AutoValue
- abstract static class ActionContextInformation<T extends ActionContext> {
- abstract T context();
-
- abstract Class<T> identifyingType();
-
- abstract ImmutableList<String> commandLineIdentifiers();
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyResolver.java
similarity index 66%
rename from src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java
rename to src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyResolver.java
index 8d2d9fc..e231d96 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ProxySpawnActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyResolver.java
@@ -15,6 +15,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.Spawn;
@@ -22,32 +23,34 @@
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.actions.UserExecException;
-import com.google.devtools.build.lib.events.NullEventHandler;
import java.util.List;
import java.util.stream.Collectors;
-/** Proxy that looks up the right SpawnActionContext for a spawn during {@link #exec}. */
-public final class ProxySpawnActionContext implements SpawnStrategy {
-
- private final SpawnActionContextMaps spawnActionContextMaps;
+/**
+ * Resolver that looks up the right strategy for a spawn during {@link #exec} (via a {@link
+ * SpawnStrategyRegistry}) and uses it to execute the spawn.
+ */
+public final class SpawnStrategyResolver implements ActionContext {
/**
- * Creates a new {@link ProxySpawnActionContext}.
+ * Executes the given spawn with the {@linkplain SpawnStrategyRegistry highest priority strategy}
+ * that can be found for it.
*
- * @param spawnActionContextMaps The {@link SpawnActionContextMaps} to use to decide which {@link
- * SpawnStrategy} should execute a given {@link Spawn} during {@link #exec}.
+ * @param actionExecutionContext context in which to execute the spawn
+ * @return result(s) from the spawn's execution
*/
- public ProxySpawnActionContext(SpawnActionContextMaps spawnActionContextMaps) {
- this.spawnActionContextMaps = spawnActionContextMaps;
- }
-
- @Override
public ImmutableList<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
return resolveOne(spawn, actionExecutionContext).exec(spawn, actionExecutionContext);
}
- @Override
+ /**
+ * Queues execution of the given spawn with the {@linkplain SpawnStrategyRegistry highest priority
+ * strategy} that can be found for it.
+ *
+ * @param actionExecutionContext context in which to execute the spawn
+ * @return handle to the spawn's pending execution (or failure thereof)
+ */
public SpawnContinuation beginExecution(
Spawn spawn, ActionExecutionContext actionExecutionContext) throws InterruptedException {
SpawnStrategy resolvedStrategy;
@@ -61,7 +64,7 @@
private SpawnStrategy resolveOne(Spawn spawn, ActionExecutionContext actionExecutionContext)
throws UserExecException {
- List<SpawnStrategy> strategies = resolve(spawn, actionExecutionContext);
+ List<? extends SpawnStrategy> strategies = resolve(spawn, actionExecutionContext);
// Because the strategies are ordered by preference, we can execute the spawn with the best
// possible one by simply filtering out the ones that can't execute it and then picking the
@@ -72,16 +75,15 @@
/**
* Returns the list of {@link SpawnStrategy}s that should be used to execute the given spawn.
*
- * @param spawn The spawn for which the correct {@link SpawnStrategy} should be determined.
- * @param eventHandler An event handler that can be used to print messages while resolving the
- * correct {@link SpawnStrategy} for the given spawn.
+ * @param spawn spawn for which the correct {@link SpawnStrategy} should be determined
*/
@VisibleForTesting
- public List<SpawnStrategy> resolve(Spawn spawn, ActionExecutionContext actionExecutionContext)
- throws UserExecException {
- List<SpawnStrategy> strategies =
- spawnActionContextMaps.getSpawnActionContexts(
- spawn, actionExecutionContext.getEventHandler());
+ public List<? extends SpawnStrategy> resolve(
+ Spawn spawn, ActionExecutionContext actionExecutionContext) throws UserExecException {
+ List<? extends SpawnStrategy> strategies =
+ actionExecutionContext
+ .getContext(SpawnStrategyRegistry.class)
+ .getStrategies(spawn, actionExecutionContext.getEventHandler());
strategies =
strategies.stream()
@@ -100,10 +102,4 @@
return strategies;
}
-
- @Override
- public boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry) {
- return spawnActionContextMaps.getSpawnActionContexts(spawn, NullEventHandler.INSTANCE).stream()
- .anyMatch(spawnActionContext -> spawnActionContext.canExec(spawn, actionContextRegistry));
- }
}
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 6d68f36..dd32ba7 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
@@ -31,7 +31,6 @@
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnContinuation;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.actions.TestExecException;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
@@ -304,7 +303,7 @@
long startTimeMillis = actionExecutionContext.getClock().currentTimeMillis();
SpawnContinuation spawnContinuation =
actionExecutionContext
- .getContext(SpawnStrategy.class)
+ .getContext(SpawnStrategyResolver.class)
.beginExecution(spawn, actionExecutionContext.withFileOutErr(testOutErr));
return new BazelTestAttemptContinuation(
testAction,
@@ -603,13 +602,14 @@
&& fileOutErr.getOutputPath().exists()
&& !xmlOutputPath.exists()) {
Spawn xmlGeneratingSpawn = createXmlGeneratingSpawn(testAction, primaryResult);
- SpawnStrategy strategy = actionExecutionContext.getContext(SpawnStrategy.class);
+ SpawnStrategyResolver spawnStrategyResolver =
+ actionExecutionContext.getContext(SpawnStrategyResolver.class);
// We treat all failures to generate the test.xml here as catastrophic, and won't rerun
// the test if this fails. We redirect the output to a temporary file.
FileOutErr xmlSpawnOutErr = actionExecutionContext.getFileOutErr().childOutErr();
try {
SpawnContinuation xmlContinuation =
- strategy.beginExecution(
+ spawnStrategyResolver.beginExecution(
xmlGeneratingSpawn, actionExecutionContext.withFileOutErr(xmlSpawnOutErr));
return new BazelXmlCreationContinuation(
resolvedPaths, xmlSpawnOutErr, builder, spawnResults, xmlContinuation);
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
index 9e9ddb0..67be91e 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadHostile;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.includescanning.IncludeParser.Inclusion;
import com.google.devtools.build.lib.rules.cpp.CppIncludeExtractionContext;
import com.google.devtools.build.lib.rules.cpp.CppIncludeScanningContext;
@@ -75,6 +76,7 @@
private final MutableSupplier<SpawnIncludeScanner> spawnIncludeScannerSupplier =
new MutableSupplier<>();
private final MutableSupplier<ArtifactFactory> artifactFactory = new MutableSupplier<>();
+ private IncludeScannerLifecycleManager lifecycleManager;
protected PathFragment getIncludeHintsFilename() {
return INCLUDE_HINTS_FILENAME;
@@ -82,18 +84,22 @@
@Override
@ThreadHostile
- public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
- IncludeScannerLifecycleManager lifecycleManager =
+ public void registerActionContexts(
+ ModuleActionContextRegistry.Builder registryBuilder,
+ CommandEnvironment env,
+ BuildRequest buildRequest) {
+ registryBuilder
+ .register(CppIncludeExtractionContext.class, new CppIncludeExtractionContextImpl(env))
+ .register(SwigIncludeScanningContext.class, lifecycleManager.getSwigActionContext())
+ .register(CppIncludeScanningContext.class, lifecycleManager.getCppActionContext());
+ }
+
+ @Override
+ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder)
+ throws ExecutorInitException {
+ lifecycleManager =
new IncludeScannerLifecycleManager(env, request, spawnIncludeScannerSupplier);
- builder
- .addExecutorLifecycleListener(lifecycleManager)
- .addActionContext(
- CppIncludeExtractionContext.class, new CppIncludeExtractionContextImpl(env))
- .addActionContext(SwigIncludeScanningContext.class, lifecycleManager.getSwigActionContext())
- .addActionContext(CppIncludeScanningContext.class, lifecycleManager.getCppActionContext())
- .addStrategyByContext(CppIncludeExtractionContext.class, "")
- .addStrategyByContext(SwigIncludeScanningContext.class, "")
- .addStrategyByContext(CppIncludeScanningContext.class, "");
+ builder.addExecutorLifecycleListener(lifecycleManager);
}
@Override
@@ -112,6 +118,7 @@
public void afterCommand() {
spawnIncludeScannerSupplier.set(null);
artifactFactory.set(null);
+ lifecycleManager = null;
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java
index 6252111..8d712ea 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java
@@ -41,11 +41,11 @@
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnContinuation;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.includescanning.IncludeParser.GrepIncludesFileType;
import com.google.devtools.build.lib.includescanning.IncludeParser.Inclusion;
import com.google.devtools.build.lib.util.io.FileOutErr;
@@ -374,11 +374,12 @@
// Don't share the originalOutErr across spawnGrep calls. Doing so would not be thread-safe.
FileOutErr originalOutErr = actionExecutionContext.getFileOutErr();
FileOutErr grepOutErr = originalOutErr.childOutErr();
- SpawnStrategy strategy = actionExecutionContext.getContext(SpawnStrategy.class);
+ SpawnStrategyResolver spawnStrategyResolver =
+ actionExecutionContext.getContext(SpawnStrategyResolver.class);
ActionExecutionContext spawnContext = actionExecutionContext.withFileOutErr(grepOutErr);
List<SpawnResult> results;
try {
- results = strategy.exec(spawn, spawnContext);
+ results = spawnStrategyResolver.exec(spawn, spawnContext);
dump(spawnContext, actionExecutionContext);
} catch (ExecException e) {
dump(spawnContext, actionExecutionContext);
@@ -500,7 +501,7 @@
SpawnContinuation spawnContinuation;
try {
spawnContinuation =
- grepContext.getContext(SpawnStrategy.class).beginExecution(spawn, grepContext);
+ grepContext.getContext(SpawnStrategyResolver.class).beginExecution(spawn, grepContext);
} catch (InterruptedException e) {
dump(grepContext, actionExecutionContext);
return Futures.immediateCancelledFuture();
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index 0a0ed81..3506993 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -22,12 +22,12 @@
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ExecutorInitException;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.analysis.ArtifactsToOwnerLabels;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.SpawnCache;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -80,45 +80,59 @@
env, cache, executor, retryScheduler, digestUtil, logDir);
}
- /** Registers the action contexts whose lifecycle this class manages. */
- public void registerActionContexts(ExecutorBuilder executorBuilder) {
- ExecutionOptions executionOptions =
- checkNotNull(env.getOptions().getOptions(ExecutionOptions.class));
- RemoteOptions remoteOptions = checkNotNull(env.getOptions().getOptions(RemoteOptions.class));
- String buildRequestId = env.getBuildRequestId();
- String commandId = env.getCommandId().toString();
-
+ /**
+ * Registers a remote spawn strategy if this instance was created with an executor, otherwise does
+ * nothing.
+ *
+ * @param registryBuilder builder with which to register the strategy
+ */
+ public void registerRemoteSpawnStrategyIfApplicable(
+ SpawnStrategyRegistry.Builder registryBuilder) {
if (executor == null) {
- RemoteSpawnCache spawnCache =
- new RemoteSpawnCache(
- env.getExecRoot(),
- remoteOptions,
- cache,
- buildRequestId,
- commandId,
- env.getReporter(),
- digestUtil,
- filesToDownload);
- executorBuilder.addActionContext(SpawnCache.class, spawnCache, "remote-cache");
- } else {
- RemoteSpawnRunner spawnRunner =
- new RemoteSpawnRunner(
- env.getExecRoot(),
- remoteOptions,
- env.getOptions().getOptions(ExecutionOptions.class),
- executionOptions.verboseFailures,
- env.getReporter(),
- buildRequestId,
- commandId,
- (RemoteExecutionCache) cache,
- executor,
- retryScheduler,
- digestUtil,
- logDir,
- filesToDownload);
- executorBuilder.addActionContext(
- SpawnStrategy.class, new RemoteSpawnStrategy(env.getExecRoot(), spawnRunner), "remote");
+ return; // Can't use a spawn strategy without executor.
}
+
+ RemoteSpawnRunner spawnRunner =
+ new RemoteSpawnRunner(
+ env.getExecRoot(),
+ checkNotNull(env.getOptions().getOptions(RemoteOptions.class)),
+ env.getOptions().getOptions(ExecutionOptions.class),
+ checkNotNull(env.getOptions().getOptions(ExecutionOptions.class)).verboseFailures,
+ env.getReporter(),
+ env.getBuildRequestId(),
+ env.getCommandId().toString(),
+ (RemoteExecutionCache) cache,
+ executor,
+ retryScheduler,
+ digestUtil,
+ logDir,
+ filesToDownload);
+ registryBuilder.registerStrategy(
+ new RemoteSpawnStrategy(env.getExecRoot(), spawnRunner), "remote");
+ }
+
+ /**
+ * Registers a spawn cache action context if this instance was created without an executor,
+ * otherwise does nothing.
+ *
+ * @param registryBuilder builder with which to register the cache
+ */
+ public void registerSpawnCacheIfApplicable(ModuleActionContextRegistry.Builder registryBuilder) {
+ if (executor != null) {
+ return; // No need to register cache if we're using a remote executor.
+ }
+
+ RemoteSpawnCache spawnCache =
+ new RemoteSpawnCache(
+ env.getExecRoot(),
+ checkNotNull(env.getOptions().getOptions(RemoteOptions.class)),
+ cache,
+ env.getBuildRequestId(),
+ env.getCommandId().toString(),
+ env.getReporter(),
+ digestUtil,
+ filesToDownload);
+ registryBuilder.register(SpawnCache.class, spawnCache, "remote-cache");
}
/** Returns the remote cache. */
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 0f4bb97..9aa09fc 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -41,6 +41,8 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
import com.google.devtools.build.lib.remote.logging.LoggingInterceptor;
@@ -485,6 +487,31 @@
}
@Override
+ public void registerSpawnStrategies(
+ SpawnStrategyRegistry.Builder registryBuilder, CommandEnvironment env) {
+ if (actionContextProvider == null) {
+ return;
+ }
+ RemoteOptions remoteOptions =
+ Preconditions.checkNotNull(
+ env.getOptions().getOptions(RemoteOptions.class), "RemoteOptions");
+ registryBuilder.setRemoteLocalFallbackStrategyIdentifier(
+ remoteOptions.remoteLocalFallbackStrategy);
+ actionContextProvider.registerRemoteSpawnStrategyIfApplicable(registryBuilder);
+ }
+
+ @Override
+ public void registerActionContexts(
+ ModuleActionContextRegistry.Builder registryBuilder,
+ CommandEnvironment env,
+ BuildRequest buildRequest) {
+ if (actionContextProvider == null) {
+ return;
+ }
+ actionContextProvider.registerSpawnCacheIfApplicable(registryBuilder);
+ }
+
+ @Override
public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
Preconditions.checkState(actionInputFetcher == null, "actionInputFetcher must be null");
Preconditions.checkNotNull(remoteOutputsMode, "remoteOutputsMode must not be null");
@@ -492,7 +519,6 @@
if (actionContextProvider == null) {
return;
}
- actionContextProvider.registerActionContexts(builder);
builder.addExecutorLifecycleListener(actionContextProvider);
RemoteOptions remoteOptions =
Preconditions.checkNotNull(
@@ -508,8 +534,6 @@
builder.setActionInputPrefetcher(actionInputFetcher);
remoteOutputService.setActionInputFetcher(actionInputFetcher);
}
-
- builder.setRemoteFallbackStrategy(remoteOptions.remoteLocalFallbackStrategy);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index d355fbf..6d38f0d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -49,7 +49,6 @@
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnContinuation;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.actions.extra.CppCompileInfo;
import com.google.devtools.build.lib.actions.extra.EnvironmentVariable;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
@@ -61,6 +60,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -1379,7 +1379,9 @@
}
SpawnContinuation spawnContinuation =
- actionExecutionContext.getContext(SpawnStrategy.class).beginExecution(spawn, spawnContext);
+ actionExecutionContext
+ .getContext(SpawnStrategyResolver.class)
+ .beginExecution(spawn, spawnContext);
return new CppCompileActionContinuation(
actionExecutionContext,
spawnContext,
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 58ef75e..57a7e49 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
@@ -43,7 +43,6 @@
import com.google.devtools.build.lib.actions.SimpleSpawn;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnContinuation;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.actions.extra.CppLinkInfo;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
@@ -53,6 +52,7 @@
import com.google.devtools.build.lib.collect.CollectionUtils;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -314,7 +314,7 @@
Spawn spawn = createSpawn(actionExecutionContext);
SpawnContinuation spawnContinuation =
actionExecutionContext
- .getContext(SpawnStrategy.class)
+ .getContext(SpawnStrategyResolver.class)
.beginExecution(spawn, actionExecutionContext);
return new CppLinkActionContinuation(actionExecutionContext, spawnContinuation);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
index 683f24a..9859733 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
@@ -32,11 +32,11 @@
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.util.ShellEscaper;
@@ -137,8 +137,9 @@
byte[] dotDContents = null;
try {
Spawn spawn = createSpawn(actionExecutionContext.getClientEnv());
- SpawnStrategy strategy = actionExecutionContext.getContext(SpawnStrategy.class);
- spawnResults = strategy.exec(spawn, actionExecutionContext);
+ SpawnStrategyResolver spawnStrategyResolver =
+ actionExecutionContext.getContext(SpawnStrategyResolver.class);
+ spawnResults = spawnStrategyResolver.exec(spawn, actionExecutionContext);
// The SpawnActionContext guarantees that the first list entry is the successful one.
dotDContents = getDotDContents(spawnResults.get(0));
} catch (ExecException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
index a90cd3e..8e70a66 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -49,7 +49,6 @@
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnContinuation;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -59,6 +58,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode;
import com.google.devtools.build.lib.rules.java.JavaPluginInfoProvider.JavaPluginInfo;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -339,7 +339,7 @@
}
SpawnContinuation spawnContinuation =
actionExecutionContext
- .getContext(SpawnStrategy.class)
+ .getContext(SpawnStrategyResolver.class)
.beginExecution(spawn, actionExecutionContext);
return new JavaActionContinuation(actionExecutionContext, reducedClasspath, spawnContinuation);
}
@@ -604,7 +604,7 @@
}
SpawnContinuation fallbackContinuation =
actionExecutionContext
- .getContext(SpawnStrategy.class)
+ .getContext(SpawnStrategyResolver.class)
.beginExecution(spawn, actionExecutionContext);
return new JavaFallbackActionContinuation(
actionExecutionContext, results, fallbackContinuation);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index 8ec26a8..d20e833 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -16,6 +16,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
@@ -29,6 +30,8 @@
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.skyframe.AspectValue;
@@ -291,6 +294,37 @@
throws ExecutorInitException {}
/**
+ * Registers any action contexts this module provides with the execution phase. They will be
+ * available for {@linkplain ActionContext.ActionContextRegistry#getContext querying} to actions
+ * and other action contexts.
+ *
+ * <p>This method is invoked before actions are executed but after {@link #executorInit}.
+ *
+ * @param registryBuilder builder with which to register action contexts
+ * @param env environment for the current command
+ * @param buildRequest the current build request
+ * @throws ExecutorInitException if there are fatal issues creating or registering action contexts
+ */
+ public void registerActionContexts(
+ ModuleActionContextRegistry.Builder registryBuilder,
+ CommandEnvironment env,
+ BuildRequest buildRequest)
+ throws ExecutorInitException {}
+
+ /**
+ * Registers any spawn strategies this module provides with the execution phase.
+ *
+ * <p>This method is invoked before actions are executed but after {@link #executorInit}.
+ *
+ * @param registryBuilder builder with which to register strategies
+ * @param env environment for the current command
+ * @throws ExecutorInitException if there are fatal issues creating or registering strategies
+ */
+ public void registerSpawnStrategies(
+ SpawnStrategyRegistry.Builder registryBuilder, CommandEnvironment env)
+ throws ExecutorInitException {}
+
+ /**
* Called after each command.
*
* @throws AbruptExitException modules can throw this exception to modify the command exit code
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/NoSpawnCacheModule.java b/src/main/java/com/google/devtools/build/lib/runtime/NoSpawnCacheModule.java
index 40a1750..0cdb89b 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/NoSpawnCacheModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/NoSpawnCacheModule.java
@@ -14,7 +14,7 @@
package com.google.devtools.build.lib.runtime;
import com.google.devtools.build.lib.buildtool.BuildRequest;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.SpawnCache;
/**
@@ -23,7 +23,10 @@
public final class NoSpawnCacheModule extends BlazeModule {
@Override
- public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
- builder.addActionContext(SpawnCache.class, SpawnCache.NO_CACHE, "no-cache");
+ public void registerActionContexts(
+ ModuleActionContextRegistry.Builder registryBuilder,
+ CommandEnvironment env,
+ BuildRequest buildRequest) {
+ registryBuilder.register(SpawnCache.class, SpawnCache.NO_CACHE, "no-cache");
}
}
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 bd042e7..115e1ea 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
@@ -24,14 +24,12 @@
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
-import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent;
import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.RunfilesTreeUpdater;
import com.google.devtools.build.lib.exec.SpawnRunner;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
@@ -133,11 +131,12 @@
}
@Override
- public void executorInit(CommandEnvironment cmdEnv, BuildRequest request, ExecutorBuilder builder)
+ public void registerSpawnStrategies(
+ SpawnStrategyRegistry.Builder registryBuilder, CommandEnvironment env)
throws ExecutorInitException {
checkNotNull(env, "env not initialized; was beforeCommand called?");
try {
- setup(cmdEnv, builder);
+ setup(env, registryBuilder);
} catch (IOException e) {
throw new ExecutorInitException("Failed to initialize sandbox", e);
}
@@ -175,7 +174,7 @@
throw new IllegalStateException("Not reachable");
}
- private void setup(CommandEnvironment cmdEnv, ExecutorBuilder builder)
+ private void setup(CommandEnvironment cmdEnv, SpawnStrategyRegistry.Builder builder)
throws IOException {
SandboxOptions options = checkNotNull(env.getOptions().getOptions(SandboxOptions.class));
sandboxBase = computeSandboxBase(options, env);
@@ -275,8 +274,7 @@
timeoutKillDelay,
treeDeleter));
spawnRunners.add(spawnRunner);
- builder.addActionContext(
- SpawnStrategy.class,
+ builder.registerStrategy(
new ProcessWrapperSandboxedStrategy(cmdEnv.getExecRoot(), spawnRunner),
"sandboxed",
"processwrapper-sandbox");
@@ -303,10 +301,8 @@
useCustomizedImages,
treeDeleter));
spawnRunners.add(spawnRunner);
- builder.addActionContext(
- SpawnStrategy.class,
- new DockerSandboxedStrategy(cmdEnv.getExecRoot(), spawnRunner),
- "docker");
+ builder.registerStrategy(
+ new DockerSandboxedStrategy(cmdEnv.getExecRoot(), spawnRunner), "docker");
}
} else if (options.dockerVerbose) {
cmdEnv.getReporter().handle(Event.info(
@@ -327,8 +323,7 @@
options.sandboxfsMapSymlinkTargets,
treeDeleter));
spawnRunners.add(spawnRunner);
- builder.addActionContext(
- SpawnStrategy.class,
+ builder.registerStrategy(
new LinuxSandboxedStrategy(cmdEnv.getExecRoot(), spawnRunner),
"sandboxed",
"linux-sandbox");
@@ -347,8 +342,7 @@
options.sandboxfsMapSymlinkTargets,
treeDeleter));
spawnRunners.add(spawnRunner);
- builder.addActionContext(
- SpawnStrategy.class,
+ builder.registerStrategy(
new DarwinSandboxedStrategy(cmdEnv.getExecRoot(), spawnRunner),
"sandboxed",
"darwin-sandbox");
@@ -360,8 +354,7 @@
cmdEnv,
new WindowsSandboxedSpawnRunner(cmdEnv, timeoutKillDelay, windowsSandboxPath));
spawnRunners.add(spawnRunner);
- builder.addActionContext(
- SpawnStrategy.class,
+ builder.registerStrategy(
new WindowsSandboxedStrategy(cmdEnv.getExecRoot(), spawnRunner),
"sandboxed",
"windows-sandbox");
@@ -371,13 +364,9 @@
|| linuxSandboxSupported
|| darwinSandboxSupported
|| windowsSandboxSupported) {
- // This makes the "sandboxed" strategy available via --spawn_strategy=sandboxed,
- // but it is not necessarily the default.
- builder.addStrategyByContext(SpawnStrategy.class, "sandboxed");
-
- // This makes the "sandboxed" strategy the default Spawn strategy, unless it is
- // overridden by a later BlazeModule.
- builder.addStrategyByMnemonic("", ImmutableList.of("sandboxed"));
+ // This makes the "sandboxed" strategy the default Spawn strategy, unless it is overridden by
+ // a later BlazeModule.
+ builder.setDefaultStrategies(ImmutableList.of("sandboxed"));
}
}
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 8c0193f..0c98bc3 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
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.standalone;
import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext;
import com.google.devtools.build.lib.analysis.actions.LocalTemplateExpansionStrategy;
import com.google.devtools.build.lib.analysis.actions.TemplateExpansionContext;
@@ -22,10 +21,11 @@
import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.dynamic.DynamicExecutionOptions;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.FileWriteStrategy;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.RunfilesTreeUpdater;
import com.google.devtools.build.lib.exec.SpawnRunner;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.exec.StandaloneTestStrategy;
import com.google.devtools.build.lib.exec.TestStrategy;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
@@ -65,11 +65,14 @@
}
@Override
- public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
+ public void registerActionContexts(
+ ModuleActionContextRegistry.Builder registryBuilder,
+ CommandEnvironment env,
+ BuildRequest buildRequest) {
// TODO(ulfjack): Move this to another module.
- builder.addActionContext(
- CppIncludeExtractionContext.class, new DummyCppIncludeExtractionContext(env));
- builder.addActionContext(CppIncludeScanningContext.class, new DummyCppIncludeScanningContext());
+ registryBuilder
+ .register(CppIncludeExtractionContext.class, new DummyCppIncludeExtractionContext(env))
+ .register(CppIncludeScanningContext.class, new DummyCppIncludeScanningContext());
ExecutionOptions executionOptions = env.getOptions().getOptions(ExecutionOptions.class);
Path testTmpRoot =
@@ -80,6 +83,17 @@
env.getBlazeWorkspace().getBinTools(),
testTmpRoot);
+ registryBuilder.register(TestActionContext.class, testStrategy, "standalone");
+ registryBuilder.register(
+ TestActionContext.class, new ExclusiveTestStrategy(testStrategy), "exclusive");
+ registryBuilder.register(FileWriteActionContext.class, new FileWriteStrategy(), "local");
+ registryBuilder.register(
+ TemplateExpansionContext.class, new LocalTemplateExpansionStrategy(), "local");
+ }
+
+ @Override
+ public void registerSpawnStrategies(
+ SpawnStrategyRegistry.Builder registryBuilder, CommandEnvironment env) {
SpawnRunner localSpawnRunner =
new LocalSpawnRunner(
env.getExecRoot(),
@@ -93,24 +107,11 @@
// Order of strategies passed to builder is significant - when there are many strategies that
// could potentially be used and a spawnActionContext doesn't specify which one it wants, the
// last one from strategies list will be used
- builder.addActionContext(
- SpawnStrategy.class,
- new StandaloneSpawnStrategy(env.getExecRoot(), localSpawnRunner),
- "standalone",
- "local");
- builder.addActionContext(TestActionContext.class, testStrategy, "standalone");
- builder.addActionContext(
- TestActionContext.class, new ExclusiveTestStrategy(testStrategy), "exclusive");
- builder.addActionContext(FileWriteActionContext.class, new FileWriteStrategy(), "local");
- builder.addActionContext(
- TemplateExpansionContext.class, new LocalTemplateExpansionStrategy(), "local");
+ registryBuilder.registerStrategy(
+ new StandaloneSpawnStrategy(env.getExecRoot(), localSpawnRunner), "standalone", "local");
- // This makes the "sandboxed" strategy the default Spawn strategy, unless it is overridden by a
+ // This makes the "standalone" strategy the default Spawn strategy, unless it is overridden by a
// later BlazeModule.
- builder.addStrategyByMnemonic("", ImmutableList.of("standalone"));
-
- // This makes the "standalone" strategy available via --spawn_strategy=standalone, but it is not
- // necessarily the default.
- builder.addStrategyByContext(SpawnStrategy.class, "standalone");
+ registryBuilder.setDefaultStrategies(ImmutableList.of("standalone"));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
index 4d521cd..c935833 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
@@ -18,15 +18,13 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.eventbus.Subscribe;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
-import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent;
import com.google.devtools.build.lib.buildtool.buildevent.BuildStartingEvent;
import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.RunfilesTreeUpdater;
import com.google.devtools.build.lib.exec.SpawnRunner;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
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;
@@ -137,7 +135,8 @@
}
@Override
- public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
+ public void registerSpawnStrategies(
+ SpawnStrategyRegistry.Builder registryBuilder, CommandEnvironment env) {
Preconditions.checkNotNull(workerPool);
ImmutableMultimap<String, String> extraFlags =
ImmutableMultimap.copyOf(env.getOptions().getOptions(WorkerOptions.class).workerExtraFlags);
@@ -157,11 +156,8 @@
env.getLocalResourceManager(),
// TODO(buchgr): Replace singleton by a command-scoped RunfilesTreeUpdater
RunfilesTreeUpdater.INSTANCE);
- builder.addActionContext(
- SpawnStrategy.class, new WorkerSpawnStrategy(env.getExecRoot(), spawnRunner), "worker");
-
- builder.addStrategyByContext(SpawnStrategy.class, "standalone");
- builder.addStrategyByContext(SpawnStrategy.class, "worker");
+ registryBuilder.registerStrategy(
+ new WorkerSpawnStrategy(env.getExecRoot(), spawnRunner), "worker");
}
private static SpawnRunner createFallbackRunner(
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
index 80707f5..736b236 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
@@ -60,7 +60,7 @@
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.events.util.EventCollectionApparatus;
import com.google.devtools.build.lib.exec.BinTools;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.includescanning.IncludeScanningModule;
import com.google.devtools.build.lib.integration.util.IntegrationMock;
import com.google.devtools.build.lib.network.ConnectivityStatusProvider;
@@ -304,9 +304,11 @@
}
@Override
- public void executorInit(
- CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
- builder.addActionContext(
+ public void registerActionContexts(
+ ModuleActionContextRegistry.Builder registryBuilder,
+ CommandEnvironment env,
+ BuildRequest buildRequest) {
+ registryBuilder.register(
WorkspaceStatusAction.Context.class, new DummyWorkspaceStatusActionContext());
}
};
diff --git a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
index ef01393..80734a5 100644
--- a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.BaseSpawn;
+import com.google.devtools.build.lib.actions.DynamicStrategyRegistry;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.Executor;
@@ -46,8 +47,8 @@
import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction;
import com.google.devtools.build.lib.exec.BlazeExecutor;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.ExecutorBuilder;
-import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
+import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.testutil.TestThread;
import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.util.io.FileOutErr;
@@ -156,7 +157,7 @@
public ImmutableList<SpawnResult> exec(
Spawn spawn,
ActionExecutionContext actionExecutionContext,
- @Nullable StopConcurrentSpawns stopConcurrentSpawns)
+ @Nullable SandboxedSpawnStrategy.StopConcurrentSpawns stopConcurrentSpawns)
throws ExecException, InterruptedException {
executedSpawn = spawn;
@@ -194,7 +195,7 @@
}
@Override
- public boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry) {
+ public boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry) {
return true;
}
@@ -316,18 +317,25 @@
checkState(executorServiceForCleanup == null);
executorServiceForCleanup = executorService;
- ExecutorBuilder executorBuilder =
- new ExecutorBuilder()
- .addActionContext(SpawnStrategy.class, localStrategy, "mock-local")
- .addActionContext(SpawnStrategy.class, remoteStrategy, "mock-remote");
+ SpawnStrategyRegistry.Builder spawnRegistryBuilder =
+ new SpawnStrategyRegistry.Builder()
+ .registerStrategy(localStrategy, "mock-local")
+ .registerStrategy(remoteStrategy, "mock-remote")
+ .addMnemonicFilter("RunDynamic", ImmutableList.of("dynamic"));
if (sandboxedStrategy != null) {
- executorBuilder.addActionContext(SpawnStrategy.class, sandboxedStrategy, "mock-sandboxed");
+ spawnRegistryBuilder.registerStrategy(sandboxedStrategy, "mock-sandboxed");
}
- new DynamicExecutionModule(executorService).initStrategies(executorBuilder, options);
- SpawnActionContextMaps spawnActionContextMaps = executorBuilder.getSpawnActionContextMaps();
+ new DynamicExecutionModule(executorService)
+ .registerSpawnStrategies(spawnRegistryBuilder, options);
+ SpawnStrategyRegistry strategyRegistry = spawnRegistryBuilder.build();
+ ModuleActionContextRegistry contextRegistry =
+ new ModuleActionContextRegistry.Builder()
+ .register(DynamicStrategyRegistry.class, strategyRegistry)
+ .register(SpawnStrategyRegistry.class, strategyRegistry)
+ .build();
Executor executor =
new BlazeExecutor(
null,
@@ -337,7 +345,8 @@
OptionsParser.builder()
.optionsClasses(ImmutableList.of(ExecutionOptions.class))
.build(),
- spawnActionContextMaps);
+ contextRegistry,
+ strategyRegistry);
ActionExecutionContext actionExecutionContext =
ActionsTestUtil.createContext(
@@ -349,8 +358,12 @@
/*metadataHandler=*/ null,
/*actionGraph=*/ null);
- Optional<ActionContext> optionalContext =
- spawnActionContextMaps.allContexts().stream()
+ List<? extends SpawnStrategy> dynamicStrategies =
+ strategyRegistry.getStrategies(
+ newCustomSpawn("RunDynamic", ImmutableMap.of()), event -> {});
+
+ Optional<? extends SpawnStrategy> optionalContext =
+ dynamicStrategies.stream()
.filter(
c -> c instanceof DynamicSpawnStrategy || c instanceof LegacyDynamicSpawnStrategy)
.findAny();
diff --git a/src/test/java/com/google/devtools/build/lib/exec/BlazeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/exec/BlazeExecutorTest.java
index aa88c4c..cfe4412 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/BlazeExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/BlazeExecutorTest.java
@@ -15,10 +15,10 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
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.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -34,7 +34,6 @@
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
import com.google.devtools.common.options.OptionsParser;
-import javax.annotation.Nullable;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -79,18 +78,11 @@
.setReporter(reporter)
.setOptionsParser(parser)
.setExecution("fake", "fake")
- .addStrategy(SpawnStrategy.class, new FakeSpawnStrategy(), "fake")
+ .addStrategy(new FakeSpawnStrategy(), "fake")
.build();
Event event =
- Iterables.find(
- storedEventHandler.getEvents(),
- new Predicate<Event>() {
- @Override
- public boolean apply(@Nullable Event event) {
- return event.getMessage().contains("SpawnActionContextMap: \"fake\" = ");
- }
- });
+ Iterables.find(storedEventHandler.getEvents(), e -> e.getMessage().contains("\"fake\" = "));
assertThat(event).isNotNull();
assertThat(event.getMessage())
.contains("\"fake\" = [" + strategy.getClass().getSimpleName() + "]");
@@ -105,7 +97,7 @@
}
@Override
- public boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry) {
+ public boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry) {
return false;
}
}
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
deleted file mode 100644
index 508ad07..0000000
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
+++ /dev/null
@@ -1,145 +0,0 @@
-// 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.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
-import com.google.devtools.build.lib.actions.ExecException;
-import com.google.devtools.build.lib.actions.Spawn;
-import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
-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 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 AC1 ac1 = new AC1();
- private static final AC2 ac2 = new AC2();
-
- @Before
- public void setUp() {
- builder =
- new SpawnActionContextMaps.Builder()
- .addContext(SpawnStrategy.class, ac1, "ac1")
- .addContext(SpawnStrategy.class, ac2, "ac2");
- }
-
- @Test
- public void duplicateMnemonics_bothGetStored() throws Exception {
- builder.strategyByMnemonicMap().put("Spawn1", "ac1");
- builder.strategyByMnemonicMap().put("Spawn1", "ac2");
- SpawnActionContextMaps maps = builder.build();
- List<SpawnStrategy> result = maps.getSpawnActionContexts(mockSpawn("Spawn1", null), reporter);
- assertThat(result).containsExactly(ac1, ac2);
- }
-
- @Test
- public void emptyStrategyFallsBackToEmptyMnemonicNotToDefault() throws Exception {
- builder.strategyByMnemonicMap().put("Spawn1", "");
- builder.strategyByMnemonicMap().put("", "ac2");
- SpawnActionContextMaps maps = builder.build();
- List<SpawnStrategy> result = maps.getSpawnActionContexts(mockSpawn("Spawn1", null), reporter);
- assertThat(result).containsExactly(ac2);
- }
-
- @Test
- public void multipleRegexps_firstMatchWins() throws Exception {
- builder.addStrategyByRegexp(converter.convert("foo"), ImmutableList.of("ac1"));
- builder.addStrategyByRegexp(converter.convert("foo/bar"), ImmutableList.of("ac2"));
- SpawnActionContextMaps maps = builder.build();
-
- List<SpawnStrategy> result =
- maps.getSpawnActionContexts(mockSpawn(null, "Doing something with foo/bar/baz"), reporter);
-
- assertThat(result).containsExactly(ac1);
- }
-
- @Test
- public void regexpAndMnemonic_regexpWins() throws Exception {
- builder.strategyByMnemonicMap().put("Spawn1", "ac1");
- builder.addStrategyByRegexp(converter.convert("foo/bar"), ImmutableList.of("ac2"));
- SpawnActionContextMaps maps = builder.build();
-
- List<SpawnStrategy> result =
- maps.getSpawnActionContexts(
- mockSpawn("Spawn1", "Doing something with foo/bar/baz"), reporter);
-
- assertThat(result).containsExactly(ac2);
- }
-
- @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 mnemonic, String message) {
- Spawn mockSpawn = Mockito.mock(Spawn.class);
- ActionExecutionMetadata mockOwner = Mockito.mock(ActionExecutionMetadata.class);
- when(mockOwner.getProgressMessage()).thenReturn(message);
- when(mockSpawn.getResourceOwner()).thenReturn(mockOwner);
- when(mockSpawn.getMnemonic()).thenReturn(mnemonic);
- return mockSpawn;
- }
-
- private static class AC1 implements SpawnStrategy {
- @Override
- public ImmutableList<SpawnResult> exec(
- Spawn spawn, ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry) {
- return true;
- }
- }
-
- private static class AC2 implements SpawnStrategy {
- @Override
- public ImmutableList<SpawnResult> exec(
- Spawn spawn, ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public boolean canExec(Spawn spawn, ActionContextRegistry actionContextRegistry) {
- return true;
- }
- }
-}
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 c1e0d02..20b2d66 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
@@ -32,11 +32,13 @@
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnContinuation;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.SpawnResult.Status;
import com.google.devtools.build.lib.actions.SpawnStrategy;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.test.TestActionContext;
import com.google.devtools.build.lib.analysis.test.TestProvider;
@@ -51,8 +53,10 @@
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.exec.TestStrategy.TestOutputFormat;
+import com.google.devtools.build.lib.exec.util.TestExecutorBuilder;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.io.FileOutErr;
+import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus;
@@ -90,10 +94,31 @@
}
}
- private class FakeActionExecutionContext extends ActionExecutionContext {
- private final SpawnStrategy spawnActionContext;
+ private static ActionContext.ActionContextRegistry toContextRegistry(
+ SpawnStrategy spawnStrategy,
+ BinTools binTools,
+ FileSystem fileSystem,
+ BlazeDirectories directories) {
+ try {
+ return new TestExecutorBuilder(fileSystem, directories, binTools)
+ .addStrategy(spawnStrategy, "mock")
+ .setDefaultStrategies("mock")
+ .build();
+ } catch (ExecutorInitException e) {
+ throw new AssertionError(e);
+ }
+ }
- public FakeActionExecutionContext(FileOutErr fileOutErr, SpawnStrategy spawnActionContext) {
+ private class FakeActionExecutionContext extends ActionExecutionContext {
+ private final ActionContext.ActionContextRegistry actionContextRegistry;
+
+ public FakeActionExecutionContext(
+ FileOutErr fileOutErr, SpawnStrategy spawnStrategy, BinTools binTools) {
+ this(fileOutErr, toContextRegistry(spawnStrategy, binTools, fileSystem, directories));
+ }
+
+ public FakeActionExecutionContext(
+ FileOutErr fileOutErr, ActionContext.ActionContextRegistry actionContextRegistry) {
super(
/*executor=*/ null,
/*actionInputFileCache=*/ null,
@@ -108,7 +133,7 @@
/*artifactExpander=*/ null,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null);
- this.spawnActionContext = spawnActionContext;
+ this.actionContextRegistry = actionContextRegistry;
}
@Override
@@ -119,7 +144,7 @@
@Override
@Nullable
public <T extends ActionContext> T getContext(Class<T> type) {
- return SpawnStrategy.class.equals(type) ? type.cast(spawnActionContext) : null;
+ return actionContextRegistry.getContext(type);
}
@Override
@@ -134,17 +159,18 @@
@Override
public ActionExecutionContext withFileOutErr(FileOutErr fileOutErr) {
- return new FakeActionExecutionContext(fileOutErr, spawnActionContext);
+ return new FakeActionExecutionContext(fileOutErr, actionContextRegistry);
}
}
- @Mock private SpawnStrategy spawnActionContext;
+ @Mock private SpawnStrategy spawnStrategy;
private StoredEventHandler storedEvents = new StoredEventHandler();
@Before
public final void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
+ when(spawnStrategy.canExec(any(), any())).thenReturn(true);
}
private FileOutErr createTempOutErr(Path tmpDirRoot) {
@@ -234,11 +260,11 @@
.setWallTime(Duration.ofMillis(10))
.setRunnerName("test")
.build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.thenReturn(SpawnContinuation.immediate(expectedSpawnResult));
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -301,14 +327,14 @@
.setWallTime(Duration.ofMillis(15))
.setRunnerName("test")
.build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.thenReturn(
SpawnContinuation.failedWithExecException(
new SpawnExecException("test failed", failSpawnResult, false)))
.thenReturn(SpawnContinuation.immediate(passSpawnResult));
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -371,11 +397,11 @@
.setRunnerName("remote")
.setExecutorHostname("a-remote-host")
.build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.thenReturn(SpawnContinuation.immediate(expectedSpawnResult));
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -430,11 +456,11 @@
.setWallTime(Duration.ofMillis(10))
.setRunnerName("remote cache")
.build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.thenReturn(SpawnContinuation.immediate(expectedSpawnResult));
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -489,7 +515,7 @@
.setExitCode(1)
.setRunnerName("test")
.build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.thenAnswer(
(invocation) -> {
Spawn spawn = invocation.getArgument(0);
@@ -518,7 +544,7 @@
FileOutErr outErr = createTempOutErr(tmpDirRoot);
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(outErr, spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -587,7 +613,7 @@
.setRunnerName("test")
.build();
List<FileOutErr> called = new ArrayList<>();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.thenAnswer(
(invocation) -> {
Spawn spawn = invocation.getArgument(0);
@@ -620,7 +646,7 @@
FileOutErr outErr = createTempOutErr(tmpDirRoot);
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(outErr, spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -671,12 +697,12 @@
SpawnResult expectedSpawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.thenReturn(SpawnContinuation.immediate(expectedSpawnResult));
FileOutErr outErr = createTempOutErr(tmpDirRoot);
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(outErr, spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -727,7 +753,7 @@
SpawnResult expectedSpawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.then(
(invocation) -> {
((ActionExecutionContext) invocation.getArgument(1)).getFileOutErr().printErr("Foo");
@@ -736,7 +762,7 @@
FileOutErr outErr = createTempOutErr(tmpDirRoot);
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(outErr, spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
// actual StandaloneTestStrategy execution
execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
@@ -783,7 +809,7 @@
SpawnResult expectedSpawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
@@ -792,10 +818,10 @@
});
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
List<SpawnResult> resultA = execute(actionA, actionExecutionContext, standaloneTestStrategy);
assertThat(cancelFuture.isCancelled()).isTrue();
- verify(spawnActionContext).beginExecution(any(), any());
+ verify(spawnStrategy).beginExecution(any(), any());
assertThat(resultA).hasSize(1);
assertThat(standaloneTestStrategy.postedResult).isNotNull();
assertThat(standaloneTestStrategy.postedResult.getData().getStatus())
@@ -805,7 +831,7 @@
// Reset postedResult.
standaloneTestStrategy.postedResult = null;
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.thenThrow(new AssertionError("failure: this should not have been called"));
List<SpawnResult> resultB = execute(actionB, actionExecutionContext, standaloneTestStrategy);
assertThat(resultB).isEmpty();
@@ -860,7 +886,7 @@
.setExitCode(1)
.setRunnerName("test")
.build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
@@ -870,10 +896,10 @@
});
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
List<SpawnResult> resultA = execute(actionA, actionExecutionContext, standaloneTestStrategy);
assertThat(cancelFuture.isCancelled()).isFalse();
- verify(spawnActionContext).beginExecution(any(), any());
+ verify(spawnStrategy).beginExecution(any(), any());
assertThat(resultA).hasSize(1);
assertThat(standaloneTestStrategy.postedResult).isNotNull();
assertThat(standaloneTestStrategy.postedResult.getData().getStatus())
@@ -886,7 +912,7 @@
SpawnResult expectedSpawnResultB =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
@@ -950,7 +976,7 @@
.setExitCode(1)
.setRunnerName("test")
.build();
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
@@ -960,10 +986,10 @@
});
ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnActionContext);
+ new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
List<SpawnResult> resultA = execute(actionA, actionExecutionContext, standaloneTestStrategy);
assertThat(cancelFuture.isCancelled()).isFalse();
- verify(spawnActionContext).beginExecution(any(), any());
+ verify(spawnStrategy).beginExecution(any(), any());
assertThat(resultA).hasSize(1);
assertThat(standaloneTestStrategy.postedResult).isNotNull();
assertThat(standaloneTestStrategy.postedResult.getData().getStatus())
@@ -974,7 +1000,7 @@
// Reset postedResult.
standaloneTestStrategy.postedResult = null;
- when(spawnActionContext.beginExecution(any(), any()))
+ when(spawnStrategy.beginExecution(any(), any()))
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
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 5e4b4e5..e1a1d9d 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
@@ -17,6 +17,7 @@
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ExecutorInitException;
+import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext;
import com.google.devtools.build.lib.analysis.actions.LocalTemplateExpansionStrategy;
@@ -28,7 +29,9 @@
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.ModuleActionContextRegistry;
+import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.exec.SymlinkTreeStrategy;
import com.google.devtools.build.lib.runtime.CommonCommandOptions;
import com.google.devtools.build.lib.testutil.TestConstants;
@@ -49,8 +52,10 @@
private Reporter reporter = new Reporter(new EventBus());
private OptionsParser optionsParser =
OptionsParser.builder().optionsClasses(DEFAULT_OPTIONS).build();
- private final SpawnActionContextMaps.Builder spawnMapsBuilder =
- new SpawnActionContextMaps.Builder();
+ private final SpawnStrategyRegistry.Builder strategyRegistryBuilder =
+ new SpawnStrategyRegistry.Builder();
+ private final ModuleActionContextRegistry.Builder actionContextRegistryBuilder =
+ new ModuleActionContextRegistry.Builder();
public TestExecutorBuilder(
FileSystem fileSystem, BlazeDirectories directories, BinTools binTools) {
@@ -60,9 +65,10 @@
public TestExecutorBuilder(FileSystem fileSystem, Path execRoot, BinTools binTools) {
this.fileSystem = fileSystem;
this.execRoot = execRoot;
- addStrategy(FileWriteActionContext.class, new FileWriteStrategy());
- addStrategy(TemplateExpansionContext.class, new LocalTemplateExpansionStrategy());
- addStrategy(SymlinkTreeActionContext.class, new SymlinkTreeStrategy(null, binTools));
+ addContext(FileWriteActionContext.class, new FileWriteStrategy());
+ addContext(TemplateExpansionContext.class, new LocalTemplateExpansionStrategy());
+ addContext(SymlinkTreeActionContext.class, new SymlinkTreeStrategy(null, binTools));
+ addContext(SpawnStrategyResolver.class, new SpawnStrategyResolver());
}
public TestExecutorBuilder setReporter(Reporter reporter) {
@@ -86,26 +92,40 @@
* <p>If two action contexts are registered with the same identifying type and commandline
* identifier the last registered will take precedence.
*/
- public <T extends ActionContext> TestExecutorBuilder addStrategy(
+ public <T extends ActionContext> TestExecutorBuilder addContext(
Class<T> identifyingType, T strategy, String... commandlineIdentifiers) {
- spawnMapsBuilder.strategyByContextMap().put(identifyingType, "");
- spawnMapsBuilder.addContext(identifyingType, strategy, commandlineIdentifiers);
+ actionContextRegistryBuilder.register(identifyingType, strategy, commandlineIdentifiers);
+ return this;
+ }
+
+ /** Makes the given strategy available in the execution phase. */
+ public TestExecutorBuilder addStrategy(
+ SpawnStrategy strategy, String firstCommandlineIdentifier, String... commandlineIdentifiers) {
+ strategyRegistryBuilder.registerStrategy(
+ strategy, firstCommandlineIdentifier, commandlineIdentifiers);
return this;
}
public TestExecutorBuilder setExecution(String mnemonic, String strategy) {
- spawnMapsBuilder.strategyByMnemonicMap().replaceValues(mnemonic, ImmutableList.of(strategy));
+ strategyRegistryBuilder.addMnemonicFilter(mnemonic, ImmutableList.of(strategy));
+ return this;
+ }
+
+ public TestExecutorBuilder setDefaultStrategies(String... strategies) {
+ strategyRegistryBuilder.setDefaultStrategies(ImmutableList.copyOf(strategies));
return this;
}
public BlazeExecutor build() throws ExecutorInitException {
- SpawnActionContextMaps spawnActionContextMaps = spawnMapsBuilder.build();
+ SpawnStrategyRegistry strategyRegistry = strategyRegistryBuilder.build();
+ addContext(SpawnStrategyRegistry.class, strategyRegistry);
return new BlazeExecutor(
fileSystem,
execRoot,
reporter,
BlazeClock.instance(),
optionsParser,
- spawnActionContextMaps);
+ actionContextRegistryBuilder.build(),
+ strategyRegistry);
}
}
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 f655b68..3177645 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
@@ -35,7 +35,6 @@
import com.google.devtools.build.lib.actions.SimpleSpawn;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.SpawnStrategy;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ServerDirectories;
@@ -48,6 +47,7 @@
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.RunfilesTreeUpdater;
import com.google.devtools.build.lib.exec.SingleBuildFileCache;
+import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
import com.google.devtools.build.lib.exec.local.LocalSpawnRunner;
import com.google.devtools.build.lib.exec.util.TestExecutorBuilder;
@@ -152,8 +152,8 @@
Mockito.mock(RunfilesTreeUpdater.class)));
this.executor =
new TestExecutorBuilder(fileSystem, directories, binTools)
- .addStrategy(SpawnStrategy.class, strategy, "standalone")
- .setExecution("", "standalone")
+ .addStrategy(strategy, "standalone")
+ .setDefaultStrategies("standalone")
.build();
executor.getExecRoot().createDirectoryAndParents();
@@ -180,7 +180,7 @@
@Test
public void testBinTrueExecutesFine() throws Exception {
Spawn spawn = createSpawn(getTrueCommand());
- executor.getContext(SpawnStrategy.class).exec(spawn, createContext());
+ executor.getContext(SpawnStrategyResolver.class).exec(spawn, createContext());
if (OS.getCurrent() != OS.WINDOWS) {
assertThat(out()).isEmpty();
@@ -189,7 +189,7 @@
}
private List<SpawnResult> run(Spawn spawn) throws Exception {
- return executor.getContext(SpawnStrategy.class).exec(spawn, createContext());
+ return executor.getContext(SpawnStrategyResolver.class).exec(spawn, createContext());
}
private ActionExecutionContext createContext() {
diff --git a/src/test/py/bazel/action_temp_test.py b/src/test/py/bazel/action_temp_test.py
index 3f859f2..4d94d9b 100644
--- a/src/test/py/bazel/action_temp_test.py
+++ b/src/test/py/bazel/action_temp_test.py
@@ -202,7 +202,7 @@
])
self.AssertExitCode(exit_code, 2, stderr)
pattern = re.compile(
- r'^ERROR:.*is an invalid value for.*Valid values are: (.*)$')
+ r'^ERROR:.*no strategy.*Valid values are: \[(.*)\]$')
for line in stderr:
m = pattern.match(line)
if m:
diff --git a/src/test/shell/integration/execution_strategies_test.sh b/src/test/shell/integration/execution_strategies_test.sh
index 8f6f96c..4b7c88d 100755
--- a/src/test/shell/integration/execution_strategies_test.sh
+++ b/src/test/shell/integration/execution_strategies_test.sh
@@ -62,7 +62,7 @@
function test_multiple_strategies() {
bazel build --spawn_strategy=worker,local --debug_print_action_contexts &> $TEST_log || fail
# Can't test for exact strategy names here, because they differ between platforms and products.
- expect_log "\"\" = \[.*, .*\]"
+ expect_log "DefaultStrategyImplementations: \[.*, .*\]"
}
# Tests that the hardcoded Worker strategies are not introduced with the new
