Automated rollback of commit 4465dae23de989f1452e93d0a88ac2a289103dd9.

*** Reason for rollback ***

The no-cache tag is not respected (see b/77857812) and thus this breaks remote caching for all projects with symlink outputs.

*** Original change description ***

Only allow regular files and directories spawn outputs to be uploaded to a remote cache.

The remote cache protocol only knows about regular files and
directories. Currently, during action output upload, symlinks are
resolved into regular files. This means cached "executions" of an
action may have different output file types than the original
execution, which can be a footgun. This CL bans symlinks from cachable
spawn outputs and fixes http...

***

PiperOrigin-RevId: 193338629

1 files changed, 8 insertions, 20 deletions

diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
index 980822c5f2..098b4adf4b 100644
--- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
+++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
@@ -24,7 +24,6 @@ import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
-import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.remote.CacheNotFoundException;
 import com.google.devtools.build.lib.remote.ExecutionStatusException;
 import com.google.devtools.build.lib.remote.SimpleBlobStoreActionCache;
@@ -253,7 +252,6 @@ final class ExecutionServer extends ExecutionImplBase {
         (cmdResult != null && cmdResult.getTerminationStatus().timedOut())
             || wasTimeout(timeoutMillis, System.currentTimeMillis() - startTime);
     final int exitCode;
-    Status errStatus = null;
     ExecuteResponse.Builder resp = ExecuteResponse.newBuilder();
     if (wasTimeout) {
       final String errMessage =
@@ -261,11 +259,11 @@ final class ExecutionServer extends ExecutionImplBase {
               "Command:\n%s\nexceeded deadline of %f seconds.",
               Arrays.toString(command.getArgumentsList().toArray()), timeoutMillis / 1000.0);
       logger.warning(errMessage);
-      errStatus =
+      resp.setStatus(
           Status.newBuilder()
               .setCode(Code.DEADLINE_EXCEEDED.getNumber())
               .setMessage(errMessage)
-              .build();
+              .build());
       exitCode = LOCAL_EXEC_ERROR;
     } else if (cmdResult == null) {
       exitCode = LOCAL_EXEC_ERROR;
@@ -274,29 +272,19 @@ final class ExecutionServer extends ExecutionImplBase {
     }
 
     ActionResult.Builder result = ActionResult.newBuilder();
-    try {
-      cache.upload(result, execRoot, outputs);
-    } catch (ExecException e) {
-      if (errStatus == null) {
-        errStatus =
-            Status.newBuilder()
-                .setCode(Code.FAILED_PRECONDITION.getNumber())
-                .setMessage(e.getMessage())
-                .build();
-      }
-    }
+    cache.upload(result, execRoot, outputs);
     byte[] stdout = cmdResult.getStdout();
     byte[] stderr = cmdResult.getStderr();
     cache.uploadOutErr(result, stdout, stderr);
     ActionResult finalResult = result.setExitCode(exitCode).build();
-    resp.setResult(finalResult);
-    if (errStatus != null) {
-      resp.setStatus(errStatus);
-      throw new ExecutionStatusException(errStatus, resp.build());
-    } else if (exitCode == 0 && !action.getDoNotCache()) {
+    if (exitCode == 0 && !action.getDoNotCache()) {
       ActionKey actionKey = digestUtil.computeActionKey(action);
       cache.setCachedActionResult(actionKey, finalResult);
     }
+    resp.setResult(finalResult);
+    if (wasTimeout) {
+      throw new ExecutionStatusException(resp.getStatus(), resp.build());
+    }
     return finalResult;
   }