Pass ActionFS paths through to action-level FileOutErr for the Action…

…'s stdout/stderr. RELNOTES: None PiperOrigin-RevId: 200459354
bazelbuild · Jun 13, 2018 · fc83d75 · fc83d75
1 parent adb3587
commit fc83d75
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 18 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
@@ -84,7 +84,7 @@ private ActionExecutionContext(
     this.artifactExpander = artifactExpander;
     this.env = env;
     this.actionFileSystem = actionFileSystem;
-    this.pathResolver = createPathResolver(actionFileSystem,
+    this.pathResolver = ArtifactPathResolver.createPathResolver(actionFileSystem,
         // executor is only ever null in testing.
         executor == null ? null : executor.getExecRoot());
   }
@@ -180,16 +180,6 @@ public Root getRoot(Artifact artifact) {
     return pathResolver.transformRoot(artifact.getRoot().getRoot());
   }
 
-  private static ArtifactPathResolver createPathResolver(FileSystem actionFileSystem,
-      Path execRoot) {
-    if (actionFileSystem == null) {
-      return ArtifactPathResolver.forExecRoot(execRoot);
-    } else {
-      return ArtifactPathResolver.withTransformedFileSystem(
-          actionFileSystem.getPath(execRoot.asFragment()));
-    }
-  }
-
   public ArtifactPathResolver getPathResolver() {
     return pathResolver;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLogBufferPathGenerator.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLogBufferPathGenerator.java
@@ -15,7 +15,6 @@
 
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.Path;
-
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
@@ -34,9 +33,10 @@ public ActionLogBufferPathGenerator(Path actionOutputRoot) {
   /**
    * Generates a unique filename for an action to store its output.
    */
-  public FileOutErr generate() {
+  public FileOutErr generate(ArtifactPathResolver resolver) {
     int actionId = actionCounter.incrementAndGet();
-    return new FileOutErr(actionOutputRoot.getRelative("stdout-" + actionId),
-                          actionOutputRoot.getRelative("stderr-" + actionId));
+    return new FileOutErr(
+        resolver.convertPath(actionOutputRoot.getRelative("stdout-" + actionId)),
+        resolver.convertPath(actionOutputRoot.getRelative("stderr-" + actionId)));
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactPathResolver.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactPathResolver.java
@@ -17,12 +17,29 @@
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.Root;
+import javax.annotation.Nullable;
 
 /**
  * An indirection layer on Path resolution of {@link Artifact} and {@link Root}.
+ *
+ * Serves as converter interface primarily for switching the {@link FileSystem} underyling the
+ * values.
  */
 public interface ArtifactPathResolver {
+
+  /**
+   * @return a resolved Path corresponding to the given actionInput.
+   */
   Path toPath(ActionInput actionInput);
+
+  /**
+   * @return a resolved Path corresponding to the given path.
+   */
+  Path convertPath(Path path);
+
+  /**
+   * @return a resolved Rooth corresponding to the given Root.
+   */
   Root transformRoot(Root root);
 
   ArtifactPathResolver IDENTITY = new IdentityResolver(null);
@@ -35,6 +52,16 @@ static ArtifactPathResolver withTransformedFileSystem(Path execRoot) {
     return new TransformResolver(execRoot);
   }
 
+  static ArtifactPathResolver createPathResolver(@Nullable FileSystem fileSystem,
+      Path execRoot) {
+    if (fileSystem == null) {
+      return forExecRoot(execRoot);
+    } else {
+      return withTransformedFileSystem(
+          fileSystem.getPath(execRoot.asFragment()));
+    }
+  }
+
   /**
    * Path resolution that uses an Artifact's path directly, or looks up the input execPath relative
    * to the given execRoot.
@@ -58,6 +85,11 @@ public Path toPath(ActionInput actionInput) {
     public Root transformRoot(Root root) {
       return Preconditions.checkNotNull(root);
     }
+
+    @Override
+    public Path convertPath(Path path) {
+      return path;
+    }
   };
 
   /**
@@ -84,5 +116,10 @@ public Path toPath(ActionInput input) {
     public Root transformRoot(Root root) {
       return Root.toFileSystem(Preconditions.checkNotNull(root), fileSystem);
     }
+
+    @Override
+    public Path convertPath(Path path) {
+      return fileSystem.getPath(path.asFragment());
+    }
   }
 }
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
@@ -51,6 +51,7 @@
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
 import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper;
+import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException;
 import com.google.devtools.build.lib.actions.CachedActionEvent;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
@@ -520,7 +521,8 @@ public ActionExecutionContext getContext(
       Map<Artifact, Collection<Artifact>> expandedInputs,
       ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings,
       @Nullable ActionFileSystem actionFileSystem) {
-    FileOutErr fileOutErr = actionLogBufferPathGenerator.generate();
+    FileOutErr fileOutErr = actionLogBufferPathGenerator.generate(
+        ArtifactPathResolver.createPathResolver(actionFileSystem, executorEngine.getExecRoot()));
     return new ActionExecutionContext(
         executorEngine,
         createFileCache(graphFileCache, actionFileSystem),
@@ -655,7 +657,8 @@ Iterable<Artifact> discoverInputs(
             actionInputPrefetcher,
             actionKeyContext,
             metadataHandler,
-            actionLogBufferPathGenerator.generate(),
+            actionLogBufferPathGenerator.generate(ArtifactPathResolver.createPathResolver(
+                actionFileSystem, executorEngine.getExecRoot())),
             clientEnv,
             env,
             actionFileSystem);

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -39,6 +39,7 @@
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
 import com.google.devtools.build.lib.actions.ArtifactOwner;
+import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.CommandAction;
 import com.google.devtools.build.lib.actions.CommandLine;
@@ -2121,7 +2122,7 @@ public ActionExecutionContext build() {
           /*actionInputPrefetcher=*/ null,
           actionKeyContext,
           /*metadataHandler=*/ null,
-          actionLogBufferPathGenerator.generate(),
+          actionLogBufferPathGenerator.generate(ArtifactPathResolver.IDENTITY),
           clientEnv,
           ImmutableMap.of(),
           artifactExpander,