Remote: Add support for tag no-remote-cache-upload.

coeuvre · copybara-github · commit 0d7d44d5b2e6 · 2021-12-07T06:35:19.000-08:00
When executing a spawn that is tagged with no-remote-cache-upload, Bazel still looks up the remote cache. However, its local outputs are not uploaded after local execution. Part of bazelbuild#14338. PiperOrigin-RevId: 414708472
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java
@@ -194,6 +194,9 @@ public enum WorkerProtocolFormat {
   /** Disables remote caching of a spawn. Note: does not disable remote execution */
   public static final String NO_REMOTE_CACHE = "no-remote-cache";
 
+  /** Disables upload part of remote caching of a spawn. Note: does not disable remote execution */
+  public static final String NO_REMOTE_CACHE_UPLOAD = "no-remote-cache-upload";
+
   /** Disables remote execution of a spawn. Note: does not disable remote caching */
   public static final String NO_REMOTE_EXEC = "no-remote-exec";
 
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
@@ -19,24 +19,33 @@
 import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
 import java.io.IOException;
 import java.time.Duration;
+import java.util.Map;
 
 /** Helper methods relating to implementations of {@link Spawn}. */
 public final class Spawns {
   private Spawns() {}
 
-  /**
-   * Returns {@code true} if the result of {@code spawn} may be cached.
-   */
+  /** Returns {@code true} if the result of {@code spawn} may be cached. */
   public static boolean mayBeCached(Spawn spawn) {
-    return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_CACHE)
-        && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL);
+    return mayBeCached(spawn.getExecutionInfo());
+  }
+
+  /** Returns {@code true} if the result of {@code spawn} may be cached. */
+  public static boolean mayBeCached(Map<String, String> executionInfo) {
+    return !executionInfo.containsKey(ExecutionRequirements.NO_CACHE)
+        && !executionInfo.containsKey(ExecutionRequirements.LOCAL);
   }
 
   /** Returns {@code true} if the result of {@code spawn} may be cached remotely. */
   public static boolean mayBeCachedRemotely(Spawn spawn) {
-    return mayBeCached(spawn)
-        && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE)
-        && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE_CACHE);
+    return mayBeCachedRemotely(spawn.getExecutionInfo());
+  }
+
+  /** Returns {@code true} if the result of {@code spawn} may be cached remotely. */
+  public static boolean mayBeCachedRemotely(Map<String, String> executionInfo) {
+    return mayBeCached(executionInfo)
+        && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE)
+        && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE_CACHE);
   }
 
   /** Returns {@code true} if {@code spawn} may be executed remotely. */
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
@@ -25,6 +25,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.AsyncCallable;
 import com.google.common.util.concurrent.FluentFuture;
@@ -71,6 +72,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Locale;
+import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.function.BiFunction;
@@ -596,9 +598,20 @@ public static boolean shouldAcceptCachedResultFromCombinedCache(
   }
 
   public static boolean shouldUploadLocalResultsToRemoteCache(
-      RemoteOptions remoteOptions, @Nullable Spawn spawn) {
+      RemoteOptions remoteOptions, @Nullable Map<String, String> executionInfo) {
     return remoteOptions.remoteUploadLocalResults
-        && (spawn == null || Spawns.mayBeCachedRemotely(spawn));
+        && (executionInfo == null
+            || (Spawns.mayBeCachedRemotely(executionInfo)
+                && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE_CACHE_UPLOAD)));
+  }
+
+  public static boolean shouldUploadLocalResultsToRemoteCache(
+      RemoteOptions remoteOptions, @Nullable Spawn spawn) {
+    ImmutableMap<String, String> executionInfo = null;
+    if (spawn != null) {
+      executionInfo = spawn.getExecutionInfo();
+    }
+    return shouldUploadLocalResultsToRemoteCache(remoteOptions, executionInfo);
   }
 
   public static boolean shouldUploadLocalResultsToDiskCache(
@@ -614,13 +627,11 @@ public static boolean shouldUploadLocalResultsToCombinedDisk(
       RemoteOptions remoteOptions, @Nullable Spawn spawn) {
     if (remoteOptions.incompatibleRemoteResultsIgnoreDisk) {
       // If --incompatible_remote_results_ignore_disk is set, we treat the disk cache part as local
-      // cache. Actions
-      // which are tagged with `no-remote-cache` can still hit the disk cache.
-      return spawn == null || Spawns.mayBeCached(spawn);
+      // cache. Actions which are tagged with `no-remote-cache` can still hit the disk cache.
+      return shouldUploadLocalResultsToDiskCache(remoteOptions, spawn);
     } else {
       // Otherwise, it's treated as a remote cache and disabled for `no-remote-cache`.
-      return remoteOptions.remoteUploadLocalResults
-          && (spawn == null || Spawns.mayBeCachedRemotely(spawn));
+      return shouldUploadLocalResultsToRemoteCache(remoteOptions, spawn);
     }
   }
 }
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -1900,6 +1900,42 @@ EOF
   expect_not_log "remote cache hit"
 }
 
+function test_tag_no_remote_cache_upload() {
+  mkdir -p a
+  cat > a/BUILD <<'EOF'
+genrule(
+  name = "foo",
+  srcs = [],
+  outs = ["foo.txt"],
+  cmd = "echo \"foo\" > \"$@\"",
+)
+EOF
+
+  bazel build \
+    --remote_cache=grpc://localhost:${worker_port} \
+    --modify_execution_info=.*=+no-remote-cache-upload \
+    //a:foo >& $TEST_log || "Failed to build //a:foo"
+
+  remote_ac_files="$(count_remote_ac_files)"
+  [[ "$remote_ac_files" == 0 ]] || fail "Expected 0 remote action cache entries, not $remote_ac_files"
+
+  # populate the cache
+  bazel clean
+
+  bazel build \
+    --remote_cache=grpc://localhost:${worker_port} \
+    //a:foo >& $TEST_log || "Failed to build //a:foo"
+
+  bazel clean
+
+  bazel build \
+    --remote_cache=grpc://localhost:${worker_port} \
+    --modify_execution_info=.*=+no-remote-cache-upload \
+    //a:foo >& $TEST_log || "Failed to build //a:foo"
+
+  expect_log "remote cache hit"
+}
+
 function test_tag_no_remote_cache_for_disk_cache() {
   mkdir -p a
   cat > a/BUILD <<'EOF'
