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 ebe6efc6cb8659..506b6da393a9d0 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 @@ -1255,6 +1255,11 @@ private boolean shouldDownloadOutputsFor(RemoteActionResult result) { if (remoteOptions.remoteOutputsMode.downloadAllOutputs()) { return true; } + // An output materialized as a symlink might point to one of the other outputs. + if (!result.getOutputSymlinks().isEmpty() || !result.getOutputFileSymlinks().isEmpty() + || !result.getOutputDirectorySymlinks().isEmpty()) { + return true; + } // In case the action failed, download all outputs. It might be helpful for debugging and there // is no point in injecting output metadata of a failed action. if (result.getExitCode() != 0) { diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index bcdc577ff5ef0e..08460cc65eeab2 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -11,8 +11,8 @@ filegroup( srcs = glob(["**"]) + [ "//src/test/java/com/google/devtools/build/lib/remote/circuitbreaker:srcs", "//src/test/java/com/google/devtools/build/lib/remote/downloader:srcs", - "//src/test/java/com/google/devtools/build/lib/remote/http:srcs", "//src/test/java/com/google/devtools/build/lib/remote/grpc:srcs", + "//src/test/java/com/google/devtools/build/lib/remote/http:srcs", "//src/test/java/com/google/devtools/build/lib/remote/logging:srcs", "//src/test/java/com/google/devtools/build/lib/remote/merkletree:srcs", "//src/test/java/com/google/devtools/build/lib/remote/options:srcs", @@ -169,6 +169,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/standalone", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/test/java/com/google/devtools/build/lib/remote/util:integration_test_utils", "//third_party:guava", "//third_party:junit4", diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java index c6ea8418c1a46d..e0ca5951cf40ce 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; import java.io.IOException; import org.junit.After; @@ -680,4 +681,49 @@ public void remoteCacheEvictBlobs_whenUploadingInputTree_incrementalBuildCanCont "file-inside" + lineSeparator() + "updated bar" + lineSeparator(), /* isLocal= */ true); } + + @Test + public void downloadTopLevel_genruleSymlinkToOutput() throws Exception { + // Disable test on Windows. + assumeFalse(OS.getCurrent() == OS.WINDOWS); + + setDownloadToplevel(); + write( + "BUILD", + "genrule(", + " name = 'gen',", + " outs = ['foo', 'foo-link'],", + " cmd = 'cd $(RULEDIR) && echo hello > foo && ln -s foo foo-link',", + // In Blaze, heuristic label expansion defaults to True and will cause `foo` to be expanded + // into `blaze-out/.../bin/foo` in the genrule command line. + " heuristic_label_expansion = False,", + ")"); + + buildTarget("//:gen"); + + assertSymlink("foo-link", PathFragment.create("foo")); + assertValidOutputFile("foo-link", "hello\n"); + + // Delete link, re-plant symlink + getOutputPath("foo").delete(); + buildTarget("//:gen"); + + assertSymlink("foo-link", PathFragment.create("foo")); + assertValidOutputFile("foo-link", "hello\n"); + + // Delete target, re-download it + getOutputPath("foo").delete(); + + buildTarget("//:gen"); + + assertSymlink("foo-link", PathFragment.create("foo")); + assertValidOutputFile("foo-link", "hello\n"); + } + + protected void assertSymlink(String binRelativeLinkPath, PathFragment targetPath) + throws Exception { + Path output = getOutputPath(binRelativeLinkPath); + assertThat(output.isSymbolicLink()).isTrue(); + assertThat(output.readSymbolicLink()).isEqualTo(targetPath); + } }