Switching Bazel to use the new remote execution API: https://docs.google.com/document/d/1AaGk7fOPByEvpAbqeXIyE8HX_A3_axxNnvroblTZ_6s/edit

Also refactored away the various *Interface* files, no need since unit testing can be done with mocking the appropriate gRPC Impl classes directly (see tests). This also fixes the RemoteSpawnRunner, which should use different objects for remote caching and remote execution, the same way RemoteSpawnStrategy does.

RELNOTES: n/a
PiperOrigin-RevId: 158473700

1 files changed, 58 insertions, 44 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunner.java
index 87da7c2405..62605e1c2d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunner.java
@@ -13,28 +13,27 @@
 // limitations under the License.
 package com.google.devtools.build.lib.remote;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Spawn;
+import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.exec.SpawnResult;
 import com.google.devtools.build.lib.exec.SpawnResult.Status;
 import com.google.devtools.build.lib.exec.SpawnRunner;
-import com.google.devtools.build.lib.remote.ContentDigests.ActionKey;
-import com.google.devtools.build.lib.remote.RemoteProtocol.Action;
-import com.google.devtools.build.lib.remote.RemoteProtocol.ActionResult;
-import com.google.devtools.build.lib.remote.RemoteProtocol.Command;
-import com.google.devtools.build.lib.remote.RemoteProtocol.ContentDigest;
-import com.google.devtools.build.lib.remote.RemoteProtocol.Platform;
+import com.google.devtools.build.lib.remote.Digests.ActionKey;
 import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode;
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.remoteexecution.v1test.Action;
+import com.google.devtools.remoteexecution.v1test.ActionResult;
+import com.google.devtools.remoteexecution.v1test.Command;
+import com.google.devtools.remoteexecution.v1test.Digest;
+import com.google.devtools.remoteexecution.v1test.Platform;
+import com.google.protobuf.Duration;
 import com.google.protobuf.TextFormat;
 import com.google.protobuf.TextFormat.ParseException;
 import io.grpc.StatusRuntimeException;
