From adc9bcfbe9a8cfafe36ee4cbbaeb0fdfc9e161ff Mon Sep 17 00:00:00 2001
From: Chi Wang <chiwang@google.com>
Date: Thu, 23 Dec 2021 04:14:50 -0800
Subject: [PATCH] Remote: Make
 --incompatible_remote_build_event_upload_respect_no_cache working with
 --incompatible_remote_results_ignore_disk.

Fixes https://github.com/bazelbuild/bazel/issues/14463.

Closes #14468.

PiperOrigin-RevId: 417984062
---
 .../lib/remote/RemoteExecutionService.java    | 23 ++++++--------
 .../build/lib/remote/RemoteModule.java        |  3 +-
 .../bazel/remote/remote_execution_test.sh     | 30 +++++++++++++++++++
 3 files changed, 40 insertions(+), 16 deletions(-)

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 7600cc9c3a35ff..195e1b6f61663c 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
@@ -356,19 +356,6 @@ public boolean shouldAcceptCachedResult(Spawn spawn) {
     }
   }
 
-  public static boolean shouldUploadLocalResults(
-      RemoteOptions remoteOptions, 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
index bfb16b6bd3aa43..30560a6235ba8a 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
@@ -763,8 +763,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
index 71fc9e71d3a548..dfdf71dfe90182 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -3505,4 +3505,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"