[6.4.0] Fix output materialized as symlink when building without the …

…bytes.

This appears to have worked in 6.2.x, got broken in 6.3.x, and is working
again at head. The divergence is too great at this point, so it's not possible
to cherry-pick the necessary changes from head. Instead, I've written a
targeted fix that falls back to downloading every target output if at least
one of the outputs was materialized as a symlink.

Fixes bazelbuild#19143.

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) {

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",

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);
+  }
 }