Simplify Executor.getSpawnActionContext() by removing mnemonic: spawn is enough.
RELNOTES: None
PiperOrigin-RevId: 190544948
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
index a0ea5698..a75ee38 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
@@ -183,9 +183,9 @@
return executor.getContext(type);
}
- /** Returns the action context implementation for spawn actions with a given mnemonic. */
- public SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn) {
- return executor.getSpawnActionContext(mnemonic, spawn);
+ /** Returns the action context implementation for a given spawn action. */
+ public SpawnActionContext getSpawnActionContext(Spawn spawn) {
+ return executor.getSpawnActionContext(spawn);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Executor.java b/src/main/java/com/google/devtools/build/lib/actions/Executor.java
index bb1f6b4..3dc0e5d 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Executor.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Executor.java
@@ -87,6 +87,6 @@
*/
<T extends ActionContext> T getContext(Class<? extends T> type);
- /** Returns the action context implementation for spawn actions with a given mnemonic. */
- SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn);
+ /** Returns the action context implementation for the given spawn. */
+ SpawnActionContext getSpawnActionContext(Spawn spawn);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java
index ec46996..6567c93 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java
@@ -235,7 +235,7 @@
private SpawnActionContext getContext(
Spawn spawn, ActionExecutionContext actionExecutionContext) {
- return actionExecutionContext.getSpawnActionContext(getMnemonic(), spawn);
+ return actionExecutionContext.getSpawnActionContext(spawn);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index fc068bc..3d352c4 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
@@ -451,7 +451,7 @@
protected SpawnActionContext getContext(
ActionExecutionContext actionExecutionContext, Spawn spawn) {
- return actionExecutionContext.getSpawnActionContext(getMnemonic(), spawn);
+ return actionExecutionContext.getSpawnActionContext(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 fe05762..7d41a15 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
@@ -190,8 +190,8 @@
* set, then it returns the default strategy for spawn actions.
*/
@Override
- public SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn) {
- return spawnActionContextMaps.getSpawnActionContext(mnemonic, spawn, reporter);
+ public SpawnActionContext getSpawnActionContext(Spawn spawn) {
+ return spawnActionContextMaps.getSpawnActionContext(spawn, reporter);
}
/** Returns true iff the --verbose_failures option was enabled. */
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
index fd017d0f..fc179ba 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
@@ -16,6 +16,7 @@
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.ImmutableList;
@@ -81,14 +82,14 @@
}
/**
- * Returns the appropriate {@link ActionContext} to execute an action with the given {@code
- * mnemonic} and {@link Spawn}.
+ * Returns the appropriate {@link ActionContext} to execute the given {@link Spawn} with.
*
* <p>If the reason for selecting the context is worth mentioning to the user, logs a message
* using the given {@link Reporter}.
*/
- public SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn, Reporter reporter) {
- if (!spawnStrategyRegexList.isEmpty() && spawn != null && spawn.getResourceOwner() != null) {
+ public SpawnActionContext getSpawnActionContext(Spawn spawn, Reporter reporter) {
+ Preconditions.checkNotNull(spawn);
+ if (!spawnStrategyRegexList.isEmpty() && spawn.getResourceOwner() != null) {
String description = spawn.getResourceOwner().getProgressMessage();
if (description != null) {
for (RegexFilterSpawnActionContext entry : spawnStrategyRegexList) {
@@ -100,7 +101,7 @@
}
}
}
- SpawnActionContext context = spawnStrategyMnemonicMap.get(mnemonic);
+ SpawnActionContext context = spawnStrategyMnemonicMap.get(spawn.getMnemonic());
if (context != null) {
return context;
}
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 91ff96f..c3828d3 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
@@ -347,8 +347,7 @@
TestResultData.Builder builder = TestResultData.newBuilder();
long startTime = actionExecutionContext.getClock().currentTimeMillis();
- SpawnActionContext spawnActionContext =
- actionExecutionContext.getSpawnActionContext(action.getMnemonic(), spawn);
+ SpawnActionContext spawnActionContext = actionExecutionContext.getSpawnActionContext(spawn);
List<SpawnResult> spawnResults = ImmutableList.of();
try {
try {
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
index 612b926..5f231e4 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
@@ -124,7 +124,7 @@
shellEnvironment,
inputManifestArtifact);
return actionExecutionContext
- .getSpawnActionContext(owner.getMnemonic(), spawn)
+ .getSpawnActionContext(spawn)
.exec(spawn, actionExecutionContext);
} else {
// Pretend we created the runfiles tree by copying the manifest
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 2b214ee..8bd167f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -317,7 +317,7 @@
estimateResourceConsumptionLocal());
return ActionResult.create(
actionExecutionContext
- .getSpawnActionContext(getMnemonic(), spawn)
+ .getSpawnActionContext(spawn)
.exec(spawn, actionExecutionContext));
} catch (ExecException e) {
throw e.toActionExecutionException(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
index 3f40a10..5d29cb7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
@@ -62,9 +62,7 @@
action.estimateResourceConsumptionLocal());
List<SpawnResult> spawnResults =
- actionExecutionContext
- .getSpawnActionContext(action.getMnemonic(), spawn)
- .exec(spawn, actionExecutionContext);
+ actionExecutionContext.getSpawnActionContext(spawn).exec(spawn, actionExecutionContext);
return CppCompileActionResult.builder().setSpawnResults(spawnResults).build();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java b/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
index d9d78cd..b4f8786 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
@@ -84,7 +84,7 @@
}
@Override
- public SpawnActionContext getSpawnActionContext(String mnemonic, Spawn spawn) {
+ public SpawnActionContext getSpawnActionContext(Spawn spawn) {
throw new UnsupportedOperationException();
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
index 8e9f610..38ee97c 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
@@ -71,7 +71,7 @@
public void duplicateMnemonics_lastOneWins() throws Exception {
builder.strategyByMnemonicMap().put("Spawn1", "ac1").put("Spawn1", "ac2");
SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest");
- SpawnActionContext result = maps.getSpawnActionContext("Spawn1", null, reporter);
+ SpawnActionContext result = maps.getSpawnActionContext(mockSpawn("Spawn1", null), reporter);
assertThat(result).isInstanceOf(AC2.class);
}
@@ -82,7 +82,7 @@
SpawnActionContextMaps maps = builder.build(PROVIDERS, "actest");
SpawnActionContext result =
- maps.getSpawnActionContext("", mockSpawn("Doing something with foo/bar/baz"), reporter);
+ maps.getSpawnActionContext(mockSpawn(null, "Doing something with foo/bar/baz"), reporter);
assertThat(result).isInstanceOf(AC1.class);
}
@@ -95,7 +95,7 @@
SpawnActionContext result =
maps.getSpawnActionContext(
- "Spawn1", mockSpawn("Doing something with foo/bar/baz"), reporter);
+ mockSpawn("Spawn1", "Doing something with foo/bar/baz"), reporter);
assertThat(result).isInstanceOf(AC2.class);
}
@@ -107,11 +107,12 @@
builder.strategyByContextMap().put(AC1.class, "");
}
- private Spawn mockSpawn(String message) {
+ 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;
}
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 9ebe4aa..57ad243 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
@@ -128,7 +128,7 @@
.build();
when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult));
- when(actionExecutionContext.getSpawnActionContext(any(), any())).thenReturn(spawnActionContext);
+ when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -208,7 +208,7 @@
expectedSpawnResult,
/*forciblyRunRemotely=*/ false,
/*catastrophe=*/ false));
- when(actionExecutionContext.getSpawnActionContext(any(), any())).thenReturn(spawnActionContext);
+ when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
@@ -285,7 +285,7 @@
SpawnResult expectedSpawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).build();
when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult));
- when(actionExecutionContext.getSpawnActionContext(any(), any())).thenReturn(spawnActionContext);
+ when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext);
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
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 0e93ff2..6a46b88 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
@@ -171,14 +171,14 @@
@Test
public void testBinTrueExecutesFine() throws Exception {
Spawn spawn = createSpawn(getTrueCommand());
- executor.getSpawnActionContext(spawn.getMnemonic(), spawn).exec(spawn, createContext());
+ executor.getSpawnActionContext(spawn).exec(spawn, createContext());
assertThat(out()).isEmpty();
assertThat(err()).isEmpty();
}
private List<SpawnResult> run(Spawn spawn) throws Exception {
- return executor.getSpawnActionContext(spawn.getMnemonic(), spawn).exec(spawn, createContext());
+ return executor.getSpawnActionContext(spawn).exec(spawn, createContext());
}
private ActionExecutionContext createContext() {
