Wire up PathMapper in SpawnInputsExpander

src/main/java/com/google/devtools/build/lib/actions/PathMapper.java

@@ -33,7 +33,15 @@
  * part (e.g. "k8-fastbuild") from exec paths to allow for cross-configuration cache hits.
  */
 public interface PathMapper {
-  /** Returns the exec path with the path mapping applied. */
+  /**
+   * Returns the exec path with the path mapping applied.
+   *
+   * <p>Path mappers may return paths with different roots for two paths that have the same root
+   * (e.g., they may map an artifact at {@code bazel-out/k8-fastbuild/bin/pkg/foo} to {@code
+   * bazel-out/<hash of the file>/bin/pkg/foo}). Paths of artifacts that should share the same
+   * parent directory, such as runfiles or tree artifact files, should thus be derived from the
+   * mapped path of their parent.
+   */
   PathFragment map(PathFragment execPath);
 
   /** Returns the exec path of the input with the path mapping applied. */
@@ -80,6 +88,17 @@ default boolean isNoop() {
     return this == NOOP;
   }
 
+  /**
+   * Returns an opaque object whose equality class should encode all information that goes into the
+   * behavior of the {@link #map(PathFragment)} function of this path mapper. This is used as a key
+   * for in-memory caches.
+   *
+   * <p>The default implementation returns the {@link Class} of the path mapper.
+   */
+  default Object cacheKey() {
+    return this.getClass();
+  }
+
   /** A {@link PathMapper} that doesn't change paths. */
   PathMapper NOOP = execPath -> execPath;
 }

src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java

@@ -33,6 +33,7 @@
 import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
 import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
 import com.google.devtools.build.lib.actions.InputMetadataProvider;
+import com.google.devtools.build.lib.actions.PathMapper;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
@@ -125,6 +126,7 @@ void addRunfilesToInputs(
       RunfilesSupplier runfilesSupplier,
       InputMetadataProvider actionFileCache,
       ArtifactExpander artifactExpander,
+      PathMapper pathMapper,
       PathFragment baseDirectory)
       throws IOException, ForbiddenActionInputException {
     Map<PathFragment, Map<PathFragment, Artifact>> rootsAndMappings =
@@ -145,6 +147,7 @@ void addRunfilesToInputs(
                     ? null
                     : artifactExpander.getArchivedTreeArtifact((SpecialArtifact) localArtifact);
             if (archivedTreeArtifact != null) {
+              // TODO(bazel-team): Add path mapping support for archived tree artifacts.
               addMapping(inputMap, location, localArtifact, baseDirectory);
             } else {
               List<ActionInput> expandedInputs =
@@ -155,7 +158,8 @@ void addRunfilesToInputs(
             for (ActionInput input : expandedInputs) {
               addMapping(
                   inputMap,
-                  location.getRelative(((TreeFileArtifact) input).getParentRelativePath()),
+                  mapForRunfiles(pathMapper, root, location).getRelative(
+                      ((TreeFileArtifact) input).getParentRelativePath()),
                   input,
                   baseDirectory);
               }
@@ -167,15 +171,18 @@ void addRunfilesToInputs(
             } catch (MissingExpansionException e) {
               throw new IllegalStateException(e);
             }
+            // TODO(bazel-team): Add path mapping support for filesets.
             addFilesetManifest(location, localArtifact, filesetLinks, inputMap, baseDirectory);
           } else {
             if (strict) {
               failIfDirectory(actionFileCache, localArtifact);
             }
-            addMapping(inputMap, location, localArtifact, baseDirectory);
+            addMapping(inputMap, mapForRunfiles(pathMapper, root, location), localArtifact,
+                baseDirectory);
           }
         } else {
-          addMapping(inputMap, location, VirtualActionInput.EMPTY_MARKER, baseDirectory);
+          addMapping(inputMap, mapForRunfiles(pathMapper, root, location),
+              VirtualActionInput.EMPTY_MARKER, baseDirectory);
         }
       }
     }
