commit 0a2aac4e3c6f5ef98fc233e2ce83e44930edb188
author Googler <jhorvitz@google.com> Tue Oct 17 08:25:23 2023 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Oct 17 08:28:57 2023 -0700
tree dfd8250bb147bbe0cfa3eb8eb685c5622b8ed760
parent 05bea52ed3159aa5d15d967f5f56fc084a2b6c73

Make action rewinding compatible with `--use_action_cache`.

When preparing an action for rewinding, evict it from the action cache to ensure that it is executed.

PiperOrigin-RevId: 574158458
Change-Id: I82075f9a1a79cdd601ac6e0d2b3c17969dae81ca

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

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
index 56f5ed0..adc8992 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -149,7 +149,8 @@
     return ActionCacheUtils.getCacheEntry(actionCache, action);
   }
 
-  private void removeCacheEntry(Action action) {
+  public void removeCacheEntry(Action action) {
+    checkState(enabled(), "Action cache disabled");
     ActionCacheUtils.removeCacheEntry(actionCache, action);
   }
 

src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 1efb78c..a9c9215 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -317,16 +317,6 @@
     if (buildRequestOptions == null || !buildRequestOptions.rewindLostInputs) {
       return false;
     }
-    if (buildRequestOptions.useActionCache) {
-      throw new AbruptExitException(
-          DetailedExitCode.of(
-              FailureDetail.newBuilder()
-                  .setMessage("--rewind_lost_inputs requires --nouse_action_cache")
-                  .setActionRewinding(
-                      ActionRewinding.newBuilder()
-                          .setCode(ActionRewinding.Code.REWIND_LOST_INPUTS_PREREQ_UNMET))
-                  .build()));
-    }
     if (isMergedSkyframeAnalysisExecution()) {
       throw new AbruptExitException(
           DetailedExitCode.of(

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index 4a83096..b75b2ce 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -464,6 +464,10 @@
     if (!actionFileSystemType().inMemoryFileSystem()) {
       outputDirectoryHelper.invalidateTreeArtifactDirectoryCreation(dep.getOutputs());
     }
+    // Evict the rewinding action from the action cache to ensure that it is executed.
+    if (actionCacheChecker.enabled()) {
+      actionCacheChecker.removeCacheEntry(dep);
+    }
   }
 
   @Nullable

src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
index 75a4613..bfde8fe 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -2664,14 +2664,11 @@
 
   @Test
   public void rewindingPrerequisites(
-      @TestParameter boolean trackIncrementalState,
-      @TestParameter boolean useActionCache,
-      @TestParameter boolean skymeldEnabled)
+      @TestParameter boolean trackIncrementalState, @TestParameter boolean skymeldEnabled)
       throws Exception {
     initializeSkyframeExecutor();
     options.parse(
         "--rewind_lost_inputs",
-        "--use_action_cache=" + useActionCache,
         "--experimental_merged_skyframe_analysis_execution=" + skymeldEnabled);
     skyframeExecutor.setMergedSkyframeAnalysisExecutionSupplier(() -> skymeldEnabled);
 
@@ -2685,7 +2682,7 @@
         reporter);
     skyframeExecutor.setActive(true);
 
-    if (useActionCache || skymeldEnabled) {
+    if (skymeldEnabled) {
       AbruptExitException e = assertThrows(AbruptExitException.class, this::syncSkyframeExecutor);
       assertThat(e.getDetailedExitCode().getFailureDetail().getActionRewinding().getCode())
           .isEqualTo(ActionRewinding.Code.REWIND_LOST_INPUTS_PREREQ_UNMET);

src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java
index 522b8e8..44183d3 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java
@@ -71,7 +71,6 @@
     super.setupOptions();
     addOptions(
         "--spawn_strategy=standalone",
-        "--nouse_action_cache",
         "--noexperimental_merged_skyframe_analysis_execution",
         "--rewind_lost_inputs",
         "--features=cc_include_scanning",