Skip to content

Commit

Permalink
Fix incorrect stdout/stderr in remote action cache. Fixes #9072
Browse files Browse the repository at this point in the history
Commit a6b5b05 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
  • Loading branch information
buchgr authored and copybara-github committed Aug 6, 2019
1 parent 47d0173 commit b7d300c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -639,14 +639,24 @@ 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
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();
Expand All @@ -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()
Expand Down

0 comments on commit b7d300c

Please sign in to comment.