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);
