Google Git
Sign in
bazel/bazel/73eccc2bd089a9e1062f65bf0e026a392e10cb86^!/./src
commit
73eccc2bd089a9e1062f65bf0e026a392e10cb86
[log]
author
tomlu <tomlu@google.com>
Thu Sep 06 08:02:37 2018 -0700
committer
Copybara-Service <copybara-piper@google.com>
Thu Sep 06 08:04:33 2018 -0700
tree
0f0c190f8d99b87bab4a7124f02643e5225ec0ad
parent
34f8b34727322ccbea1785bd2e393e684dc0ff0a [diff]
Change the key type we keep around in our fileset mappings.

There is no need to throw away the fact that the mapping is artifact -> fileset symlinks so early.

This CL is otherwise a no-op.

RELNOTES: None
PiperOrigin-RevId: 211808154

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
index 04d9427..411506a 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java

@@ -28,7 +28,6 @@
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.FileSystem;
 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;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
@@ -50,7 +49,7 @@
     private final boolean shouldShowSubcommands;
     private final boolean prettyPrintArgs;
 
-    private ShowSubcommands(boolean shouldShowSubcommands, boolean prettyPrintArgs) {
+    ShowSubcommands(boolean shouldShowSubcommands, boolean prettyPrintArgs) {
       this.shouldShowSubcommands = shouldShowSubcommands;
       this.prettyPrintArgs = prettyPrintArgs;
     }
@@ -63,7 +62,7 @@
   private final MetadataHandler metadataHandler;
   private final FileOutErr fileOutErr;
   private final ImmutableMap<String, String> clientEnv;
-  private final ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> topLevelFilesets;
+  private final ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets;
   @Nullable private final ArtifactExpander artifactExpander;
   @Nullable private final Environment env;
 
@@ -82,7 +81,7 @@
       MetadataHandler metadataHandler,
       FileOutErr fileOutErr,
       Map<String, String> clientEnv,
-      ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
+      ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
       @Nullable ArtifactExpander artifactExpander,
       @Nullable SkyFunction.Environment env,
       @Nullable FileSystem actionFileSystem,
@@ -112,7 +111,7 @@
       MetadataHandler metadataHandler,
       FileOutErr fileOutErr,
       Map<String, String> clientEnv,
-      ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
+      ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
       ArtifactExpander artifactExpander,
       @Nullable FileSystem actionFileSystem,
       @Nullable Object skyframeDepsResult) {
@@ -187,8 +186,8 @@
    * <p>Notably, in the future, we want any action-scoped artifacts to resolve paths using this
    * method instead of {@link Artifact#getPath} because that does not allow filesystem injection.
    *
-   * <p>TODO(shahan): cleanup {@link Action}-scoped references to {@link Artifact.getPath} and
-   * {@link Artifact.getRoot}.
+   * <p>TODO(shahan): cleanup {@link Action}-scoped references to {@link Artifact#getPath} and
+   * {@link Artifact#getRoot}.
    */
   public Path getInputPath(ActionInput input) {
     return pathResolver.toPath(input);
@@ -228,7 +227,7 @@
     return executor.getEventHandler();
   }
 
-  public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getTopLevelFilesets() {
+  public ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getTopLevelFilesets() {
     return topLevelFilesets;
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
index aa49f2f..dcba403 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java

@@ -88,7 +88,7 @@
   }
 
   @Override
-  public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() {
+  public ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() {
     return ImmutableMap.of();
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
index 3b6447a..9d1abe4 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java

@@ -17,7 +17,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
-import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 import javax.annotation.Nullable;
 
@@ -54,7 +53,7 @@
   }
 
   @Override
-  public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() {
+  public ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() {
     return spawn.getFilesetMappings();
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java
index c5abd15..a9b6e54 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java

@@ -18,7 +18,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
-import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Map;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
@@ -36,7 +35,7 @@
   private final ImmutableList<? extends ActionInput> inputs;
   private final ImmutableList<? extends ActionInput> tools;
   private final RunfilesSupplier runfilesSupplier;
-  private final Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings;
+  private final Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings;
   private final ImmutableList<? extends ActionInput> outputs;
   private final ResourceSet localResources;
 
@@ -46,7 +45,7 @@
       ImmutableMap<String, String> environment,
       ImmutableMap<String, String> executionInfo,
       RunfilesSupplier runfilesSupplier,
-      Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings,
+      Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings,
       ImmutableList<? extends ActionInput> inputs,
       ImmutableList<? extends ActionInput> tools,
       ImmutableList<? extends ActionInput> outputs,
@@ -106,7 +105,7 @@
   }
 
   @Override
-  public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() {
+  public ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() {
     return ImmutableMap.copyOf(filesetMappings);
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
index 0440d22..3f7aa20 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java

@@ -17,7 +17,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
-import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 import javax.annotation.Nullable;
 
@@ -58,7 +57,7 @@
    * Map of the execpath at which we expect the Fileset symlink trees, to a list of
    * FilesetOutputSymlinks which contains the details of the Symlink trees.
    */
-  ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings();
+  ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getFilesetMappings();
 
   /**
    * Returns the list of files that are required to execute this spawn (e.g. the compiler binary),

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index a818dec..62f7dab 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java

@@ -363,7 +363,7 @@
   Spawn getSpawn(
       ArtifactExpander artifactExpander,
       Map<String, String> clientEnv,
-      Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings)
+      Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings)
       throws CommandLineExpansionException {
     ExpandedCommandLines expandedCommandLines =
         commandLines.expand(artifactExpander, getPrimaryOutput().getExecPath(), commandLineLimits);
@@ -501,7 +501,7 @@
   private class ActionSpawn extends BaseSpawn {
 
     private final ImmutableList<ActionInput> inputs;
-    private final Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings;
+    private final Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings;
     private final ImmutableMap<String, String> effectiveEnvironment;
 
     /**
@@ -514,7 +514,7 @@
         ImmutableList<String> arguments,
         Map<String, String> clientEnv,
         Iterable<? extends ActionInput> additionalInputs,
-        Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings) {
+        Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings) {
       super(
           arguments,
           ImmutableMap.<String, String>of(),
@@ -543,7 +543,7 @@
     }
 
     @Override
-    public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() {
+    public ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() {
       return ImmutableMap.copyOf(filesetMappings);
     }
 

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 ae9ad7c..113ff88 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

@@ -175,14 +175,14 @@
 
   @VisibleForTesting
   void addFilesetManifests(
-      Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings,
+      Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings,
       Map<PathFragment, ActionInput> inputMappings)
       throws IOException {
-    for (PathFragment manifestExecpath : filesetMappings.keySet()) {
-      ImmutableList<FilesetOutputSymlink> outputSymlinks = filesetMappings.get(manifestExecpath);
+    for (Artifact fileset : filesetMappings.keySet()) {
+      ImmutableList<FilesetOutputSymlink> outputSymlinks = filesetMappings.get(fileset);
       FilesetManifest filesetManifest =
           FilesetManifest.constructFilesetManifest(
-              outputSymlinks, manifestExecpath, relSymlinkBehavior);
+              outputSymlinks, fileset.getExecPath(), relSymlinkBehavior);
 
       for (Map.Entry<PathFragment, String> mapping : filesetManifest.getEntries().entrySet()) {
         String value = mapping.getValue();

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 ef8d0fa..eb30532 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

@@ -463,7 +463,7 @@
 
     // Make sure this is a regular HashMap rather than ImmutableMapBuilder so that we are safe
     // in case of collisions.
-    Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings = new HashMap<>();
+    Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings = new HashMap<>();
     for (Artifact actionInput : action.getInputs()) {
       if (!actionInput.isFileset()) {
         continue;
@@ -474,16 +474,15 @@
       if (mapping == null) {
         return null;
       }
-      filesetMappings.put(actionInput.getExecPath(), mapping);
+      filesetMappings.put(actionInput, mapping);
     }
 
-    ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> topLevelFilesets =
+    ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets =
         ImmutableMap.copyOf(filesetMappings);
 
     // Aggregate top-level Filesets with Filesets nested in Runfiles. Both should be used to update
     // the FileSystem context.
-    state.expandedFilesets
-        .forEach((artifact, links) -> filesetMappings.put(artifact.getExecPath(), links));
+    state.expandedFilesets.forEach(filesetMappings::put);
     try {
       state.updateFileSystemContext(skyframeActionExecutor, env, metadataHandler,
           ImmutableMap.copyOf(filesetMappings));
@@ -836,7 +835,7 @@
         SkyframeActionExecutor executor,
         Environment env,
         ActionMetadataHandler metadataHandler,
-        ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> filesets)
+        ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> filesets)
         throws IOException {
       if (actionFileSystem != null) {
         executor.updateActionFileSystemContext(

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 4215391..1c7bd0e 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

@@ -396,8 +396,11 @@
   }
 
   void updateActionFileSystemContext(
-      FileSystem actionFileSystem, Environment env, MetadataConsumer consumer,
-      ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> filesets) throws IOException {
+      FileSystem actionFileSystem,
+      Environment env,
+      MetadataConsumer consumer,
+      ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> filesets)
+      throws IOException {
     outputService.updateActionFileSystemContext(actionFileSystem, env, consumer, filesets);
   }
 
@@ -510,7 +513,7 @@
       MetadataHandler metadataHandler,
       Map<Artifact, Collection<Artifact>> expandedInputs,
       Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets,
-      ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
+      ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
       @Nullable FileSystem actionFileSystem,
       @Nullable Object skyframeDepsResult) {
     FileOutErr fileOutErr = actionLogBufferPathGenerator.generate(

diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
index 0f87039..d45912d 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java

@@ -143,8 +143,10 @@
    * @param filesets The Fileset symlinks known for this action.
    */
   default void updateActionFileSystemContext(
-      FileSystem actionFileSystem, Environment env, MetadataConsumer consumer,
-      ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> filesets)
+      FileSystem actionFileSystem,
+      Environment env,
+      MetadataConsumer consumer,
+      ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> filesets)
       throws IOException {}
 
   default boolean supportsPathResolverForArtifactValues() {

diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
index 581dfc6..b7fd511 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java

@@ -68,6 +68,7 @@
 
   private FileSystem fs;
   private Path execRoot;
+  private ArtifactRoot rootDir;
   private SpawnInputExpander expander;
   private Map<PathFragment, ActionInput> inputMappings;
 
@@ -75,6 +76,7 @@
   public final void createSpawnInputExpander() {
     fs = new InMemoryFileSystem();
     execRoot = fs.getPath("/root");
+    rootDir = ArtifactRoot.asDerivedRoot(execRoot, fs.getPath("/root/out"));
     expander = new SpawnInputExpander(execRoot, /*strict=*/ true);
     inputMappings = Maps.newHashMap();
   }
@@ -391,13 +393,21 @@
         PathFragment.create(from), PathFragment.create(to));
   }
 
-  private ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> simpleFilesetManifest() {
+  private ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> simpleFilesetManifest() {
     return ImmutableMap.of(
-        PathFragment.create("out"),
+        createFileset("out"),
         ImmutableList.of(
             filesetSymlink("workspace/bar", "foo"), filesetSymlink("workspace/foo", "/foo/bar")));
   }
 
+  private SpecialArtifact createFileset(String execPath) {
+    return new SpecialArtifact(
+        rootDir,
+        PathFragment.create(execPath),
+        ArtifactOwner.NullArtifactOwner.INSTANCE,
+        SpecialArtifactType.FILESET);
+  }
+
   @Test
   public void testManifestWithErrorOnRelativeSymlink() throws Exception {
     expander = new SpawnInputExpander(execRoot, /*strict=*/ true, ERROR);