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
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 03b417d..8d27c2e 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 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 a33ebdb..1482053 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 @@
}
@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 @@
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 @@
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 @@
outErr,
result);
ActionResult.Builder expectedResult = ActionResult.newBuilder();
+ expectedResult.setStdoutDigest(stdoutDigest);
+ expectedResult.setStderrDigest(stderrDigest);
expectedResult.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest);
expectedResult
.addOutputFilesBuilder()
