Allow the post processing spawns to bypass the execution check.

The post processing spawn should likely not be governed by the strategy flags, considering that it is not language specific.

This will eventually go away after some internal projects are landed, but there's no guaranteed timeline yet.

PiperOrigin-RevId: 839585528
Change-Id: I3b7eccbb259c75417bc46f771b0374b192bdba47

Author: bigelephant29

src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java

@@ -308,4 +308,11 @@ public enum WorkerProtocolFormat {
    * segment from the paths of all inputs and outputs.
    */
   public static final String SUPPORTS_PATH_MAPPING = "supports-path-mapping";
+
+  /**
+   * Google internal use only. If specified, this spawn is a post-processing step for archived tree
+   * artifacts and should bypass dynamic strategy checks. See b/462023192.
+   */
+  public static final String IS_ARCHIVED_TREE_POST_PROCESSING =
+      "internal-is-archived-tree-post-processing";
 }

src/main/java/com/google/devtools/build/lib/dynamic/BUILD

@@ -19,6 +19,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+        "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
         "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/concurrent:thread_safety",

src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java

@@ -30,6 +30,7 @@
 import com.google.devtools.build.lib.actions.DynamicStrategyRegistry;
 import com.google.devtools.build.lib.actions.DynamicStrategyRegistry.DynamicMode;
 import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.SandboxedSpawnStrategy;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnResult;
@@ -154,6 +155,12 @@ private static boolean canExecLocal(
       ExecutionPolicy executionPolicy,
       ActionContext.ActionContextRegistry acr,
       DynamicStrategyRegistry dsr) {
+    if (spawn.getExecutionInfo() != null
+        && spawn
+            .getExecutionInfo()
+            .containsKey(ExecutionRequirements.IS_ARCHIVED_TREE_POST_PROCESSING)) {
+      return executionPolicy.canRunLocally();
+    }
     return getLocalStrategy(spawn, executionPolicy, acr, dsr) != null;
   }
 

src/test/java/com/google/devtools/build/lib/dynamic/BUILD

