Use the in-memory metadata in blaze as the source of truth for Fileset mappings

instead of the manifest files.

RELNOTES: None
PiperOrigin-RevId: 195149880

4 files changed, 94 insertions, 75 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
index c516b7875a..d9ae236622 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
@@ -35,7 +35,6 @@ import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.exec.SpawnCache.CacheHandle;
 import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
 import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
-import com.google.devtools.build.lib.rules.fileset.FilesetActionContext;
 import com.google.devtools.build.lib.util.CommandFailureUtils;
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.Path;
@@ -233,11 +232,11 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte
     @Override
     public SortedMap<PathFragment, ActionInput> getInputMapping() throws IOException {
       if (lazyInputMapping == null) {
-        lazyInputMapping = spawnInputExpander.getInputMapping(
-            spawn,
-            actionExecutionContext.getArtifactExpander(),
-            actionExecutionContext.getActionInputFileCache(),
-            actionExecutionContext.getContext(FilesetActionContext.class));
+        lazyInputMapping =
+            spawnInputExpander.getInputMapping(
+                spawn,
+                actionExecutionContext.getArtifactExpander(),
+                actionExecutionContext.getActionInputFileCache());
       }
       return lazyInputMapping;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java b/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
index 319ea6bfa8..05dee2b1b8 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
@@ -16,7 +16,7 @@ package com.google.devtools.build.lib.exec;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.io.LineProcessor;
-import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -25,6 +25,7 @@ import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -52,19 +53,6 @@ public final class FilesetManifest {
   }
 
   public static FilesetManifest parseManifestFile(
-      Artifact manifest,
-      Path execRoot,
-      String workspaceName,
-      RelativeSymlinkBehavior relSymlinkBehavior)
-          throws IOException {
-    return parseManifestFile(
-        manifest.getExecPath(),
-        execRoot,
-        workspaceName,
-        relSymlinkBehavior);
-  }
-
-  public static FilesetManifest parseManifestFile(
       PathFragment manifest,
       Path execRoot,
       String workspaceName,
@@ -82,13 +70,29 @@ public final class FilesetManifest {
     }
   }
 
+  public static FilesetManifest constructFilesetManifest(
+      List<FilesetOutputSymlink> outputSymlinks,
+      PathFragment targetPrefix,
+      RelativeSymlinkBehavior relSymlinkbehavior)
+      throws IOException {
+    LinkedHashMap<PathFragment, String> entries = new LinkedHashMap<>();
+    Map<PathFragment, String> relativeLinks = new HashMap<>();
+    for (FilesetOutputSymlink outputSymlink : outputSymlinks) {
+      PathFragment fullLocation = targetPrefix.getRelative(outputSymlink.name);
+      String artifact = outputSymlink.target.getPathString();
+      artifact = artifact.isEmpty() ? null : artifact;
+      addSymlinkEntry(artifact, fullLocation, relSymlinkbehavior, entries, relativeLinks);
+    }
+    return constructFilesetManifest(entries, relativeLinks);
+  }
+
   private static final class ManifestLineProcessor implements LineProcessor<FilesetManifest> {
     private final String workspaceName;
     private final PathFragment targetPrefix;
     private final RelativeSymlinkBehavior relSymlinkBehavior;
 
     private int lineNum;
-    private final Map<PathFragment, String> entries = new LinkedHashMap<>();
+    private final LinkedHashMap<PathFragment, String> entries = new LinkedHashMap<>();
     private final Map<PathFragment, String> relativeLinks = new HashMap<>();
 
     ManifestLineProcessor(
@@ -134,44 +138,59 @@ public final class FilesetManifest {
       }
 
       PathFragment fullLocation = targetPrefix.getRelative(location);
-      if (!entries.containsKey(fullLocation)) {
-        boolean isRelativeSymlink = artifact != null && !artifact.startsWith("/");
-        if (isRelativeSymlink && relSymlinkBehavior.equals(RelativeSymlinkBehavior.ERROR)) {
-          throw new IOException(String.format("runfiles target is not absolute: %s", artifact));
-        }
-        if (!isRelativeSymlink
-            || relSymlinkBehavior.equals(RelativeSymlinkBehavior.RESOLVE)) {
-          entries.put(fullLocation, artifact);
-          if (artifact != null && !artifact.startsWith("/")) {
-            relativeLinks.put(fullLocation, artifact);
-          }
-        }
-      }
+      addSymlinkEntry(artifact, fullLocation, relSymlinkBehavior, entries, relativeLinks);
       return true;
     }
 
     @Override
     public FilesetManifest getResult() {
-      // Resolve relative symlinks if possible. Note that relativeLinks only contains entries in
-      // RESOLVE mode.
-      for (Map.Entry<PathFragment, String> e : relativeLinks.entrySet()) {
-        PathFragment location = e.getKey();
-        String value = e.getValue();
-        PathFragment actualLocation = location.getParentDirectory().getRelative(value);
-        String actual = entries.get(actualLocation);
-        boolean isActualAcceptable = actual == null || actual.startsWith("/");
-        if (!entries.containsKey(actualLocation) || !isActualAcceptable) {
-          throw new IllegalStateException(
-              String.format(
-                  "runfiles target '%s' is not absolute, and could not be resolved in the same "
-                  + "Fileset", value));
+      return constructFilesetManifest(entries, relativeLinks);
+    }
+  }
+
+  private static void addSymlinkEntry(
+      String artifact,
+      PathFragment fullLocation,
+      RelativeSymlinkBehavior relSymlinkBehavior,
+      LinkedHashMap<PathFragment, String> entries,
+      Map<PathFragment, String> relativeLinks)
+      throws IOException {
+    if (!entries.containsKey(fullLocation)) {
+      boolean isRelativeSymlink = artifact != null && !artifact.startsWith("/");
+      if (isRelativeSymlink && relSymlinkBehavior.equals(RelativeSymlinkBehavior.ERROR)) {
+        throw new IOException(String.format("runfiles target is not absolute: %s", artifact));
+      }
+      if (!isRelativeSymlink || relSymlinkBehavior.equals(RelativeSymlinkBehavior.RESOLVE)) {
+        entries.put(fullLocation, artifact);
+        if (artifact != null && !artifact.startsWith("/")) {
+          relativeLinks.put(fullLocation, artifact);
         }
-        entries.put(location, actual);
       }
-      return new FilesetManifest(entries);
     }
   }
 
+  private static FilesetManifest constructFilesetManifest(
+      Map<PathFragment, String> entries, Map<PathFragment, String> relativeLinks) {
+    // Resolve relative symlinks if possible. Note that relativeLinks only contains entries in
+    // RESOLVE mode.
+    for (Map.Entry<PathFragment, String> e : relativeLinks.entrySet()) {
+      PathFragment location = e.getKey();
+      String value = e.getValue();
+      PathFragment actualLocation = location.getParentDirectory().getRelative(value);
+      String actual = entries.get(actualLocation);
+      boolean isActualAcceptable = actual == null || actual.startsWith("/");
+      if (!entries.containsKey(actualLocation) || !isActualAcceptable) {
+        throw new IllegalStateException(
+            String.format(
+                "runfiles target '%s' is not absolute, and could not be resolved in the same "
+                    + "Fileset",
+                value));
+      }
+      entries.put(location, actual);
+    }
+    return new FilesetManifest(entries);
+  }
+
   private final Map<PathFragment, String> entries;
 
   private FilesetManifest(Map<PathFragment, String> entries) {
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java
index f57968984d..cff57d121f 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java
@@ -17,16 +17,16 @@ import static com.google.devtools.build.lib.exec.FilesetManifest.RelativeSymlink
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputFileCache;
 import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
+import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
 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;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput;
-import com.google.devtools.build.lib.rules.fileset.FilesetActionContext;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
@@ -109,12 +109,13 @@ public class SpawnInputExpander {
    * Parses the fileset manifest file, adding to the inputMappings where appropriate. Lines
    * referring to directories are recursed.
    */
+  // TODO(kush): make tests use the method with in-memory fileset data.
   @VisibleForTesting
   void parseFilesetManifest(
       Map<PathFragment, ActionInput> inputMappings, Artifact manifest, String workspaceName)
       throws IOException {
     FilesetManifest filesetManifest =
-        FilesetManifest.parseManifestFile(manifest, execRoot, workspaceName, ERROR);
+        FilesetManifest.parseManifestFile(manifest.getExecPath(), execRoot, workspaceName, ERROR);
     for (Map.Entry<PathFragment, String> mapping : filesetManifest.getEntries().entrySet()) {
       String value = mapping.getValue();
       ActionInput artifact = value == null ? EMPTY_FILE : ActionInputHelper.fromPath(value);
@@ -122,6 +123,23 @@ public class SpawnInputExpander {
     }
   }
 
+  public void addFilesetManifests(
+      Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings,
+      Map<PathFragment, ActionInput> inputMappings)
+      throws IOException {
+    for (PathFragment manifestExecpath : filesetMappings.keySet()) {
+      ImmutableList<FilesetOutputSymlink> outputSymlinks = filesetMappings.get(manifestExecpath);
+      FilesetManifest filesetManifest =
+          FilesetManifest.constructFilesetManifest(outputSymlinks, manifestExecpath, ERROR);
+
+      for (Map.Entry<PathFragment, String> mapping : filesetManifest.getEntries().entrySet()) {
+        String value = mapping.getValue();
+        ActionInput artifact = value == null ? EMPTY_FILE : ActionInputHelper.fromPath(value);
+        addMapping(inputMappings, mapping.getKey(), artifact);
+      }
+    }
+  }
+
   private void addInputs(
       Map<PathFragment, ActionInput> inputMap, Spawn spawn, ArtifactExpander artifactExpander) {
     List<ActionInput> inputs =
@@ -134,34 +152,17 @@ public class SpawnInputExpander {
   /**
    * Convert the inputs of the given spawn to a map from exec-root relative paths to action inputs.
    * The returned map never contains {@code null} values; it uses {@link #EMPTY_FILE} for empty
-   * files, which is an instance of {@link VirtualActionInput}.
-   */
-  public SortedMap<PathFragment, ActionInput> getInputMapping(
-      Spawn spawn, ArtifactExpander artifactExpander, ActionInputFileCache actionInputFileCache,
-      FilesetActionContext filesetContext)
-          throws IOException {
-    return getInputMapping(
-        spawn,
-        artifactExpander,
-        actionInputFileCache,
-        filesetContext == null ? null : filesetContext.getWorkspaceName());
-  }
-
-  /**
-   * Convert the inputs of the given spawn to a map from exec-root relative paths to action inputs.
-   * In some cases, this generates empty files, for which it uses {@code null}.
+   * files, which is an instance of {@link
+   * com.google.devtools.build.lib.actions.cache.VirtualActionInput}.
    */
   public SortedMap<PathFragment, ActionInput> getInputMapping(
-      Spawn spawn, ArtifactExpander artifactExpander, ActionInputFileCache actionInputFileCache,
-      String workspaceName)
-          throws IOException {
+      Spawn spawn, ArtifactExpander artifactExpander, ActionInputFileCache actionInputFileCache)
+      throws IOException {
     TreeMap<PathFragment, ActionInput> inputMap = new TreeMap<>();
     addInputs(inputMap, spawn, artifactExpander);
     addRunfilesToInputs(
         inputMap, spawn.getRunfilesSupplier(), actionInputFileCache);
-    for (Artifact manifest : spawn.getFilesetManifests()) {
-      parseFilesetManifest(inputMap, manifest, workspaceName);
-    }
+    addFilesetManifests(spawn.getFilesetMappings(), inputMap);
     return inputMap;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
index c1f04356cc..c1248a6e4b 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
@@ -124,9 +124,9 @@ public class StandaloneTestStrategy extends TestStrategy {
             ImmutableMap.copyOf(executionInfo),
             new RunfilesSupplierImpl(
                 runfilesDir.relativeTo(execRoot), action.getExecutionSettings().getRunfiles()),
+            ImmutableMap.of(),
             /*inputs=*/ ImmutableList.copyOf(action.getInputs()),
             /*tools=*/ ImmutableList.<Artifact>of(),
-            /*filesetManifests=*/ ImmutableList.<Artifact>of(),
             ImmutableList.copyOf(action.getSpawnOutputs()),
             localResourceUsage);