Do not invalidate skyframe's in-memory action cache when changing out…

…put mode.

Previously, we invalidate skyframe's cache if the output mode is changed from BwoB to ALL because they were two different code paths and are incompatible with each other.

Now the code paths are unifded and changing the output mode only affects the decision for whether Bazel should download outputs, no more other things.

PiperOrigin-RevId: 545641839
Change-Id: I83288f7cb505ad0e47368c702797ca365bdb4191

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

@@ -1722,11 +1722,8 @@ public void deleteActionsIfRemoteOptionsChanged(OptionsProvider options)
           lastRemoteCacheEnabled != null && lastRemoteCacheEnabled && !remoteCacheEnabled;
     }
     lastRemoteCacheEnabled = remoteCacheEnabled;
-
-    RemoteOutputsMode remoteOutputsMode =
+    lastRemoteOutputsMode =
         remoteOptions != null ? remoteOptions.remoteOutputsMode : RemoteOutputsMode.ALL;
-    needsDeletion |= lastRemoteOutputsMode != null && lastRemoteOutputsMode != remoteOutputsMode;
-    this.lastRemoteOutputsMode = remoteOutputsMode;
 
     if (needsDeletion) {
       memoizingEvaluator.delete(k -> SkyFunctions.ACTION_EXECUTION.equals(k.functionName()));

src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java

@@ -194,7 +194,7 @@ public void intermediateOutputsAreInputForInternalActions_prefetchIntermediateOu
   }
 
   @Test
-  public void changeOutputMode_invalidateActions() throws Exception {
+  public void changeOutputMode_notInvalidateActions() throws Exception {
     write(
         "a/BUILD",
         "genrule(",
@@ -210,21 +210,26 @@ public void changeOutputMode_invalidateActions() throws Exception {
         "  outs = ['foobar.txt'],",
         "  cmd = 'cat $(location :foo) > $@ && echo bar > $@',",
         ")");
+    // Download all outputs with regex so in the next build with ALL mode, the actions are not
+    // invalidated because of missing outputs.
+    addOptions("--experimental_remote_download_regex=.*");
     ActionEventCollector actionEventCollector = new ActionEventCollector();
     runtimeWrapper.registerSubscriber(actionEventCollector);
     buildTarget("//a:foobar");
+    // Add the new option here because waitDownloads below will internally create a new command
+    // which will parse the new option.
+    setDownloadAll();
+    waitDownloads();
     // 3 = workspace status action + //:foo + //:foobar
     assertThat(actionEventCollector.getNumActionNodesEvaluated()).isEqualTo(3);
     actionEventCollector.clear();
     events.clear();
 
-    setDownloadAll();
     buildTarget("//a:foobar");
 
-    // Changing output mode should invalidate SkyFrame's in-memory caching and make it re-evaluate
-    // the action nodes.
-    assertThat(actionEventCollector.getNumActionNodesEvaluated()).isEqualTo(3);
-    events.assertContainsInfo("2 processes: 2 remote cache hit");
+    // Changing output mode should not invalidate SkyFrame's in-memory caching.
+    assertThat(actionEventCollector.getNumActionNodesEvaluated()).isEqualTo(0);
+    events.assertContainsInfo("0 processes");
   }
 
   @Test