diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index 8cf0a8128d0ba6..149f9c752c1e01 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/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. 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 index e58bc28010ef4b..bfdde5a950f513 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/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 diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index 9fe119bc5b6fed..31bd22fd56915a 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/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" diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 5170d66d180400..038f15755829fd 100644 --- a/src/main/protobuf/failure_details.proto +++ b/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; diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 09e76fc3e2d075..060b322574539d 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/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"