Rolling back change because of subtle race condition when multiple actions insert ActionInputs into SingleBuildFileCache.

--
MOS_MIGRATED_REVID=89032980

4 files changed, 21 insertions, 44 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputFileCache.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputFileCache.java
index 79c8785e92..b45e9cd65a 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputFileCache.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputFileCache.java
@@ -13,9 +13,9 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.protobuf.ByteString;
 
+import java.io.File;
 import java.io.IOException;
 
 import javax.annotation.Nullable;
@@ -70,16 +70,8 @@ public interface ActionInputFileCache {
    * based on files previously seen as inputs.
    *
    * @param digest the digest.
-   * @return an ActionInput corresponding to the given digest.
+   * @return a File path.
    */
   @Nullable
-  ActionInput getInputFromDigest(ByteString digest) throws IOException;
-
-  /**
-   * The absolute path that this input is located at. The usual {@link ActionInput} implementation
-   * is {@link Artifact}, which currently embeds its full path, so implementations should just
-   * return this path if {@code input} is an {@link Artifact}. Otherwise, implementations should
-   * resolve the relative path into an absolute one and return that.
-   */
-  Path getInputPath(ActionInput input);
+  File getFileFromDigest(ByteString digest) throws IOException;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
index ddbe12a14a..8ec1e51583 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
@@ -23,7 +23,6 @@ import com.google.common.collect.Maps;
 import com.google.common.io.BaseEncoding;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputFileCache;
-import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.DigestOfDirectoryException;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.FileSystem;
@@ -100,16 +99,9 @@ public class SingleBuildFileCache implements ActionInputFileCache {
 
   @Nullable
   @Override
-  public ActionInput getInputFromDigest(ByteString digest) {
-    return digestToPath.get(digest);
-  }
-
-  @Override
-  public Path getInputPath(ActionInput input) {
-    if (input instanceof Artifact) {
-      return ((Artifact) input).getPath();
-    }
-    return fs.getPath(fullPath(input));
+  public File getFileFromDigest(ByteString digest) {
+    ActionInput relPath = digestToPath.get(digest);
+    return relPath == null ? null : new File(fullPath(relPath));
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java
index 69fb15ebab..5014c0f5f4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java
@@ -38,6 +38,7 @@ import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.protobuf.ByteString;
 
+import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -71,7 +72,7 @@ public class FileAndMetadataCache implements ActionInputFileCache, MetadataHandl
   /** This should never be read directly. Use {@link #getInputFileArtifactValue} instead. */
   private final Map<Artifact, FileArtifactValue> inputArtifactData;
   private final Map<Artifact, Collection<Artifact>> expandedInputMiddlemen;
-  private final Path execRoot;
+  private final File execRoot;
   private final Map<ByteString, Artifact> reverseMap = new ConcurrentHashMap<>();
   private final ConcurrentMap<Artifact, FileValue> outputArtifactData =
       new ConcurrentHashMap<>();
@@ -87,7 +88,7 @@ public class FileAndMetadataCache implements ActionInputFileCache, MetadataHandl
   private static final Interner<ByteString> BYTE_INTERNER = Interners.newWeakInterner();
 
   public FileAndMetadataCache(Map<Artifact, FileArtifactValue> inputArtifactData,
-      Map<Artifact, Collection<Artifact>> expandedInputMiddlemen, Path execRoot,
+      Map<Artifact, Collection<Artifact>> expandedInputMiddlemen, File execRoot,
       Iterable<Artifact> outputs, @Nullable SkyFunction.Environment env,
       TimestampGranularityMonitor tsgm) {
     this.inputArtifactData = Preconditions.checkNotNull(inputArtifactData);
@@ -409,16 +410,13 @@ public class FileAndMetadataCache implements ActionInputFileCache, MetadataHandl
 
   @Nullable
   @Override
-  public Artifact getInputFromDigest(ByteString digest) throws IOException {
-    return reverseMap.get(digest);
-  }
-
-  @Override
-  public Path getInputPath(ActionInput input) {
-    if (input instanceof Artifact) {
-      return ((Artifact) input).getPath();
+  public File getFileFromDigest(ByteString digest) throws IOException {
+    Artifact artifact = reverseMap.get(digest);
+    if (artifact != null) {
+      String relPath = artifact.getExecPathString();
+      return relPath.startsWith("/") ? new File(relPath) : new File(execRoot, relPath);
     }
-    return execRoot.getRelative(input.getExecPathString());
+    return null;
   }
 
   @Nullable
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index d7d713ba09..7b71fba451 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -71,6 +71,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.protobuf.ByteString;
 
+import java.io.File;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.HashSet;
@@ -454,8 +455,8 @@ public final class SkyframeActionExecutor {
     this.executorEngine = null;
   }
 
-  Path getExecRoot() {
-    return executorEngine.getExecRoot();
+  File getExecRoot() {
+    return executorEngine.getExecRoot().getPathFile();
   }
 
   boolean probeActionExecution(Action action) {
@@ -1160,15 +1161,9 @@ public final class SkyframeActionExecutor {
 
     @Nullable
     @Override
-    public ActionInput getInputFromDigest(ByteString digest) throws IOException {
-      ActionInput file = perActionCache.getInputFromDigest(digest);
-      return file != null ? file : perBuildFileCache.getInputFromDigest(digest);
-    }
-
-    @Override
-    public Path getInputPath(ActionInput input) {
-      // Resolving an input only requires the execRoot, which the per-action cache has.
-      return perActionCache.getInputPath(input);
+    public File getFileFromDigest(ByteString digest) throws IOException {
+      File file = perActionCache.getFileFromDigest(digest);
+      return file != null ? file : perBuildFileCache.getFileFromDigest(digest);
     }
   }
 }