RemoteExecutionService: support output_symlinks in ActionResult

Since Remote API v2.1, both
  - output_file_symlinks
  - output_directory_symlinks
were marked as deprecated in favor of output_symlinks.

However, Bazel downloadOutputs has only support the 2 deprecated fields
and not output_symlinks.

Add support for output_symlinks.

Author: sluongng

src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java

@@ -644,6 +644,10 @@ public List<OutputSymlink> getOutputDirectorySymlinks() {
       return actionResult.getOutputDirectorySymlinksList();
     }
 
+    public List<OutputSymlink> getOutputSymlinks() {
+      return actionResult.getOutputSymlinksList();
+    }
+
     /**
      * Returns the freeform informational message with details on the execution of the action that
      * may be displayed to the user upon failure or when requested explicitly.
@@ -1089,8 +1093,11 @@ ActionResultMetadata parseActionResultMetadata(
     }
 
     ImmutableMap.Builder<Path, SymlinkMetadata> symlinks = ImmutableMap.builder();
-    Iterable<OutputSymlink> outputSymlinks =
-        Iterables.concat(result.getOutputFileSymlinks(), result.getOutputDirectorySymlinks());
+    var outputSymlinks = ImmutableSet.<OutputSymlink>builder()
+      .addAll(result.getOutputFileSymlinks())
+      .addAll(result.getOutputDirectorySymlinks())
+      .addAll(result.getOutputSymlinks())
+      .build();
     for (OutputSymlink symlink : outputSymlinks) {
       Path localPath =
           remotePathResolver.outputPathToLocalPath(encodeBytestringUtf8(symlink.getPath()));

src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java

@@ -590,6 +590,54 @@ public void downloadOutputs_relativeDirectorySymlink_success() throws Exception
     assertThat(context.isLockOutputFilesCalled()).isTrue();
   }
 
+  @Test
+  public void downloadOutputs_relativeOutputSymlinks_success() throws Exception {
+    // Test that download outputs works when the action result only contains output_symlinks
+    // and not output_file_symlinks or output_directory_symlinks, which are deprecated.
+    ActionResult.Builder builder = ActionResult.newBuilder();
+    builder.addOutputSymlinksBuilder().setPath("outputs/a/b/link").setTarget("../../foo");
+    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);
+
+    // Doesn't check for dangling links, hence download succeeds.
+    service.downloadOutputs(action, result);
+
+    Path path = execRoot.getRelative("outputs/a/b/link");
+    assertThat(path.isSymbolicLink()).isTrue();
+    assertThat(path.readSymbolicLink()).isEqualTo(PathFragment.create("../../foo"));
+    assertThat(context.isLockOutputFilesCalled()).isTrue();
+  }
+
+  @Test
+  public void downloadOutputs_outputSymlinksCompatibility_success() throws Exception {
+    // Test that download outputs works when the action result contains both output_symlinks
+    // and output_file_symlinks (or output_directory_symlinks).
+    //
+    // Remote Execution Server may set both fields to ensure backward compatibility with
+    // clients that don't support output_symlinks.
+    ActionResult.Builder builder = ActionResult.newBuilder();
+    builder.addOutputFileSymlinksBuilder().setPath("outputs/a/b/link").setTarget("foo");
+    builder.addOutputSymlinksBuilder().setPath("outputs/a/b/link").setTarget("foo");
+    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);
+
+    // Doesn't check for dangling links, hence download succeeds.
+    service.downloadOutputs(action, result);
+
+    Path path = execRoot.getRelative("outputs/a/b/link");
+    assertThat(path.isSymbolicLink()).isTrue();
+    assertThat(path.readSymbolicLink()).isEqualTo(PathFragment.create("foo"));
+    assertThat(context.isLockOutputFilesCalled()).isTrue();
+  }
+
   @Test
   public void downloadOutputs_relativeSymlinkInDirectory_success() throws Exception {
     Tree tree =
@@ -2014,6 +2062,12 @@ private Spawn newSpawnFromResult(
       outputs.add(output);
     }
 
+    for (OutputSymlink symlink : result.getOutputSymlinks()) {
+      Path path = remotePathResolver.outputPathToLocalPath(symlink.getPath());
+      Artifact output = ActionsTestUtil.createArtifact(artifactRoot, path);
+      outputs.add(output);
+    }
+
     return newSpawn(executionInfo, outputs.build());
   }