@@ -56,14 +55,11 @@ final class CachedLocalSpawnRunner implements SpawnRunner {
   // TODO(olaola): This will be set on a per-action basis instead.
   private final Platform platform;
 
-  private final RemoteActionCache actionCache;
+  private final RemoteActionCache remoteCache;
   private final SpawnRunner delegate;
 
   CachedLocalSpawnRunner(
-      Path execRoot,
-      RemoteOptions options,
-      RemoteActionCache actionCache,
-      SpawnRunner delegate) {
+      Path execRoot, RemoteOptions options, RemoteActionCache remoteCache, SpawnRunner delegate) {
     this.execRoot = execRoot;
     this.options = options;
     if (options.experimentalRemotePlatformOverride != null) {
@@ -77,15 +73,13 @@ final class CachedLocalSpawnRunner implements SpawnRunner {
     } else {
       platform = null;
     }
-    this.actionCache = actionCache;
+    this.remoteCache = remoteCache;
     this.delegate = delegate;
   }
 
   @Override
-  public SpawnResult exec(
-      Spawn spawn,
-      SpawnExecutionPolicy policy)
-          throws InterruptedException, IOException, ExecException {
+  public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy)
+      throws InterruptedException, IOException, ExecException {
     ActionKey actionKey = null;
     String mnemonic = spawn.getMnemonic();
 
@@ -100,24 +94,26 @@ final class CachedLocalSpawnRunner implements SpawnRunner {
       Action action =
           buildAction(
               spawn.getOutputFiles(),
-              ContentDigests.computeDigest(command),
-              repository.getMerkleDigest(inputRoot));
+              Digests.computeDigest(command),
+              repository.getMerkleDigest(inputRoot),
+              // TODO(olaola): set sensible local and remote timeouts.
+              Spawns.getTimeoutSeconds(spawn, 120));
 
       // Look up action cache, and reuse the action output if it is found.
-      actionKey = ContentDigests.computeActionKey(action);
+      actionKey = Digests.computeActionKey(action);
       ActionResult result =
-          this.options.remoteAcceptCached ? actionCache.getCachedActionResult(actionKey) : null;
+          this.options.remoteAcceptCached ? remoteCache.getCachedActionResult(actionKey) : null;
       if (result != null) {
         // We don't cache failed actions, so we know the outputs exist.
         // For now, download all outputs locally; in the future, we can reuse the digests to
         // just update the TreeNodeRepository and continue the build.
         try {
           // TODO(ulfjack): Download stdout, stderr, and the output files in a single call.
-          actionCache.downloadAllResults(result, execRoot);
+          remoteCache.downloadAllResults(result, execRoot);
           passRemoteOutErr(result, policy.getFileOutErr());
           return new SpawnResult.Builder()
               .setStatus(Status.SUCCESS)
-              .setExitCode(result.getReturnCode())
+              .setExitCode(result.getExitCode())
               .build();
         } catch (CacheNotFoundException e) {
           // TODO(ulfjack): Track down who throws this exception in what cases and double-check that
@@ -138,44 +134,62 @@ final class CachedLocalSpawnRunner implements SpawnRunner {
   }
 
   private Action buildAction(
-      Collection<? extends ActionInput> outputs, ContentDigest command, ContentDigest inputRoot) {
+      Collection<? extends ActionInput> outputs,
+      Digest command,
+      Digest inputRoot,
+      long timeoutSeconds) {
     Action.Builder action = Action.newBuilder();
     action.setCommandDigest(command);
     action.setInputRootDigest(inputRoot);
     // Somewhat ugly: we rely on the stable order of outputs here for remote action caching.
     for (ActionInput output : outputs) {
-      action.addOutputPath(output.getExecPathString());
+      // TODO: output directories should be handled here, when they are supported.
+      action.addOutputFiles(output.getExecPathString());
     }
     if (platform != null) {
       action.setPlatform(platform);
     }
+    action.setTimeout(Duration.newBuilder().setSeconds(timeoutSeconds));
     return action.build();
   }
 
-  private static Command buildCommand(
-      List<String> arguments, ImmutableMap<String, String> environment) {
+  private Command buildCommand(List<String> arguments, ImmutableMap<String, String> environment) {
     Command.Builder command = Command.newBuilder();
-    command.addAllArgv(arguments);
+    command.addAllArguments(arguments);
     // Sorting the environment pairs by variable name.
     TreeSet<String> variables = new TreeSet<>(environment.keySet());
     for (String var : variables) {
-      command.addEnvironmentBuilder().setVariable(var).setValue(environment.get(var));
+      command.addEnvironmentVariablesBuilder().setName(var).setValue(environment.get(var));
     }
     return command.build();
   }
 
-  private void passRemoteOutErr(
-      ActionResult result, FileOutErr outErr)
-          throws CacheNotFoundException {
-    ImmutableList<byte[]> streams =
-        actionCache.downloadBlobs(
-            ImmutableList.of(result.getStdoutDigest(), result.getStderrDigest()));
-    outErr.printOut(new String(streams.get(0), UTF_8));
-    outErr.printErr(new String(streams.get(1), UTF_8));
+  private void passRemoteOutErr(ActionResult result, FileOutErr outErr) throws IOException {
+    try {
+      if (!result.getStdoutRaw().isEmpty()) {
+        result.getStdoutRaw().writeTo(outErr.getOutputStream());
+        outErr.getOutputStream().flush();
+      } else if (result.hasStdoutDigest()) {
+        byte[] stdoutBytes = remoteCache.downloadBlob(result.getStdoutDigest());
+        outErr.getOutputStream().write(stdoutBytes);
+        outErr.getOutputStream().flush();
+      }
+      if (!result.getStderrRaw().isEmpty()) {
+        result.getStderrRaw().writeTo(outErr.getErrorStream());
+        outErr.getErrorStream().flush();
+      } else if (result.hasStderrDigest()) {
+        byte[] stderrBytes = remoteCache.downloadBlob(result.getStderrDigest());
+        outErr.getErrorStream().write(stderrBytes);
+        outErr.getErrorStream().flush();
+      }
+    } catch (CacheNotFoundException e) {
+      outErr.printOutLn("Failed to fetch remote stdout/err due to cache miss.");
+      outErr.getOutputStream().flush();
+    }
   }
 
   private void writeCacheEntry(Spawn spawn, FileOutErr outErr, ActionKey actionKey)
-          throws IOException, InterruptedException {
+      throws IOException, InterruptedException {
     ArrayList<Path> outputFiles = new ArrayList<>();
     for (ActionInput output : spawn.getOutputFiles()) {
       Path outputPath = execRoot.getRelative(output.getExecPathString());
@@ -186,11 +200,11 @@ final class CachedLocalSpawnRunner implements SpawnRunner {
       }
     }
     ActionResult.Builder result = ActionResult.newBuilder();
-    actionCache.uploadAllResults(execRoot, outputFiles, result);
-    ContentDigest stderr = actionCache.uploadFileContents(outErr.getErrorPath());
-    ContentDigest stdout = actionCache.uploadFileContents(outErr.getOutputPath());
+    remoteCache.uploadAllResults(execRoot, outputFiles, result);
+    Digest stderr = remoteCache.uploadFileContents(outErr.getErrorPath());
+    Digest stdout = remoteCache.uploadFileContents(outErr.getOutputPath());
     result.setStderrDigest(stderr);
     result.setStdoutDigest(stdout);
-    actionCache.setCachedActionResult(actionKey, result.build());
+    remoteCache.setCachedActionResult(actionKey, result.build());
   }
 }