diff --git a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java index 080788e0ac9377..c4fa7fd15a1fd5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java +++ b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java @@ -84,8 +84,8 @@ public class UploadManifest { private final boolean followSymlinks; private final boolean allowDanglingSymlinks; private final boolean allowAbsoluteSymlinks; - private final Map digestToFile = new HashMap<>(); - private final Map digestToBlobs = new HashMap<>(); + private final Map casDigestToFile = new HashMap<>(); + private final Map casDigestToBlob = new HashMap<>(); @Nullable private ActionKey actionKey; private Digest stderrDigest; private Digest stdoutDigest; @@ -172,11 +172,11 @@ public UploadManifest( private void setStdoutStderr(FileOutErr outErr) throws IOException { if (outErr.getErrorPath().exists()) { stderrDigest = digestUtil.compute(outErr.getErrorPath()); - digestToFile.put(stderrDigest, outErr.getErrorPath()); + casDigestToFile.put(stderrDigest, outErr.getErrorPath()); } if (outErr.getOutputPath().exists()) { stdoutDigest = digestUtil.compute(outErr.getOutputPath()); - digestToFile.put(stdoutDigest, outErr.getOutputPath()); + casDigestToFile.put(stdoutDigest, outErr.getOutputPath()); } } @@ -255,22 +255,21 @@ void addFiles(Collection files) throws ExecException, IOException { private void addAction(RemoteCacheClient.ActionKey actionKey, Action action, Command command) { Preconditions.checkState(this.actionKey == null, "Already added an action"); this.actionKey = actionKey; - digestToBlobs.put(actionKey.getDigest(), action.toByteString()); - digestToBlobs.put(action.getCommandDigest(), command.toByteString()); + casDigestToBlob.put(action.getCommandDigest(), command.toByteString()); } /** Map of digests to file paths to upload. */ - public Map getDigestToFile() { - return digestToFile; + public Map getCasDigestToFile() { + return casDigestToFile; } /** * Map of digests to chunkers to upload. When the file is a regular, non-directory file it is - * transmitted through {@link #getDigestToFile()}. When it is a directory, it is transmitted as a - * {@link Tree} protobuf message through {@link #getDigestToBlobs()}. + * transmitted through {@link #getCasDigestToFile()}. When it is a directory, it is transmitted as + * a {@link Tree} protobuf message through {@link #getCasDigestToBlob()}. */ - public Map getDigestToBlobs() { - return digestToBlobs; + public Map getCasDigestToBlob() { + return casDigestToBlob; } @Nullable @@ -311,7 +310,7 @@ private void addFile(Digest digest, Path file) throws IOException { // The permission of output file is changed to 0555 after action execution .setIsExecutable(true); - digestToFile.put(digest, file); + casDigestToFile.put(digest, file); } // Field numbers of the 'root' and 'directory' fields in the Tree message. @@ -347,7 +346,7 @@ private void addDirectory(Path dir) throws ExecException, IOException { .setIsTopologicallySorted(true); } - digestToBlobs.put(digest, data); + casDigestToBlob.put(digest, data); } private ByteString computeDirectory(Path path, Set directories) @@ -379,7 +378,7 @@ private ByteString computeDirectory(Path path, Set directories) if (statFollow.isFile() && !statFollow.isSpecialFile()) { Digest digest = digestUtil.compute(child); b.addFilesBuilder().setName(name).setDigest(digest).setIsExecutable(child.isExecutable()); - digestToFile.put(digest, child); + casDigestToFile.put(digest, child); } else if (statFollow.isDirectory()) { ByteString dir = computeDirectory(child, directories); b.addDirectoriesBuilder().setName(name).setDigest(digestUtil.compute(dir.toByteArray())); @@ -389,7 +388,7 @@ private ByteString computeDirectory(Path path, Set directories) } else if (dirent.getType() == Dirent.Type.FILE) { Digest digest = digestUtil.compute(child); b.addFilesBuilder().setName(name).setDigest(digest).setIsExecutable(child.isExecutable()); - digestToFile.put(digest, child); + casDigestToFile.put(digest, child); } else { illegalOutput(child); } @@ -439,12 +438,12 @@ public ActionResult upload( private Completable upload( RemoteActionExecutionContext context, RemoteCache remoteCache, Digest digest) { - Path file = digestToFile.get(digest); + Path file = casDigestToFile.get(digest); if (file != null) { return toCompletable(() -> remoteCache.uploadFile(context, digest, file), directExecutor()); } - ByteString blob = digestToBlobs.get(digest); + ByteString blob = casDigestToBlob.get(digest); if (blob == null) { String message = "FindMissingBlobs call returned an unknown digest: " + digest; return Completable.error(new IOException(message)); @@ -487,8 +486,8 @@ public Single uploadAsync( RemoteCache remoteCache, ExtendedEventHandler reporter) { Collection digests = new ArrayList<>(); - digests.addAll(digestToFile.keySet()); - digests.addAll(digestToBlobs.keySet()); + digests.addAll(casDigestToFile.keySet()); + digests.addAll(casDigestToBlob.keySet()); ActionExecutionMetadata action = context.getSpawnOwner(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java index d0377e21ad3584..a5b3ebce1eb93d 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java @@ -620,8 +620,7 @@ public void findMissingBlobs( FindMissingBlobsRequest request, StreamObserver responseObserver) { assertThat(request.getBlobDigestsList()) - .containsExactly( - cmdDigest, actionDigest, fooDigest, barDigest, stdoutDigest, stderrDigest); + .containsExactly(cmdDigest, fooDigest, barDigest, stdoutDigest, stderrDigest); // Nothing is missing. responseObserver.onNext(FindMissingBlobsResponse.getDefaultInstance()); responseObserver.onCompleted(); @@ -725,7 +724,7 @@ public void updateActionResult( .setDigest(barDigest) .setIsExecutable(true); assertThat(result).isEqualTo(expectedResult.build()); - assertThat(numGetMissingCalls.get()).isEqualTo(4); + assertThat(numGetMissingCalls.get()).isEqualTo(3); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java b/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java index c0b93f2a4d002c..eb5a704e7822d7 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java @@ -214,7 +214,7 @@ public void actionResult_followSymlinks_absoluteFileSymlinkAsFile() throws Excep /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(target); - assertThat(um.getDigestToFile()).containsExactly(digest, link); + assertThat(um.getCasDigestToFile()).containsExactly(digest, link); ActionResult.Builder expectedResult = ActionResult.newBuilder(); expectedResult.addOutputFilesBuilder().setPath("link").setDigest(digest).setIsExecutable(true); @@ -241,7 +241,7 @@ public void actionResult_followSymlinks_absoluteDirectorySymlinkAsDirectory() th /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); + assertThat(um.getCasDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); Tree tree = Tree.newBuilder() @@ -278,7 +278,7 @@ public void actionResult_noFollowSymlinks_absoluteFileSymlinkAsFile() throws Exc /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(target); - assertThat(um.getDigestToFile()).containsExactly(digest, link); + assertThat(um.getCasDigestToFile()).containsExactly(digest, link); ActionResult.Builder expectedResult = ActionResult.newBuilder(); expectedResult.addOutputFilesBuilder().setPath("link").setDigest(digest).setIsExecutable(true); @@ -305,7 +305,7 @@ public void actionResult_noFollowSymlinks_absoluteDirectorySymlinkAsDirectory() /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); + assertThat(um.getCasDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); Tree tree = Tree.newBuilder() @@ -342,7 +342,7 @@ public void actionResult_followSymlinks_relativeFileSymlinkAsFile() throws Excep /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(target); - assertThat(um.getDigestToFile()).containsExactly(digest, link); + assertThat(um.getCasDigestToFile()).containsExactly(digest, link); ActionResult.Builder expectedResult = ActionResult.newBuilder(); expectedResult.addOutputFilesBuilder().setPath("link").setDigest(digest).setIsExecutable(true); @@ -369,7 +369,7 @@ public void actionResult_followSymlinks_relativeDirectorySymlinkAsDirectory() th /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); + assertThat(um.getCasDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); Tree tree = Tree.newBuilder() @@ -405,7 +405,7 @@ public void actionResult_noFollowSymlinks_relativeFileSymlinkAsSymlink() throws /*allowDanglingSymlinks=*/ false, /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); - assertThat(um.getDigestToFile()).isEmpty(); + assertThat(um.getCasDigestToFile()).isEmpty(); ActionResult.Builder expectedResult = ActionResult.newBuilder(); expectedResult.addOutputFileSymlinksBuilder().setPath("link").setTarget("target"); @@ -432,7 +432,7 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkAsSymlink() th /*allowDanglingSymlinks=*/ false, /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); - assertThat(um.getDigestToFile()).isEmpty(); + assertThat(um.getCasDigestToFile()).isEmpty(); ActionResult.Builder expectedResult = ActionResult.newBuilder(); expectedResult.addOutputDirectorySymlinksBuilder().setPath("link").setTarget("dir"); @@ -503,7 +503,7 @@ public void actionResult_noFollowSymlinks_noAllowDanglingSymlinks_absoluteDangli /*allowDanglingSymlinks=*/ true, /*allowAbsoluteSymlinks=*/ true); um.addFiles(ImmutableList.of(link)); - assertThat(um.getDigestToFile()).isEmpty(); + assertThat(um.getCasDigestToFile()).isEmpty(); ActionResult.Builder expectedResult = ActionResult.newBuilder(); expectedResult.addOutputFileSymlinksBuilder().setPath("link").setTarget("/execroot/target"); @@ -550,7 +550,7 @@ public void actionResult_noFollowSymlinks_allowDanglingSymlinks_relativeDangling /*allowDanglingSymlinks=*/ true, /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); - assertThat(um.getDigestToFile()).isEmpty(); + assertThat(um.getCasDigestToFile()).isEmpty(); ActionResult.Builder expectedResult = ActionResult.newBuilder(); expectedResult.addOutputFileSymlinksBuilder().setPath("link").setTarget("target"); @@ -578,7 +578,7 @@ public void actionResult_followSymlinks_absoluteFileSymlinkInDirectoryAsFile() t /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(target); - assertThat(um.getDigestToFile()).containsExactly(digest, link); + assertThat(um.getCasDigestToFile()).containsExactly(digest, link); Tree tree = Tree.newBuilder() @@ -620,7 +620,8 @@ public void actionResult_followSymlinks_absoluteDirectorySymlinkInDirectoryAsDir /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("dir/link/foo")); + assertThat(um.getCasDigestToFile()) + .containsExactly(digest, execRoot.getRelative("dir/link/foo")); Directory barDir = Directory.newBuilder() @@ -667,7 +668,7 @@ public void actionResult_noFollowSymlinks_absoluteFileSymlinkInDirectoryAsFile() /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(target); - assertThat(um.getDigestToFile()).containsExactly(digest, link); + assertThat(um.getCasDigestToFile()).containsExactly(digest, link); Tree tree = Tree.newBuilder() @@ -709,7 +710,8 @@ public void actionResult_noFollowSymlinks_absoluteDirectorySymlinkInDirectoryAsD /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("dir/link/foo")); + assertThat(um.getCasDigestToFile()) + .containsExactly(digest, execRoot.getRelative("dir/link/foo")); Directory barDir = Directory.newBuilder() @@ -755,7 +757,7 @@ public void actionResult_followSymlinks_relativeFileSymlinkInDirectoryAsFile() t /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(target); - assertThat(um.getDigestToFile()).containsExactly(digest, link); + assertThat(um.getCasDigestToFile()).containsExactly(digest, link); Tree tree = Tree.newBuilder() @@ -797,7 +799,8 @@ public void actionResult_followSymlinks_relativeDirectorySymlinkInDirectoryAsDir /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("dir/link/foo")); + assertThat(um.getCasDigestToFile()) + .containsExactly(digest, execRoot.getRelative("dir/link/foo")); Directory barDir = Directory.newBuilder() @@ -843,7 +846,7 @@ public void actionResult_noFollowSymlinks_relativeFileSymlinkInDirectoryAsSymlin /*allowDanglingSymlinks=*/ false, /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); - assertThat(um.getDigestToFile()).isEmpty(); + assertThat(um.getCasDigestToFile()).isEmpty(); Tree tree = Tree.newBuilder() @@ -884,7 +887,7 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS /*allowDanglingSymlinks=*/ false, /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); - assertThat(um.getDigestToFile()).isEmpty(); + assertThat(um.getCasDigestToFile()).isEmpty(); Tree tree = Tree.newBuilder() @@ -973,7 +976,7 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS /*allowDanglingSymlinks=*/ false, /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); - assertThat(um.getDigestToFile()).isEmpty(); + assertThat(um.getCasDigestToFile()).isEmpty(); Tree tree = Tree.newBuilder() @@ -1012,7 +1015,7 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS /*allowDanglingSymlinks=*/ true, /*allowAbsoluteSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); - assertThat(um.getDigestToFile()).isEmpty(); + assertThat(um.getCasDigestToFile()).isEmpty(); Tree tree = Tree.newBuilder() diff --git a/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh b/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh index 5f74746490ca95..a5776121c7f948 100755 --- a/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh +++ b/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh @@ -224,11 +224,11 @@ EOF function test_upload_minimal_respect_no_upload_results_combined_cache() { local cache_dir="${TEST_TMPDIR}/disk_cache" mkdir -p a - cat > a/BUILD < a/BUILD <<'EOF' genrule( name = 'foo', outs = ["foo.txt"], - cmd = "echo \"foo bar\" > \$@", + cmd = "echo out && echo err 1>&2 && echo 'foo bar' > $@", ) EOF @@ -246,18 +246,18 @@ EOF remote_cas_files="$(count_remote_cas_files)" [[ "$remote_cas_files" == 1 ]] || fail "Expected 1 remote cas entries, not $remote_cas_files" 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" + # foo.txt, stdout, stderr, Command proto for action 'foo' + [[ "$disk_cas_files" == 4 ]] || fail "Expected 4 disk cas entries, not $disk_cas_files" } function test_upload_all_combined_cache() { local cache_dir="${TEST_TMPDIR}/disk_cache" mkdir -p a - cat > a/BUILD < a/BUILD <<'EOF' genrule( name = 'foo', outs = ["foo.txt"], - cmd = "echo \"foo bar\" > \$@", + cmd = "echo out && echo err 1>&2 && echo 'foo bar' > $@", ) EOF @@ -275,8 +275,8 @@ EOF remote_cas_files="$(count_remote_cas_files)" [[ "$remote_cas_files" == 1 ]] || fail "Expected 1 remote cas entries, not $remote_cas_files" 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" + # foo.txt, stdout, stderr, Command proto for action 'foo' + [[ "$disk_cas_files" == 4 ]] || fail "Expected 4 disk cas entries, not $disk_cas_files" } function test_upload_minimal_alias_action_doesnt_upload_missing_blobs() { diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 3b903f2cf2910d..5bbbe526efe862 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -1737,7 +1737,8 @@ EOF remote_ac_files="$(count_remote_ac_files)" [[ "$remote_ac_files" == 1 ]] || fail "Expected 1 remote action cache entries, not $remote_ac_files" remote_cas_files="$(count_remote_cas_files)" - [[ "$remote_cas_files" == 3 ]] || fail "Expected 3 remote cas entries, not $remote_cas_files" + # foo.txt + Command proto; stdout/stderr not uploaded because they're empty + [[ "$remote_cas_files" == 2 ]] || fail "Expected 2 remote cas entries, not $remote_cas_files" } function test_combined_cache_with_no_remote_cache_tag_remote_cache() {