Flip use output paths

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

@@ -23,12 +23,21 @@ public class ApiVersion implements Comparable<ApiVersion> {
   public final int patch;
   public final String prerelease;
 
+  // The version of the Remote Execution API that Bazel supports initially.
+  public static final ApiVersion twoPointZero =
+      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
+  // The version of the Remote Execution API that starts supporting the
+  // Command.output_paths and ActionResult.output_symlinks fields.
+  public static final ApiVersion twoPointOne =
+      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(1).build());
+  // The latest version of the Remote Execution API that Bazel is compatible with.
+  public static final ApiVersion twoPointTwo =
+      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(2).build());
+
   // The current lowest/highest versions (inclusive) of the Remote Execution API that Bazel
   // supports. These fields will need to be updated together with all version changes.
-  public static final ApiVersion low =
-      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
-  public static final ApiVersion high =
-      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
+  public static final ApiVersion low = twoPointZero;
+  public static final ApiVersion high = twoPointTwo;
 
   public ApiVersion(int major, int minor, int patch, String prerelease) {
     this.major = major;

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

@@ -29,6 +29,7 @@
 import build.bazel.remote.execution.v2.FindMissingBlobsResponse;
 import build.bazel.remote.execution.v2.GetActionResultRequest;
 import build.bazel.remote.execution.v2.RequestMetadata;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import build.bazel.remote.execution.v2.UpdateActionResultRequest;
 import com.google.bytestream.ByteStreamGrpc;
 import com.google.bytestream.ByteStreamGrpc.ByteStreamStub;
@@ -263,6 +264,11 @@ public CacheCapabilities getCacheCapabilities() throws IOException {
     return channel.getServerCapabilities().getCacheCapabilities();
   }
 
+  @Override
+  public ServerCapabilities getServerCapabilities() throws IOException {
+    return channel.getServerCapabilities();
+  }
+
   @Override
   public ListenableFuture<String> getAuthority() {
     return channel.withChannelFuture(ch -> Futures.immediateFuture(ch.authority()));

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

@@ -26,6 +26,7 @@
 import build.bazel.remote.execution.v2.ActionResult;
 import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.flogger.GoogleLogger;
@@ -124,6 +125,13 @@ public ListenableFuture<String> getRemoteAuthority() {
     return remoteCacheClient.getAuthority();
   }
 
+  public ServerCapabilities getServerCapabilities() throws IOException {
+    if (remoteCacheClient == null) {
+      return ServerCapabilities.getDefaultInstance();
+    }
+    return remoteCacheClient.getServerCapabilities();
+  }
+
   /**
    * Class to keep track of which cache (disk or remote) a given [cached] ActionResult comes from.
    */

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

@@ -156,6 +156,7 @@
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
 /**
@@ -196,6 +197,8 @@ public class RemoteExecutionService {
   @Nullable private final Scrubber scrubber;
   private final Set<Digest> knownMissingCasDigests;
 
+  private Boolean useOutputPaths;
+
   public RemoteExecutionService(
       Executor executor,
       Reporter reporter,
@@ -242,32 +245,40 @@ public RemoteExecutionService(
   }
 
   private Command buildCommand(
+      boolean useOutputPaths,
       Collection<? extends ActionInput> outputs,
       List<String> arguments,
       ImmutableMap<String, String> env,
       @Nullable Platform platform,
       RemotePathResolver remotePathResolver,
       @Nullable SpawnScrubber spawnScrubber) {
     Command.Builder command = Command.newBuilder();
-    ArrayList<String> outputFiles = new ArrayList<>();
-    ArrayList<String> outputDirectories = new ArrayList<>();
-    ArrayList<String> outputPaths = new ArrayList<>();
-    for (ActionInput output : outputs) {
-      String pathString =
-          reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
-      if (output.isDirectory()) {
-        outputDirectories.add(pathString);
-      } else {
-        outputFiles.add(pathString);
+    if (useOutputPaths) {
+      var outputPaths = new ArrayList<String>();
+      for (ActionInput output : outputs) {
+        String pathString =
+            reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
+        outputPaths.add(pathString);
+      }
+      Collections.sort(outputPaths);
+      command.addAllOutputPaths(outputPaths);
+    } else {
+      var outputFiles = new ArrayList<String>();
+      var outputDirectories = new ArrayList<String>();
+      for (ActionInput output : outputs) {
+        String pathString =
+            reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
+        if (output.isDirectory()) {
+          outputDirectories.add(pathString);
+        } else {
+          outputFiles.add(pathString);
+        }
       }
-      outputPaths.add(pathString);
+      Collections.sort(outputFiles);
+      Collections.sort(outputDirectories);
+      command.addAllOutputFiles(outputFiles);
+      command.addAllOutputDirectories(outputDirectories);
     }
-    Collections.sort(outputFiles);
-    Collections.sort(outputDirectories);
-    Collections.sort(outputPaths);
-    command.addAllOutputFiles(outputFiles);
-    command.addAllOutputDirectories(outputDirectories);
-    command.addAllOutputPaths(outputPaths);
 
     if (platform != null) {
       command.setPlatform(platform);
@@ -542,6 +553,63 @@ private void maybeReleaseRemoteActionBuildingSemaphore() {
     remoteActionBuildingSemaphore.release();
   }
 
+  private synchronized void initUseOutputPaths() {
+    // If this has already been initialized, return
+    if (this.useOutputPaths != null) {
+      return;
+    }
+    this.useOutputPaths = remoteOptions.useOutputPaths;
+    try {
+      // If both Remote Executor and Remote Cache are configured,
+      // use the highest version that is supported by both server to compare.
+      if (remoteExecutor != null && remoteCache != null) {
+        var executorSupportStatus =
+            ClientApiVersion.current.checkServerSupportedVersions(
+                remoteExecutor.getServerCapabilities());
+        var cacheSupportStatus =
+            ClientApiVersion.current.checkServerSupportedVersions(
+                remoteCache.getServerCapabilities());
+
+        ApiVersion highestSupportedApiVersion = ApiVersion.low;
+        if (executorSupportStatus.isSupported() && cacheSupportStatus.isSupported()) {
+          var executorHighestVersion = executorSupportStatus.getHighestSupportedVersion();
+          var cacheHighestVersion = cacheSupportStatus.getHighestSupportedVersion();
+
+          var commonServerVersion =
+              executorHighestVersion.compareTo(cacheHighestVersion) <= 0
+                  ? executorHighestVersion
+                  : cacheHighestVersion;
+          this.useOutputPaths = commonServerVersion.compareTo(ApiVersion.twoPointOne) >= 0;
+          return;
+        }
+      }
+
+      if (remoteExecutor != null) {
+        var capabilities = remoteExecutor.getServerCapabilities();
+        if (capabilities != null) {
+          var supportStatus = ClientApiVersion.current.checkServerSupportedVersions(capabilities);
+          if (supportStatus.isSupported()) {
+            this.useOutputPaths =
+                supportStatus.getHighestSupportedVersion().compareTo(ApiVersion.twoPointOne) >= 0;
+            return;
+          }
+        }
+      }
+      if (remoteCache != null) {
+        var capabilities = remoteCache.getServerCapabilities();
+        if (capabilities != null) {
+          var supportStatus = ClientApiVersion.current.checkServerSupportedVersions(capabilities);
+          if (supportStatus.isSupported()) {
+            this.useOutputPaths =
+                supportStatus.getHighestSupportedVersion().compareTo(ApiVersion.twoPointOne) >= 0;
+          }
+        }
+      }
+    } catch (IOException e) {
+      // If we can't determine the server capabilities, use the default value.
+    }
+  }
+
   /** Creates a new {@link RemoteAction} instance from spawn. */
   public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context)
       throws IOException, ExecException, ForbiddenActionInputException, InterruptedException {
@@ -568,8 +636,12 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context
         platform = PlatformUtils.getPlatformProto(spawn, remoteOptions);
       }
 
+      if (this.useOutputPaths == null) {
+        initUseOutputPaths();
+      }
       Command command =
           buildCommand(
+              this.useOutputPaths,
               spawn.getOutputFiles(),
               spawn.getArguments(),
               spawn.getEnvironment(),
@@ -1509,8 +1581,18 @@ public SpawnResult waitForAndReuseOutputs(RemoteAction action, LocalExecution pr
     Map<Path, Path> realToTmpPath = new HashMap<>();
     ByteString inMemoryOutputContent = null;
     String inMemoryOutputPath = null;
+    if (this.useOutputPaths == null) {
+      initUseOutputPaths();
+    }
+    var outputPathsList =
+        this.useOutputPaths
+            ? action.getCommand().getOutputPathsList()
+            : Stream.concat(
+                    action.getCommand().getOutputFilesList().stream(),
+                    action.getCommand().getOutputDirectoriesList().stream())
+                .toList();
     try {
-      for (String output : action.getCommand().getOutputPathsList()) {
+      for (String output : outputPathsList) {
         String reencodedOutput = reencodeExternalToInternal(output);
         Path sourcePath =
             previousExecution.action.getRemotePathResolver().outputPathToLocalPath(reencodedOutput);

src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java

@@ -18,6 +18,7 @@
 import build.bazel.remote.execution.v2.ActionResult;
 import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.devtools.build.lib.vfs.Path;
@@ -33,6 +34,8 @@
 public interface RemoteCacheClient extends MissingDigestsFinder {
   CacheCapabilities getCacheCapabilities() throws IOException;
 
+  ServerCapabilities getServerCapabilities() throws IOException;
+
   ListenableFuture<String> getAuthority();
 
   /**

src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java

@@ -17,6 +17,7 @@
 import build.bazel.remote.execution.v2.ActionResult;
 import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy;
 import com.google.auth.Credentials;
 import com.google.common.collect.ImmutableList;
@@ -610,6 +611,11 @@ public CacheCapabilities getCacheCapabilities() {
         .build();
   }
 
+  @Override
+  public ServerCapabilities getServerCapabilities() {
+    return ServerCapabilities.newBuilder().setCacheCapabilities(getCacheCapabilities()).build();
+  }
+
   @Override
   public ListenableFuture<String> getAuthority() {
     return Futures.immediateFuture("");

src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java

@@ -413,6 +413,16 @@ public RemoteBuildEventUploadModeConverter() {
               + " --experimental_remote_cache_compression_threshold.")
   public boolean cacheCompression;
 
+  @Option(
+      name = "incompatible_remote_use_output_paths",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.REMOTE,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      help =
+          "If enabled, check if the remote server(s) support remote execution API v2.1 or newer."
+              + "If yes, use the newer Command.output_paths field. Default is true.")
+  public boolean useOutputPaths;
+
   @Option(
       name = "experimental_remote_cache_compression_threshold",
       // Based on discussions in #18997, `~100` is the break even point where the compression