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 https://github.com/bazelbuild/bazel/issues/4840.

The interface of SpawnCache.CacheHandle.store is refactored:
1. The outputs parameter is removed, since that can be retrieved from the underlying Spawn.
2. It can now throw ExecException in order to fail actions.

Closes #4902.

Change-Id: I0d1d94d48779b970bb5d0840c66a14c189ab0091
PiperOrigin-RevId: 190608852

1 files changed, 13 insertions, 16 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index fb9724bac9..bb858a924a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.remote;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Throwables;
+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.ActionInputFileCache;
@@ -442,12 +443,12 @@ class RemoteSpawnRunner implements SpawnRunner {
         return result;
       }
     }
-    List<Path> outputFiles = listExistingOutputFiles(execRoot, spawn);
+    boolean uploadAction =
+        Spawns.mayBeCached(spawn)
+            && Status.SUCCESS.equals(result.status())
+            && result.exitCode() == 0;
+    Collection<Path> outputFiles = resolveActionInputs(execRoot, spawn.getOutputFiles());
     try {
-      boolean uploadAction =
-          Spawns.mayBeCached(spawn)
-              && Status.SUCCESS.equals(result.status())
-              && result.exitCode() == 0;
       remoteCache.upload(actionKey, execRoot, outputFiles, policy.getFileOutErr(), uploadAction);
     } catch (IOException e) {
       if (verboseFailures) {
@@ -471,16 +472,12 @@ class RemoteSpawnRunner implements SpawnRunner {
     }
   }
 
-  static List<Path> listExistingOutputFiles(Path execRoot, Spawn spawn) {
-    ArrayList<Path> outputFiles = new ArrayList<>();
-    for (ActionInput output : spawn.getOutputFiles()) {
-      Path outputPath = execRoot.getRelative(output.getExecPathString());
-      // TODO(ulfjack): Store the actual list of output files in SpawnResult and use that instead
-      // of statting the files here again.
-      if (outputPath.exists()) {
-        outputFiles.add(outputPath);
-      }
-    }
-    return outputFiles;
+  /** Resolve a collection of {@link com.google.build.lib.actions.ActionInput}s to {@link Path}s. */
+  static Collection<Path> resolveActionInputs(
+      Path execRoot, Collection<? extends ActionInput> actionInputs) {
+    return actionInputs
+        .stream()
+        .map((inp) -> execRoot.getRelative(inp.getExecPath()))
+        .collect(ImmutableList.toImmutableList());
   }
 }