@@ -67,6 +67,7 @@ java_test(
     deps = [
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+        "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
         "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
         "//src/main/java/com/google/devtools/build/lib/bugreport",
         "//src/main/java/com/google/devtools/build/lib/dynamic",

src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java

@@ -35,6 +35,7 @@
 import com.google.devtools.build.lib.actions.BaseSpawn;
 import com.google.devtools.build.lib.actions.DynamicStrategyRegistry;
 import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.Executor;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.SandboxedSpawnStrategy;
@@ -739,6 +740,35 @@ public void actionFailsIfLocalAndRemoteRunNothing() throws Exception {
     assertThat(remoteStrategy.succeeded()).isFalse();
   }
 
+  @Test
+  public void archivedTreePostProcessingRequirementAllowsLocalExecution() throws Exception {
+    MockSpawnStrategy localStrategy =
+        new MockSpawnStrategy("MockLocalSpawnStrategy", DoExec.NOTHING, DoExec.NOTHING, false);
+    MockSpawnStrategy remoteStrategy =
+        new MockSpawnStrategy("MockRemoteSpawnStrategy", DoExec.NOTHING, DoExec.NOTHING, false);
+    StrategyAndContext strategyAndContext = createSpawnStrategy(localStrategy, remoteStrategy);
+
+    // Without the requirement, spawn cannot execute because neither local nor remote can exec.
+    ImmutableMap<String, String> baseRequirements =
+        ImmutableMap.of(ExecutionRequirements.NO_REMOTE, "1");
+    Spawn spawn = newCustomSpawn("Null", baseRequirements);
+    assertThat(strategyAndContext.strategy().canExec(spawn, strategyAndContext.context()))
+        .isFalse();
+
+    // With the requirement, spawn *can* execute because the check is bypassed for local.
+    ImmutableMap<String, String> postProcessingRequirements =
+        ImmutableMap.<String, String>builder()
+            .putAll(baseRequirements)
+            .put(ExecutionRequirements.IS_ARCHIVED_TREE_POST_PROCESSING, "1")
+            .buildOrThrow();
+    Spawn postProcessingSpawn = newCustomSpawn("Null", postProcessingRequirements);
+    assertThat(
+            strategyAndContext
+                .strategy()
+                .canExec(postProcessingSpawn, strategyAndContext.context()))
+        .isTrue();
+  }
+
   @Test
   public void stopConcurrentSpawnsWaitForCompletion() throws Exception {
     CountDownLatch countDownLatch = new CountDownLatch(2);

src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyUnitTest.java

@@ -212,83 +212,6 @@ public void exec_localOnlySpawn_runsLocalPostProcessingSpawn() throws Exception
         .inOrder();
   }
 
-  @Test
-  public void exec_localOnlySpawn_noneCanExec_fails() throws Exception {
-    Spawn spawn =
-        new SpawnBuilder().withMnemonic("ThisMnemonic1").withOwnerPrimaryOutput(output1).build();
-    Spawn postProcessingSpawn =
-        new SpawnBuilder().withMnemonic("ThatMnemonic2").withOwnerPrimaryOutput(output2).build();
-
-    DynamicSpawnStrategy dynamicSpawnStrategy =
-        createDynamicSpawnStrategy(
-            ExecutionPolicy.LOCAL_EXECUTION_ONLY, mockGetPostProcessingSpawn);
-    when(mockGetPostProcessingSpawn.apply(spawn)).thenReturn(Optional.of(postProcessingSpawn));
-    SandboxedSpawnStrategy local = createMockSpawnStrategy(false);
-    SandboxedSpawnStrategy remote = createMockSpawnStrategy();
-    ActionExecutionContext actionExecutionContext = createMockActionExecutionContext(local, remote);
-
-    UserExecException thrown =
-        assertThrows(
-            UserExecException.class,
-            () -> dynamicSpawnStrategy.exec(spawn, actionExecutionContext));
-    assertThat(thrown)
-        .hasMessageThat()
-        .isEqualTo(
-            "Spawn is not executable in local: No usable dynamic_local_strategy found (and remote"
-                + " execution disabled) for action ThisMnemonic1. Post-Processing Spawn is not"
-                + " executable in local: No usable dynamic_local_strategy found (and remote"
-                + " execution disabled) for action ThatMnemonic2. ");
-    assertThat(thrown).hasMessageThat().doesNotContain("dynamic_remote_strategy");
-    verifyNoInteractions(remote);
-  }
-
-  @Test
-  public void exec_localOnlySpawnWithNonExecutablePostProcessingSpawn_doesNotExecLocalSpawn()
-      throws Exception {
-    Spawn spawn =
-        new SpawnBuilder().withMnemonic("ThisMnemonic1").withOwnerPrimaryOutput(output1).build();
-    Spawn postProcessingSpawn =
-        new SpawnBuilder().withMnemonic("ThatMnemonic2").withOwnerPrimaryOutput(output2).build();
-
-    DynamicSpawnStrategy dynamicSpawnStrategy =
-        createDynamicSpawnStrategy(
-            ExecutionPolicy.LOCAL_EXECUTION_ONLY, mockGetPostProcessingSpawn);
-    when(mockGetPostProcessingSpawn.apply(spawn)).thenReturn(Optional.of(postProcessingSpawn));
-
-    SandboxedSpawnStrategy local = createMockSpawnStrategy();
-
-    ActionExecutionContext actionExecutionContext = mock(ActionExecutionContext.class);
-    when(actionExecutionContext.getFileOutErr()).thenReturn(new TestFileOutErr());
-    when(actionExecutionContext.getContext(DynamicStrategyRegistry.class))
-        .thenReturn(
-            new DynamicStrategyRegistry() {
-              @Override
-              public ImmutableList<SandboxedSpawnStrategy> getDynamicSpawnActionContexts(
-                  Spawn spawn, DynamicMode dynamicMode) {
-                if (spawn.getMnemonic().equals("ThisMnemonic1")) {
-                  return ImmutableList.of(local);
-                }
-                return ImmutableList.of();
-              }
-
-              @Override
-              public void notifyUsedDynamic(ActionContextRegistry actionContextRegistry) {}
-            });
-    when(actionExecutionContext.withFileOutErr(any())).thenReturn(actionExecutionContext);
-
-    UserExecException thrown =
-        assertThrows(
-            UserExecException.class,
-            () -> dynamicSpawnStrategy.exec(spawn, actionExecutionContext));
-
-    assertThat(thrown)
-        .hasMessageThat()
-        .isEqualTo(
-            "Post-Processing Spawn is not executable in local: No usable dynamic_local_strategy"
-                + " found (and remote execution disabled) for action ThatMnemonic2. ");
-    assertThat(thrown).hasMessageThat().doesNotContain("dynamic_remote_strategy");
-  }
-
   @Test
   public void exec_failedLocalSpawn_doesNotExecLocalPostProcessingSpawn() throws Exception {
     testExecFailedLocalSpawnDoesNotExecLocalPostProcessingSpawn(
@@ -333,7 +256,6 @@ private void testExecFailedLocalSpawnDoesNotExecLocalPostProcessingSpawn(SpawnRe
     assertThat(results).containsExactly(failedResult);
     assertThat(localSpawnCaptor.getAllValues()).containsExactly(spawn);
     verify(remote, never()).exec(any(), any(), any());
-    verifyNoInteractions(postProcessingSpawn);
   }
 
   @Test
@@ -638,7 +560,6 @@ public void exec_runAnywhereSpawn_localCantExec_runsRemote() throws Exception {
     assertThat(results).containsExactly(SUCCESSFUL_SPAWN_RESULT);
     // Never runs anything as it says it can't execute anything at all.
     verify(local, never()).exec(any(), any(), any());
-    verifyNoInteractions(postProcessingSpawn);
   }
 
   @Test
@@ -672,33 +593,6 @@ public void exec_runAnywhereSpawn_remoteCantExec_runsLocal() throws Exception {
     verify(remote, never()).exec(any(), any(), any());
   }
 
-  @Test
-  public void exec_runAnywhereSpawn_noneCanExec_fails() throws Exception {
-    Spawn spawn =
-        new SpawnBuilder().withMnemonic("ThisMnemonic1").withOwnerPrimaryOutput(output1).build();
-    Spawn postProcessingSpawn =
-        new SpawnBuilder().withMnemonic("ThatMnemonic2").withOwnerPrimaryOutput(output2).build();
-
-    DynamicSpawnStrategy dynamicSpawnStrategy =
-        createDynamicSpawnStrategy(ExecutionPolicy.ANYWHERE, mockGetPostProcessingSpawn);
-    when(mockGetPostProcessingSpawn.apply(spawn)).thenReturn(Optional.of(postProcessingSpawn));
-    SandboxedSpawnStrategy local = createMockSpawnStrategy(false);
-    SandboxedSpawnStrategy remote = createMockSpawnStrategy(false);
-    ActionExecutionContext actionExecutionContext = createMockActionExecutionContext(local, remote);
-
-    UserExecException thrown =
-        assertThrows(
-            UserExecException.class,
-            () -> dynamicSpawnStrategy.exec(spawn, actionExecutionContext));
-    assertThat(thrown)
-        .hasMessageThat()
-        .isEqualTo(
-            "Spawn is not executable in local: No usable dynamic_local_strategy or"
-                + " dynamic_remote_strategy found for action ThisMnemonic1. Post-Processing Spawn"
-                + " is not executable in local: No usable dynamic_local_strategy or"
-                + " dynamic_remote_strategy found for action ThatMnemonic2. ");
-  }
-
   private DynamicSpawnStrategy createDynamicSpawnStrategy(
       ExecutionPolicy executionPolicy,
       Function<Spawn, Optional<Spawn>> getPostProcessingSpawnForLocalExecution) {