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 be428b7dfc3fde..618677fb559886 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 @@ -1048,13 +1048,21 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re hasFilesToDownload(action.spawn.getOutputFiles(), filesToDownload)); if (downloadOutputs) { + HashSet queuedFilePaths = new HashSet<>(); + for (FileMetadata file : metadata.files()) { - downloadsBuilder.add(downloadFile(action, file)); + PathFragment filePath = file.path().asFragment(); + if (queuedFilePaths.add(filePath)) { + downloadsBuilder.add(downloadFile(action, file)); + } } for (Map.Entry entry : metadata.directories()) { for (FileMetadata file : entry.getValue().files()) { - downloadsBuilder.add(downloadFile(action, file)); + PathFragment filePath = file.path().asFragment(); + if (queuedFilePaths.add(filePath)) { + downloadsBuilder.add(downloadFile(action, file)); + } } } } else { diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index 18b5a688106763..dad364ca44b1be 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -360,6 +360,41 @@ public void downloadOutputs_nestedOutputDirectories_works() throws Exception { assertThat(context.isLockOutputFilesCalled()).isTrue(); } + @Test + public void downloadOutputs_outputDirectoriesWithNestedFile_works() throws Exception { + // Test that downloading an output directory containing a named output file works. + + // arrange + Digest fooDigest = cache.addContents(remoteActionExecutionContext, "foo-contents"); + Digest barDigest = cache.addContents(remoteActionExecutionContext, "bar-ontents"); + Tree subdirTreeMessage = + Tree.newBuilder() + .setRoot( + Directory.newBuilder() + .addFiles(FileNode.newBuilder().setName("foo").setDigest(fooDigest)) + .addFiles(FileNode.newBuilder().setName("bar").setDigest(barDigest))) + .build(); + Digest subdirTreeDigest = + cache.addContents(remoteActionExecutionContext, subdirTreeMessage.toByteArray()); + ActionResult.Builder builder = ActionResult.newBuilder(); + builder.addOutputFilesBuilder().setPath("outputs/subdir/foo").setDigest(fooDigest); + builder.addOutputDirectoriesBuilder().setPath("outputs/subdir").setTreeDigest(subdirTreeDigest); + RemoteActionResult result = + RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build())); + Spawn spawn = newSpawnFromResult(result); + FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn); + RemoteExecutionService service = newRemoteExecutionService(); + RemoteAction action = service.buildRemoteAction(spawn, context); + + // act + service.downloadOutputs(action, result); + + // assert + assertThat(digestUtil.compute(execRoot.getRelative("outputs/subdir/foo"))).isEqualTo(fooDigest); + assertThat(digestUtil.compute(execRoot.getRelative("outputs/subdir/bar"))).isEqualTo(barDigest); + assertThat(context.isLockOutputFilesCalled()).isTrue(); + } + @Test public void downloadOutputs_outputDirectoriesWithSameHash_works() throws Exception { // Test that downloading an output directory works when two Directory