[7.1.0] Let scrubbed actions fall back to local execution when remote…

… execution is enabled. (#21384)

As discussed in #20070, it's
sometimes useful to scrub actions in a build that is otherwise remote.

Closes #21288.

Commit
8e0343c

PiperOrigin-RevId: 607723207
Change-Id: I31403de74fb81d07aac765cca68b2991b0230496

Co-authored-by: Artem V. Navrotskiy <[email protected]>

src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java

@@ -341,7 +341,8 @@ public CachePolicy getWriteCachePolicy(Spawn spawn) {
   public boolean mayBeExecutedRemotely(Spawn spawn) {
     return remoteCache instanceof RemoteExecutionCache
         && remoteExecutor != null
-        && Spawns.mayBeExecutedRemotely(spawn);
+        && Spawns.mayBeExecutedRemotely(spawn)
+        && !isScrubbedSpawn(spawn, scrubber);
   }
 
   @VisibleForTesting
@@ -1596,6 +1597,10 @@ void report(Event evt) {
     }
   }
 
+  private static boolean isScrubbedSpawn(Spawn spawn, @Nullable Scrubber scrubber) {
+    return scrubber != null && scrubber.forSpawn(spawn) != null;
+  }
+
   /**
    * A simple value class combining a hash of the tool inputs (and their digests) as well as a set
    * of the relative paths of all tool inputs.

src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java

@@ -314,10 +314,12 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
 
     boolean enableScrubbing = remoteOptions.scrubber != null;
     if (enableScrubbing && enableRemoteExecution) {
-
-      throw createOptionsExitException(
-          "Cannot combine remote cache key scrubbing with remote execution",
-          FailureDetails.RemoteOptions.Code.EXECUTION_WITH_SCRUBBING);
+      env.getReporter()
+          .handle(
+              Event.warn(
+                  "Cache key scrubbing is incompatible with remote execution. Actions that are"
+                      + " scrubbed per the --experimental_remote_scrubbing_config configuration"
+                      + " file will be executed locally instead."));
     }
 
     // TODO(bazel-team): Consider adding a warning or more validation if the remoteDownloadRegex is

src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java

@@ -728,9 +728,11 @@ public RemoteOutputsStrategyConverter() {
               + " cache entries and result in incorrect builds.\n\n"
               + "Scrubbing does not affect how an action is executed, only how its remote/disk"
               + " cache key is computed for the purpose of retrieving or storing an action result."
-              + " It cannot be used in conjunction with remote execution. Modifying the scrubbing"
-              + " configuration does not invalidate outputs present in the local filesystem or"
-              + " internal caches; a clean build is required to reexecute affected actions.\n\n"
+              + " Scrubbed actions are incompatible with remote execution, and will always be"
+              + " executed locally instead.\n\n"
+              + "Modifying the scrubbing configuration does not invalidate outputs present in the"
+              + " local filesystem or internal caches; a clean build is required to reexecute"
+              + " affected actions.\n\n"
               + "In order to successfully use this feature, you likely want to set a custom"
               + " --host_platform together with --experimental_platform_in_output_dir (to normalize"
               + " output prefixes) and --incompatible_strict_action_env (to normalize environment"

src/main/protobuf/failure_details.proto

@@ -295,7 +295,8 @@ message RemoteOptions {
     CREDENTIALS_WRITE_FAILURE = 3 [(metadata) = { exit_code: 36 }];
     DOWNLOADER_WITHOUT_GRPC_CACHE = 4 [(metadata) = { exit_code: 2 }];
     EXECUTION_WITH_INVALID_CACHE = 5 [(metadata) = { exit_code: 2 }];
-    EXECUTION_WITH_SCRUBBING = 6 [(metadata) = { exit_code: 2 }];
+
+    reserved 6;
   }
 
   Code code = 1;

src/test/shell/bazel/remote/remote_execution_test.sh

@@ -3384,28 +3384,46 @@ EOF
   bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \
         --//:src=//:foo :gen &> $TEST_log \
         || fail "failed to build with input foo and no cache"
-    expect_log "2 processes: 1 internal, 1 .*-sandbox"
+  expect_log "2 processes: 1 internal, 1 .*-sandbox"
 
   bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \
       --//:src=//:bar :gen &> $TEST_log \
       || fail "failed to build with input bar and no cache"
   expect_log "2 processes: 1 internal, 1 .*-sandbox"
 
-  # Now build with a cache. Even though Bazel considers the actions distinct,
-  # they will be looked up in the remote cache using the scrubbed key, so one
-  # can serve as a cache hit for the other.
+  # Now build with a disk cache. Even though Bazel considers the actions to be
+  # distinct, they will be looked up in the remote cache using the scrubbed key,
+  # so one can serve as a cache hit for the other.
 
   CACHEDIR=$(mktemp -d)
 
   bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \
         --disk_cache=$CACHEDIR --//:src=//:foo :gen &> $TEST_log \
-        || fail "failed to build with input foo and a cache"
+        || fail "failed to build with input foo and a disk cache miss"
   expect_log "2 processes: 1 internal, 1 .*-sandbox"
 
   bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \
       --disk_cache=$CACHEDIR --//:src=//:bar :gen &> $TEST_log \
-      || fail "failed to build with input bar and a cache"
+      || fail "failed to build with input bar and a disk cache hit"
   expect_log "2 processes: 1 disk cache hit, 1 internal"
+
+  # Now build with remote execution enabled. The first action should fall back
+  # to local execution. The second action should be able to hit the remote cache
+  # as it was cached by its scrubbed key.
+
+  bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \
+        --remote_executor=grpc://localhost:${worker_port} --//:src=//:foo \
+        :gen &> $TEST_log \
+        || fail "failed to build with input foo and remote execution"
+  expect_log "will be executed locally instead"
+  expect_log "2 processes: 1 internal, 1 .*-sandbox"
+
+  bazel build --experimental_remote_scrubbing_config=scrubbing.cfg \
+      --remote_executor=grpc://localhost:${worker_port} --//:src=//:bar \
+      :gen &> $TEST_log \
+      || fail "failed to build with input bar and a remote cache hit"
+  expect_log "will be executed locally instead"
+  expect_log "2 processes: 1 remote cache hit, 1 internal"
 }
 
 run_suite "Remote execution and remote cache tests"