@@ -186,11 +193,12 @@ public Map<PathFragment, ActionInput> addRunfilesToInputs(
       RunfilesSupplier runfilesSupplier,
       InputMetadataProvider actionFileCache,
       ArtifactExpander artifactExpander,
+      PathMapper pathMapper,
       PathFragment baseDirectory)
       throws IOException, ForbiddenActionInputException {
     Map<PathFragment, ActionInput> inputMap = new HashMap<>();
     addRunfilesToInputs(
-        inputMap, runfilesSupplier, actionFileCache, artifactExpander, baseDirectory);
+        inputMap, runfilesSupplier, actionFileCache, artifactExpander, pathMapper, baseDirectory);
     return inputMap;
   }
 
@@ -235,6 +243,7 @@ void addFilesetManifest(
           value == null
               ? VirtualActionInput.EMPTY_MARKER
               : ActionInputHelper.fromPath(execRoot.getRelative(value).asFragment());
+      // TODO(bazel-team): Add path mapping support for filesets.
       addMapping(inputMappings, mapping.getKey(), artifact, baseDirectory);
       }
   }
@@ -243,6 +252,7 @@ private static void addInputs(
       Map<PathFragment, ActionInput> inputMap,
       NestedSet<? extends ActionInput> inputFiles,
       ArtifactExpander artifactExpander,
+      PathMapper pathMapper,
       PathFragment baseDirectory) {
     // Actions that accept TreeArtifacts as inputs generally expect the directory corresponding
     // to the artifact to be created, even if it is empty. We explicitly keep empty TreeArtifacts
@@ -251,7 +261,14 @@ private static void addInputs(
         ActionInputHelper.expandArtifacts(
             inputFiles, artifactExpander, /* keepEmptyTreeArtifacts= */ true);
     for (ActionInput input : inputs) {
-      addMapping(inputMap, input.getExecPath(), input, baseDirectory);
+      if (input instanceof TreeFileArtifact) {
+        addMapping(inputMap,
+            pathMapper.map(((TreeFileArtifact) input).getParent().getExecPath())
+                .getRelative(((TreeFileArtifact) input).getParentRelativePath()), input,
+            baseDirectory);
+      } else {
+        addMapping(inputMap, pathMapper.map(input.getExecPath()), input, baseDirectory);
+      }
     }
   }
 
@@ -273,17 +290,35 @@ public SortedMap<PathFragment, ActionInput> getInputMapping(
       InputMetadataProvider actionInputFileCache)
       throws IOException, ForbiddenActionInputException {
     TreeMap<PathFragment, ActionInput> inputMap = new TreeMap<>();
-    addInputs(inputMap, spawn.getInputFiles(), artifactExpander, baseDirectory);
+    addInputs(inputMap, spawn.getInputFiles(), artifactExpander, spawn.getPathMapper(),
+        baseDirectory);
     addRunfilesToInputs(
         inputMap,
         spawn.getRunfilesSupplier(),
         actionInputFileCache,
         artifactExpander,
+        spawn.getPathMapper(),
         baseDirectory);
     addFilesetManifests(spawn.getFilesetMappings(), inputMap, baseDirectory);
     return inputMap;
   }
 
+  private static PathFragment mapForRunfiles(
+      PathMapper pathMapper, PathFragment runfilesDir, PathFragment execPath) {
+    if (pathMapper.isNoop()) {
+      return execPath;
+    }
+    String runfilesDirName = runfilesDir.getBaseName();
+    Preconditions.checkArgument(runfilesDirName.endsWith(".runfiles"));
+    // Derive the path of the executable, apply the path mapping to it and then rederive the path
+    // of the runfiles dir.
+    PathFragment executable =
+        runfilesDir.replaceName(
+            runfilesDirName.substring(0, runfilesDirName.length() - ".runfiles".length()));
+    return pathMapper.map(executable).replaceName(runfilesDirName)
+        .getRelative(execPath.relativeTo(runfilesDir));
+  }
+
   /** The interface for accessing part of the input hierarchy. */
   public interface InputWalker {
 
@@ -326,19 +361,25 @@ public void walkInputs(
       InputMetadataProvider actionInputFileCache,
       InputVisitor visitor)
       throws IOException, ForbiddenActionInputException {
-    walkNestedSetInputs(baseDirectory, spawn.getInputFiles(), artifactExpander, visitor);
+    walkNestedSetInputs(baseDirectory, spawn.getInputFiles(), artifactExpander,
+        spawn.getPathMapper(), visitor);
 
     RunfilesSupplier runfilesSupplier = spawn.getRunfilesSupplier();
     visitor.visit(
         // Cache key for the sub-mapping containing the runfiles inputs for this spawn.
-        ImmutableList.of(runfilesSupplier, baseDirectory),
+        ImmutableList.of(runfilesSupplier, baseDirectory, spawn.getPathMapper().cacheKey()),
         new InputWalker() {
           @Override
           public SortedMap<PathFragment, ActionInput> getLeavesInputMapping()
               throws IOException, ForbiddenActionInputException {
             TreeMap<PathFragment, ActionInput> inputMap = new TreeMap<>();
             addRunfilesToInputs(
-                inputMap, runfilesSupplier, actionInputFileCache, artifactExpander, baseDirectory);
+                inputMap,
+                runfilesSupplier,
+                actionInputFileCache,
+                artifactExpander,
+                spawn.getPathMapper(),
+                baseDirectory);
             return inputMap;
           }
         });
@@ -348,7 +389,7 @@ public SortedMap<PathFragment, ActionInput> getLeavesInputMapping()
     // improved runtime.
     visitor.visit(
         // Cache key for the sub-mapping containing the fileset inputs for this spawn.
-        ImmutableList.of(filesetMappings, baseDirectory),
+        ImmutableList.of(filesetMappings, baseDirectory, spawn.getPathMapper().cacheKey()),
         new InputWalker() {
           @Override
           public SortedMap<PathFragment, ActionInput> getLeavesInputMapping()
@@ -365,11 +406,12 @@ private void walkNestedSetInputs(
       PathFragment baseDirectory,
       NestedSet<? extends ActionInput> someInputFiles,
       ArtifactExpander artifactExpander,
+      PathMapper pathMapper,
       InputVisitor visitor)
       throws IOException, ForbiddenActionInputException {
     visitor.visit(
         // Cache key for the sub-mapping containing the files in this nested set.
-        ImmutableList.of(someInputFiles.toNode(), baseDirectory),
+        ImmutableList.of(someInputFiles.toNode(), baseDirectory, pathMapper.cacheKey()),
         new InputWalker() {
           @Override
           public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
@@ -384,6 +426,7 @@ public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
                 inputMap,
                 NestedSetBuilder.wrap(someInputFiles.getOrder(), leaves),
                 artifactExpander,
+                pathMapper,
                 baseDirectory);
             return inputMap;
           }
@@ -394,11 +437,16 @@ public void visitNonLeaves(InputVisitor childVisitor)
             for (ActionInput input : someInputFiles.getLeaves()) {
               if (isTreeArtifact(input)) {
                 walkTreeInputs(
-                    baseDirectory, (SpecialArtifact) input, artifactExpander, childVisitor);
+                    baseDirectory,
+                    (SpecialArtifact) input,
+                    artifactExpander,
+                    pathMapper,
+                    childVisitor);
               }
             }
             for (NestedSet<? extends ActionInput> subInputs : someInputFiles.getNonLeaves()) {
-              walkNestedSetInputs(baseDirectory, subInputs, artifactExpander, childVisitor);
+              walkNestedSetInputs(
+                  baseDirectory, subInputs, artifactExpander, pathMapper, childVisitor);
             }
           }
         });
@@ -409,11 +457,12 @@ private void walkTreeInputs(
       PathFragment baseDirectory,
       SpecialArtifact tree,
       ArtifactExpander artifactExpander,
+      PathMapper pathMapper,
       InputVisitor visitor)
       throws IOException, ForbiddenActionInputException {
     visitor.visit(
         // Cache key for the sub-mapping containing the files in this tree artifact.
-        ImmutableList.of(tree, baseDirectory),
+        ImmutableList.of(tree, baseDirectory, pathMapper.cacheKey()),
         new InputWalker() {
           @Override
           public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
@@ -422,6 +471,7 @@ public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
                 inputMap,
                 NestedSetBuilder.create(Order.STABLE_ORDER, tree),
                 artifactExpander,
+                pathMapper,
                 baseDirectory);
             return inputMap;
           }