From b7d300c6be3e130dec0e62a4f19493105f595d57 Mon Sep 17 00:00:00 2001
From: Jakob Buchgraber <buchgr@google.com>
Date: Tue, 6 Aug 2019 04:25:24 -0700
Subject: [PATCH] Fix incorrect stdout/stderr in remote action cache. Fixes
 #9072

Commit a6b5b0540 introduced a bug where the stdout/test.log of
an action was incorrectly added as an output file to the
ActionResult protobuf. Fortunately, we found the bug on Windows
because one cannot delete a file while it's still being open.

PAIR=pcloudy

Closes #9087.

PiperOrigin-RevId: 261885751
---
 .../lib/remote/AbstractRemoteActionCache.java    |  4 ++--
 .../build/lib/remote/GrpcRemoteCacheTest.java    | 16 ++++++++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
index 03b417d3fba400..8d27c2e2d5dce8 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
@@ -665,11 +665,11 @@ public UploadManifest(
     public void setStdoutStderr(FileOutErr outErr) throws IOException {
       if (outErr.getErrorPath().exists()) {
         stderrDigest = digestUtil.compute(outErr.getErrorPath());
-        addFile(stderrDigest, outErr.getErrorPath());
+        digestToFile.put(stderrDigest, outErr.getErrorPath());
       }
       if (outErr.getOutputPath().exists()) {
         stdoutDigest = digestUtil.compute(outErr.getOutputPath());
-        addFile(stdoutDigest, outErr.getOutputPath());
+        digestToFile.put(stdoutDigest, outErr.getOutputPath());
       }
     }
 
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
index a33ebdb7b1fe86..148205386986a6 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
@@ -626,7 +626,7 @@ private ActionResult uploadDirectory(GrpcRemoteCache client, List<Path> outputs)
   }
 
   @Test
-  public void testUploadCacheHits() throws Exception {
+  public void testUpload() throws Exception {
     final GrpcRemoteCache client = newClient();
     final Digest fooDigest =
         fakeFileCache.createScratchInput(ActionInputHelper.fromPath("a/foo"), "xyz");
@@ -639,6 +639,15 @@ public void testUploadCacheHits() throws Exception {
     final Digest cmdDigest = DIGEST_UTIL.compute(command.toByteArray());
     Action action = Action.newBuilder().setCommandDigest(cmdDigest).build();
     final Digest actionDigest = DIGEST_UTIL.compute(action.toByteArray());
+
+    outErr.getOutputStream().write("foo out".getBytes(UTF_8));
+    outErr.getOutputStream().close();
+    outErr.getErrorStream().write("foo err".getBytes(UTF_8));
+    outErr.getOutputStream().close();
+
+    final Digest stdoutDigest = DIGEST_UTIL.compute(outErr.getOutputPath());
+    final Digest stderrDigest = DIGEST_UTIL.compute(outErr.getErrorPath());
+
     serviceRegistry.addService(
         new ContentAddressableStorageImplBase() {
           @Override
@@ -646,7 +655,8 @@ public void findMissingBlobs(
               FindMissingBlobsRequest request,
               StreamObserver<FindMissingBlobsResponse> responseObserver) {
             assertThat(request.getBlobDigestsList())
-                .containsExactly(cmdDigest, actionDigest, fooDigest, barDigest);
+                .containsExactly(
+                    cmdDigest, actionDigest, fooDigest, barDigest, stdoutDigest, stderrDigest);
             // Nothing is missing.
             responseObserver.onNext(FindMissingBlobsResponse.getDefaultInstance());
             responseObserver.onCompleted();
@@ -663,6 +673,8 @@ public void findMissingBlobs(
         outErr,
         result);
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
+    expectedResult.setStdoutDigest(stdoutDigest);
+    expectedResult.setStderrDigest(stderrDigest);
     expectedResult.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest);
     expectedResult
         .addOutputFilesBuilder()