Dynamic execution needs to check whether the selected strategy can actually execute a spawn.
WorkerSpawnRunner should check earlier if a spawn can't be executed because it lacks tools.
PiperOrigin-RevId: 319024147
diff --git a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicExecutionOptions.java
index 100def6..1bcc729 100644
--- a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicExecutionOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicExecutionOptions.java
@@ -76,7 +76,7 @@
allowMultiple = true,
help =
"The local strategies, in order, to use for the given mnemonic. Passing"
- + " 'remote' as the mnemonic sets the default for unspecified mnemonics. Takes"
+ + " 'local' as the mnemonic sets the default for unspecified mnemonics. Takes"
+ " [mnemonic=]local_strategy[,local_strategy,...]")
public List<Map.Entry<String, List<String>>> dynamicLocalStrategy;
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 16e40c4..9d53582 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
@@ -374,7 +374,9 @@
for (SandboxedSpawnStrategy strategy :
dynamicStrategyRegistry.getDynamicSpawnActionContexts(
spawn, DynamicStrategyRegistry.DynamicMode.LOCAL)) {
- return strategy.exec(spawn, actionExecutionContext, stopConcurrentSpawns);
+ if (strategy.canExec(spawn, actionExecutionContext)) {
+ return strategy.exec(spawn, actionExecutionContext, stopConcurrentSpawns);
+ }
}
throw new RuntimeException(
"executorCreated not yet called or no default dynamic_local_strategy set");
@@ -391,7 +393,9 @@
for (SandboxedSpawnStrategy strategy :
dynamicStrategyRegistry.getDynamicSpawnActionContexts(
spawn, DynamicStrategyRegistry.DynamicMode.REMOTE)) {
- return strategy.exec(spawn, actionExecutionContext, stopConcurrentSpawns);
+ if (strategy.canExec(spawn, actionExecutionContext)) {
+ return strategy.exec(spawn, actionExecutionContext, stopConcurrentSpawns);
+ }
}
throw new RuntimeException(
"executorCreated not yet called or no default dynamic_remote_strategy set");
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 f22c120..36cd6eb 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
@@ -32,7 +32,6 @@
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.Spawns;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.exec.ExecutionPolicy;
@@ -359,11 +358,6 @@
outDir.getChild(outBaseName + suffix), errDir.getChild(errBaseName + suffix));
}
- private static boolean supportsWorkers(Spawn spawn) {
- return (!DISABLED_MNEMONICS_FOR_WORKERS.contains(spawn.getMnemonic())
- && Spawns.supportsWorkers(spawn));
- }
-
private static SandboxedSpawnStrategy.StopConcurrentSpawns lockOutputFiles(
SandboxedSpawnStrategy token, @Nullable AtomicReference<SpawnStrategy> outputWriteBarrier) {
if (outputWriteBarrier == null) {
@@ -389,7 +383,12 @@
for (SandboxedSpawnStrategy strategy :
dynamicStrategyRegistry.getDynamicSpawnActionContexts(
spawn, DynamicStrategyRegistry.DynamicMode.LOCAL)) {
- if (!strategy.toString().contains("worker") || supportsWorkers(spawn)) {
+
+ if (strategy.toString().contains("worker")
+ && DISABLED_MNEMONICS_FOR_WORKERS.contains(spawn.getMnemonic())) {
+ continue;
+ }
+ if (strategy.canExec(spawn, actionExecutionContext)) {
return strategy.exec(
spawn, actionExecutionContext, lockOutputFiles(strategy, outputWriteBarrier));
}
@@ -409,8 +408,10 @@
for (SandboxedSpawnStrategy strategy :
dynamicStrategyRegistry.getDynamicSpawnActionContexts(
spawn, DynamicStrategyRegistry.DynamicMode.REMOTE)) {
- return strategy.exec(
- spawn, actionExecutionContext, lockOutputFiles(strategy, outputWriteBarrier));
+ if (strategy.canExec(spawn, actionExecutionContext)) {
+ return strategy.exec(
+ spawn, actionExecutionContext, lockOutputFiles(strategy, outputWriteBarrier));
+ }
}
throw new RuntimeException(
"executorCreated not yet called or no default dynamic_remote_strategy set");
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
index 2f76701..8cc3444 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
@@ -143,7 +143,13 @@
@Override
public boolean canExec(Spawn spawn) {
- return Spawns.supportsWorkers(spawn) || Spawns.supportsMultiplexWorkers(spawn);
+ if (!Spawns.supportsWorkers(spawn) && !Spawns.supportsMultiplexWorkers(spawn)) {
+ return false;
+ }
+ if (spawn.getToolFiles().isEmpty()) {
+ return false;
+ }
+ return true;
}
@Override
