Remote: Make --incompatible_remote_build_event_upload_respect_no_cache working with --incompatible_remote_results_ignore_disk.

coeuvre · coeuvre · commit cc8a56e4d587 · 2021-12-23T14:39:14.000Z
Fixes #14463. Closes #14468. PiperOrigin-RevId: 417984062
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
@@ -356,19 +356,6 @@ public boolean shouldAcceptCachedResult(Spawn spawn) {
     }
   }
 
-  public static boolean shouldUploadLocalResults(
-      RemoteOptions remoteOptions, @Nullable Map<String, String> executionInfo) {
-    if (useRemoteCache(remoteOptions)) {
-      if (useDiskCache(remoteOptions)) {
-        return shouldUploadLocalResultsToCombinedDisk(remoteOptions, executionInfo);
-      } else {
-        return shouldUploadLocalResultsToRemoteCache(remoteOptions, executionInfo);
-      }
-    } else {
-      return shouldUploadLocalResultsToDiskCache(remoteOptions, executionInfo);
-    }
-  }
-
   /**
    * Returns {@code true} if the local results of the {@code spawn} should be uploaded to remote
    * cache.
@@ -378,7 +365,15 @@ public boolean shouldUploadLocalResults(Spawn spawn) {
       return false;
     }
 
-    return shouldUploadLocalResults(remoteOptions, spawn.getExecutionInfo());
+    if (useRemoteCache(remoteOptions)) {
+      if (useDiskCache(remoteOptions)) {
+        return shouldUploadLocalResultsToCombinedDisk(remoteOptions, spawn);
+      } else {
+        return shouldUploadLocalResultsToRemoteCache(remoteOptions, spawn);
+      }
+    } else {
+      return shouldUploadLocalResultsToDiskCache(remoteOptions, spawn);
+    }
   }
 
   /** Returns {@code true} if the spawn may be executed remotely. */
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
@@ -761,8 +761,7 @@ private void parseNoCacheOutputs(AnalysisResult analysisResult) {
         RuleConfiguredTarget ruleConfiguredTarget = (RuleConfiguredTarget) configuredTarget;
         for (ActionAnalysisMetadata action : ruleConfiguredTarget.getActions()) {
           boolean uploadLocalResults =
-              RemoteExecutionService.shouldUploadLocalResults(
-                  remoteOptions, action.getExecutionInfo());
+              Utils.shouldUploadLocalResultsToRemoteCache(remoteOptions, action.getExecutionInfo());
           if (!uploadLocalResults) {
             for (Artifact output : action.getOutputs()) {
               if (output.isTreeArtifact()) {
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -3469,4 +3469,34 @@ EOF
   [[ "$disk_cas_files" == 0 ]] || fail "Expected 0 disk cas entries, not $disk_cas_files"
 }
 
+function test_uploader_incompatible_remote_results_ignore_disk() {
+  mkdir -p a
+  cat > a/BUILD <<EOF
+genrule(
+  name = 'foo',
+  outs = ["foo.txt"],
+  cmd = "echo \"foo bar\" > \$@",
+  tags = ["no-remote"],
+)
+EOF
+
+  cache_dir=$(mktemp -d)
+
+  bazel build \
+      --remote_cache=grpc://localhost:${worker_port} \
+      --disk_cache=$cache_dir \
+      --incompatible_remote_build_event_upload_respect_no_cache \
+      --incompatible_remote_results_ignore_disk \
+      --build_event_json_file=bep.json \
+      //a:foo >& $TEST_log || fail "Failed to build"
+
+  cat bep.json > $TEST_log
+  expect_not_log "a:foo.*bytestream://" || fail "local files are converted"
+  expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
+
+  disk_cas_files="$(count_disk_cas_files $cache_dir)"
+  # foo.txt, stdout and stderr for action 'foo'
+  [[ "$disk_cas_files" == 3 ]] || fail "Expected 3 disk cas entries, not $disk_cas_files"
+}
+
 run_suite "Remote execution and remote cache tests"
