commit 9c0cab44116a65b8ee5614bc84c54692d9456bed
author Bor Kae Hwang <borkaehw@lucidchart.com> Thu Dec 19 06:39:10 2019 -0800
committer Copybara-Service <copybara-worker@google.com> Thu Dec 19 06:40:11 2019 -0800
tree a1e8bbf6252720cbab72bd8e10756435d7b0745f
parent 98ad7d61309b264c47d22186768d091b33325be7

Fix noremote_upload_local_results behavior in combined blobstore

Disable the upload action to remote cache when `--noremote_upload_local_results` is set.

This problem would only happen in `CombinedDiskHttpBlobStore` at this moment when users want read/write permission to disk cache but only read permission to remote cache.

Once the GrpcCache refactoring (making GrpcCache a SimpleBlobStore implementation) is done, `CombinedDiskHttpBlobStore` should be renamed to `CombinedDiskRemoteBlobStore`. Then this fix will apply to both GrpcCache and HttpCache.

Fix https://github.com/bazelbuild/bazel/issues/8216

Closes #9338.

PiperOrigin-RevId: 286382882

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

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index 5c25075..689c024 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -114,16 +114,11 @@
   public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
       throws InterruptedException, IOException, ExecException {
     if (!Spawns.mayBeCached(spawn)
-        || (!Spawns.mayBeCachedRemotely(spawn) && isRemoteCache(options))) {
+        || (!Spawns.mayBeCachedRemotely(spawn) && useRemoteCache(options))) {
       // returning SpawnCache.NO_RESULT_NO_STORE in case the caching is disabled or in case
       // the remote caching is disabled and the only configured cache is remote.
       return SpawnCache.NO_RESULT_NO_STORE;
     }
-    boolean checkCache = options.remoteAcceptCached;
-
-    if (checkCache) {
-      context.report(ProgressStatus.CHECKING_CACHE, "remote-cache");
-    }
 
     SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(true);
     MerkleTree merkleTree =
@@ -150,7 +145,9 @@
         TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey);
 
     Profiler prof = Profiler.instance();
-    if (checkCache) {
+    if (options.remoteAcceptCached
+        || (options.incompatibleRemoteResultsIgnoreDisk && useDiskCache(options))) {
+      context.report(ProgressStatus.CHECKING_CACHE, "remote-cache");
       // Metadata will be available in context.current() until we detach.
       // This is done via a thread-local variable.
       Context previous = withMetadata.attach();
@@ -211,7 +208,8 @@
 
     context.prefetchInputs();
 
-    if (options.remoteUploadLocalResults) {
+    if (options.remoteUploadLocalResults
+        || (options.incompatibleRemoteResultsIgnoreDisk && useDiskCache(options))) {
       return new CacheHandle() {
         @Override
         public boolean hasResult() {
@@ -297,7 +295,11 @@
     }
   }
 
-  private static boolean isRemoteCache(RemoteOptions options) {
+  private static boolean useRemoteCache(RemoteOptions options) {
     return !isNullOrEmpty(options.remoteCache);
   }
+
+  private static boolean useDiskCache(RemoteOptions options) {
+    return options.diskCache != null && !options.diskCache.isEmpty();
+  }
 }