Allow for Path and Root resolution when extracting action metadata.

RELNOTES: None
PiperOrigin-RevId: 199572597

6 files changed, 99 insertions, 15 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 64e9b29681..fa22da4584 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.rules.cpp.IncludeScannable;
 import com.google.devtools.build.lib.skyframe.InputArtifactData.MutableInputArtifactData;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
+import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -385,7 +386,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
     }
     // This may be recreated if we discover inputs.
     ActionMetadataHandler metadataHandler = new ActionMetadataHandler(state.inputArtifactData,
-        action.getOutputs(), tsgm.get());
+        action.getOutputs(), tsgm.get(), pathResolver(state.actionFileSystem));
     long actionStartTime = BlazeClock.nanoTime();
     // We only need to check the action cache if we haven't done it on a previous run.
     if (!state.hasCheckedActionCache()) {
@@ -462,7 +463,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
             new PerActionFileCache(state.inputArtifactData, /*missingArtifactsAllowed=*/ false);
       }
       metadataHandler =
-          new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get());
+          new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get(),
+              pathResolver(state.actionFileSystem));
       // Set the MetadataHandler to accept output information.
       metadataHandler.discardOutputMetadata();
     }
@@ -535,7 +537,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
         // markOmitted is only called for remote execution, and this code only gets executed for
         // local execution.
         metadataHandler =
-            new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get());
+            new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get(),
+                pathResolver(state.actionFileSystem));
       }
     }
     Preconditions.checkState(!env.valuesMissing(), action);
@@ -544,6 +547,23 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
     return state.value;
   }
 
