Disable BwoB if using HTTP cache.

HTTP cache is generally configured to automatically delete old files. It doesn't support lease extension and doesn't understand the relationships between AC and CAS. Hence, they are incompatible. Fixes #18696. RELNOTES: Build without the Bytes is disabled when using HTTP cache. PiperOrigin-RevId: 671301474 Change-Id: I5281a9e37ab791f296d119511db8dc83b8060a16
bazelbuild · Sep 5, 2024 · 3e464fc · 3e464fc
1 parent 56fc099
commit 3e464fc
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 2 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -16,6 +16,7 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.ListeningScheduledExecutorService;
 import com.google.devtools.build.lib.exec.ExecutionOptions;
@@ -224,6 +225,11 @@ public void registerSpawnCache(ModuleActionContextRegistry.Builder registryBuild
     registryBuilder.register(SpawnCache.class, spawnCache, "remote-cache");
   }
 
+  @VisibleForTesting
+  RemoteOutputChecker getRemoteOutputChecker() {
+    return remoteOutputChecker;
+  }
+
   /** Returns the remote cache. */
   RemoteCache getRemoteCache() {
     return remoteCache;

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -261,6 +261,16 @@ public void workspaceInit(
     credentialModule = Preconditions.checkNotNull(runtime.getBlazeModule(CredentialModule.class));
   }
 
+  private static String remoteOutputsModeToFlagName(RemoteOutputsMode remoteOutputsMode) {
+    String flagName = "";
+    switch (remoteOutputsMode) {
+      case MINIMAL -> flagName = "--remote_download_minimal";
+      case TOPLEVEL -> flagName = "--remote_download_toplevel";
+      case ALL -> flagName = "--remote_download_all";
+    }
+    return flagName;
+  }
+
   @Override
   public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
     Preconditions.checkState(actionContextProvider == null, "actionContextProvider must be null");
@@ -356,10 +366,26 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
                   .build()));
     }
 
+    var remoteOutputsMode = remoteOptions.remoteOutputsMode;
+    if (enableHttpCache && remoteOutputsMode != RemoteOutputsMode.ALL) {
+      // HTTP cache is generally configured to automatically delete old files. It doesn't support
+      // lease extension and doesn't understand the relationships between AC and CAS. Hence, it's
+      // incompatible with other RemoteOutputsMode.
+      //
+      // See https://github.com/bazelbuild/bazel/issues/18696.
+      env.getReporter()
+          .handle(
+              Event.warn(
+                  String.format(
+                      "%s is incompatible with HTTP cache. Using --remote_download_all.",
+                      remoteOutputsModeToFlagName(remoteOutputsMode))));
+      remoteOutputsMode = RemoteOutputsMode.ALL;
+    }
+
     // TODO(bazel-team): Consider adding a warning or more validation if the remoteDownloadRegex is
     // used without Build without the Bytes.
     ImmutableList.Builder<Pattern> patternsToDownloadBuilder = ImmutableList.builder();
-    if (remoteOptions.remoteOutputsMode != RemoteOutputsMode.ALL) {
+    if (remoteOutputsMode != RemoteOutputsMode.ALL) {
       for (RegexPatternOption patternOption : remoteOptions.remoteDownloadRegex) {
         patternsToDownloadBuilder.add(patternOption.regexPattern());
       }
@@ -369,7 +395,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
         new RemoteOutputChecker(
             new JavaClock(),
             env.getCommandName(),
-            remoteOptions.remoteOutputsMode,
+            remoteOutputsMode,
             patternsToDownloadBuilder.build(),
             lastRemoteOutputChecker);
     remoteOutputChecker.maybeInvalidateSkyframeValues(env.getSkyframeExecutor().getEvaluator());

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java
@@ -18,6 +18,7 @@
 import static com.google.devtools.build.lib.packages.TargetUtils.isTestRuleName;
 import static com.google.devtools.build.lib.skyframe.CoverageReportValue.COVERAGE_REPORT_KEY;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionInput;
@@ -89,6 +90,11 @@ public RemoteOutputChecker(
     this.lastRemoteOutputChecker = lastRemoteOutputChecker;
   }
 
+  @VisibleForTesting
+  RemoteOutputsMode getRemoteOutputsMode() {
+    return outputsMode;
+  }
+
   // Skymeld-only.
   public void afterTopLevelTargetAnalysis(
       ConfiguredTarget configuredTarget,

diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java
@@ -46,6 +46,7 @@
 import com.google.devtools.build.lib.remote.circuitbreaker.FailureCircuitBreaker;
 import com.google.devtools.build.lib.remote.downloader.GrpcRemoteDownloader;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
+import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.BlazeServerStartupOptions;
 import com.google.devtools.build.lib.runtime.BlazeWorkspace;
@@ -540,6 +541,30 @@ public void bazelOutputService_noRemoteCache_exit() throws Exception {
     }
   }
 
+  @Test
+  public void httpCacheWithOutputMinimal_overrideToAll() throws Exception {
+    remoteOptions.remoteCache = "http://nonexistent.com";
+    remoteOptions.remoteOutputsMode = RemoteOutputsMode.MINIMAL;
+
+    beforeCommand();
+
+    assertThat(
+            remoteModule.getActionContextProvider().getRemoteOutputChecker().getRemoteOutputsMode())
+        .isEqualTo(RemoteOutputsMode.ALL);
+  }
+
+  @Test
+  public void httpCacheWithOutputToplevel_overrideToAll() throws Exception {
+    remoteOptions.remoteCache = "http://nonexistent.com";
+    remoteOptions.remoteOutputsMode = RemoteOutputsMode.TOPLEVEL;
+
+    beforeCommand();
+
+    assertThat(
+            remoteModule.getActionContextProvider().getRemoteOutputChecker().getRemoteOutputsMode())
+        .isEqualTo(RemoteOutputsMode.ALL);
+  }
+
   private void beforeCommand() throws IOException, AbruptExitException {
     CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions);
     remoteModule.beforeCommand(env);