+  private ArtifactPathResolver pathResolver(ActionFileSystem actionFileSystem) {
+    if (actionFileSystem != null) {
+      return new ArtifactPathResolver() {
+        @Override
+        public Path toPath(Artifact artifact) {
+          return actionFileSystem.getPath(artifact.getPath().getPathString());
+        }
+
+        @Override
+        public Root transformRoot(Root root) {
+          return Root.toFileSystem(root, actionFileSystem);
+        }
+      };
+    }
+    return ArtifactPathResolver.IDENTITY;
+  }
+
   private static final Function<Artifact, SkyKey> TO_NONMANDATORY_SKYKEY =
       new Function<Artifact, SkyKey>() {
         @Nullable
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java
index 345c4c8968..cd99db964f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java
@@ -202,8 +202,6 @@ final class ActionFileSystem extends FileSystem implements ActionInputFileCache,
 
   @Override
   protected long getFileSize(Path path, boolean followSymlinks) throws IOException {
-    Preconditions.checkArgument(
-        followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path);
     return getMetadataOrThrowFileNotFound(path).getSize();
   }
 
@@ -214,8 +212,6 @@ final class ActionFileSystem extends FileSystem implements ActionInputFileCache,
 
   @Override
   protected long getLastModifiedTime(Path path, boolean followSymlinks) throws IOException {
-    Preconditions.checkArgument(
-        followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path);
     return getMetadataOrThrowFileNotFound(path).getModifiedTime();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index f3f7e09049..b68930e492 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -120,6 +120,7 @@ public class ActionMetadataHandler implements MetadataHandler {
    * Use {@link #getTimestampGranularityMonitor(Artifact)} to fetch this member.
    */
   private final TimestampGranularityMonitor tsgm;
+  private final ArtifactPathResolver artifactPathResolver;
 
   /**
    * Whether the action is being executed or not; this flag is set to true in
@@ -131,10 +132,12 @@ public class ActionMetadataHandler implements MetadataHandler {
   public ActionMetadataHandler(
       InputArtifactData inputArtifactData,
       Iterable<Artifact> outputs,
-      TimestampGranularityMonitor tsgm) {
+      TimestampGranularityMonitor tsgm,
+      ArtifactPathResolver artifactPathResolver)  {
     this.inputArtifactData = Preconditions.checkNotNull(inputArtifactData);
     this.outputs = ImmutableSet.copyOf(outputs);
     this.tsgm = tsgm;
+    this.artifactPathResolver = artifactPathResolver;
   }
 
   /**
@@ -390,7 +393,8 @@ public class ActionMetadataHandler implements MetadataHandler {
       throws IOException {
     Preconditions.checkState(artifact.isTreeArtifact(), artifact);
 
-    if (!artifact.getPath().isDirectory() || artifact.getPath().isSymbolicLink()) {
+    if (!artifactPathResolver.toPath(artifact).isDirectory()
+        || artifactPathResolver.toPath(artifact).isSymbolicLink()) {
       return TreeArtifactValue.MISSING_TREE_ARTIFACT;
     }
 
@@ -580,7 +584,7 @@ public class ActionMetadataHandler implements MetadataHandler {
       setPathReadOnlyAndExecutable(artifact);
     }
 
-    FileValue value = fileValueFromArtifact(artifact, statNoFollow,
+    FileValue value = fileValueFromArtifact(artifact, artifactPathResolver, statNoFollow,
         getTimestampGranularityMonitor(artifact));
     FileValue oldFsValue = outputArtifactData.putIfAbsent(artifact, value);
     checkInconsistentData(artifact, oldFsValue, value);
@@ -591,9 +595,18 @@ public class ActionMetadataHandler implements MetadataHandler {
   static FileValue fileValueFromArtifact(Artifact artifact,
       @Nullable FileStatusWithDigest statNoFollow, @Nullable TimestampGranularityMonitor tsgm)
       throws IOException {
-    Path path = artifact.getPath();
+    return fileValueFromArtifact(artifact, ArtifactPathResolver.IDENTITY, statNoFollow, tsgm);
+  }
+
+  private static FileValue fileValueFromArtifact(Artifact artifact,
+      ArtifactPathResolver artifactPathResolver,
+      @Nullable FileStatusWithDigest statNoFollow, @Nullable TimestampGranularityMonitor tsgm)
+      throws IOException {
+    Path path = artifactPathResolver.toPath(artifact);
     RootedPath rootedPath =
-        RootedPath.toRootedPath(artifact.getRoot().getRoot(), artifact.getRootRelativePath());
+        RootedPath.toRootedPath(
+            artifactPathResolver.transformRoot(artifact.getRoot().getRoot()),
+            artifact.getRootRelativePath());
     if (statNoFollow == null) {
       statNoFollow = FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW));
       if (statNoFollow == null) {
@@ -614,7 +627,8 @@ public class ActionMetadataHandler implements MetadataHandler {
     }
     RootedPath realRootedPath =
         RootedPath.toRootedPathMaybeUnderRoot(
-            realPath, ImmutableList.of(artifact.getRoot().getRoot()));
+            realPath,
+            ImmutableList.of(artifactPathResolver.transformRoot(artifact.getRoot().getRoot())));
     FileStateValue fileStateValue =
         FileStateValue.createWithStatNoFollow(rootedPath, statNoFollow, tsgm);
     // TODO(bazel-team): consider avoiding a 'stat' here when the symlink target hasn't changed
@@ -631,7 +645,7 @@ public class ActionMetadataHandler implements MetadataHandler {
     if (injectedFiles.contains(artifact)) {
       return;
     }
-    Path path = artifact.getPath();
+    Path path = artifactPathResolver.toPath(artifact);
     if (path.isFile(Symlinks.NOFOLLOW)) { // i.e. regular files only.
       // We trust the files created by the execution engine to be non symlinks with expected
       // chmod() settings already applied.
@@ -641,7 +655,7 @@ public class ActionMetadataHandler implements MetadataHandler {
 
   private void setTreeReadOnlyAndExecutable(SpecialArtifact parent, PathFragment subpath)
       throws IOException {
-    Path path = parent.getPath().getRelative(subpath);
+    Path path = artifactPathResolver.toPath(parent).getRelative(subpath);
     if (path.isDirectory()) {
       path.chmod(0555);
       for (Path child : path.getDirectoryEntries()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactPathResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactPathResolver.java
new file mode 100644
index 0000000000..9d0bf7651b
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactPathResolver.java
@@ -0,0 +1,38 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.Root;
+
+/**
+ * An indirection layer on Path resolution of {@link Artifact} and {@link Root}.
+ */
+interface ArtifactPathResolver {
+  Path toPath(Artifact artifact);
+  Root transformRoot(Root root);
+
+  ArtifactPathResolver IDENTITY = new ArtifactPathResolver() {
+    @Override
+    public Path toPath(Artifact artifact) {
+      return artifact.getPath();
+    }
+
+    @Override
+    public Root transformRoot(Root root) {
+      return root;
+    }
+  };
+}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Root.java b/src/main/java/com/google/devtools/build/lib/vfs/Root.java
index c7c78900f8..454e9b26a2 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/Root.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/Root.java
@@ -40,6 +40,12 @@ public abstract class Root implements Comparable<Root>, Serializable {
     return fileSystem.getAbsoluteRoot();
   }
 
+  public static Root toFileSystem(Root root, FileSystem fileSystem) {
+    return root.isAbsolute()
+      ? new AbsoluteRoot(fileSystem)
+      : new PathRoot(fileSystem.getPath(root.asPath().asFragment()));
+  }
+
   /** Returns a path by concatenating the root and the root-relative path. */
   public abstract Path getRelative(PathFragment rootRelativePath);
 
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/RootTest.java b/src/test/java/com/google/devtools/build/lib/vfs/RootTest.java
index dd0f611b1a..640f8c4c91 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/RootTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/RootTest.java
@@ -67,6 +67,16 @@ public class RootTest {
   }
 
   @Test
+  public void testFilesystemTransform() throws Exception {
+    FileSystem fs2 = new InMemoryFileSystem(BlazeClock.instance());
+    Root root = Root.fromPath(fs.getPath("/foo"));
+    Root root2 = Root.toFileSystem(root, fs2);
+    assertThat(root2.asPath().getFileSystem()).isSameAs(fs2);
+    assertThat(root2.asPath().asFragment()).isEqualTo(PathFragment.create("/foo"));
+    assertThat(root.isAbsolute()).isFalse();
+  }
+
+  @Test
   public void testFileSystemAbsoluteRoot() throws Exception {
     Root root = Root.absoluteRoot(fs);
     assertThat(root.asPath()).isNull();