Do not reconstitute absolute path until after FilesetManifest.
This way callers aren't required to pass in the exec root.
RELNOTES: None.
PiperOrigin-RevId: 217500815
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
index 53bfb09..133aaf3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
-import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.CommandLineItem;
@@ -359,8 +358,7 @@
FilesetManifest.constructFilesetManifest(
artifactExpander.getFileset(fileset),
fileset.getExecPath(),
- RelativeSymlinkBehavior.IGNORE,
- extractExecRoot(fileset));
+ RelativeSymlinkBehavior.IGNORE);
for (PathFragment relativePath : filesetManifest.getEntries().keySet()) {
expandedValues.add(new FilesetSymlinkFile(fileset, relativePath));
}
@@ -369,14 +367,6 @@
}
}
- // TODO(b/117267351): Pass in the exec root more cleanly, or put relative paths in the manifest.
- private static PathFragment extractExecRoot(Artifact fileset) {
- Preconditions.checkArgument(!fileset.isSourceArtifact(), fileset);
- ArtifactRoot root = fileset.getRoot();
- PathFragment rootFrag = root.getRoot().asPath().asFragment();
- return rootFrag.subFragment(0, rootFrag.segmentCount() - root.getExecPath().segmentCount());
- }
-
private int addToFingerprint(
List<Object> arguments,
int argi,
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 fa4e993..7dd1a98 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
@@ -51,17 +51,15 @@
public static FilesetManifest constructFilesetManifest(
List<FilesetOutputSymlink> outputSymlinks,
PathFragment targetPrefix,
- RelativeSymlinkBehavior relSymlinkBehavior,
- PathFragment execRoot)
+ RelativeSymlinkBehavior relSymlinkBehavior)
throws IOException {
LinkedHashMap<PathFragment, String> entries = new LinkedHashMap<>();
Map<PathFragment, String> relativeLinks = new HashMap<>();
Map<String, FileArtifactValue> artifactValues = new HashMap<>();
for (FilesetOutputSymlink outputSymlink : outputSymlinks) {
PathFragment fullLocation = targetPrefix.getRelative(outputSymlink.getName());
- PathFragment linkTarget = outputSymlink.reconstituteTargetPath(execRoot);
- String artifact = Strings.emptyToNull(linkTarget.getPathString());
- if (!linkTarget.isAbsolute() && !linkTarget.isEmpty()) {
+ String artifact = Strings.emptyToNull(outputSymlink.getTargetPath().getPathString());
+ if (isRelativeSymlink(outputSymlink)) {
addRelativeSymlinkEntry(artifact, fullLocation, relSymlinkBehavior, relativeLinks);
} else if (!entries.containsKey(fullLocation)) { // Keep consistent behavior: no overwriting.
entries.put(fullLocation, artifact);
@@ -70,7 +68,14 @@
artifactValues.put(artifact, (FileArtifactValue) outputSymlink.getMetadata());
}
}
- return constructFilesetManifest(entries, relativeLinks, artifactValues);
+ resolveRelativeSymlinks(entries, relativeLinks);
+ return new FilesetManifest(entries, artifactValues);
+ }
+
+ private static boolean isRelativeSymlink(FilesetOutputSymlink symlink) {
+ return !symlink.getTargetPath().isEmpty()
+ && !symlink.getTargetPath().isAbsolute()
+ && !symlink.isRelativeToExecRoot();
}
/** Potentially adds the relative symlink to the map, depending on {@code relSymlinkBehavior}. */
@@ -95,12 +100,14 @@
private static final int MAX_SYMLINK_TRAVERSALS = 256;
- private static FilesetManifest constructFilesetManifest(
- Map<PathFragment, String> entries,
- Map<PathFragment, String> relativeLinks,
- Map<String, FileArtifactValue> artifactValues) {
- // Resolve relative symlinks. Note that relativeLinks only contains entries in RESOLVE mode.
- // We must find targets for these symlinks that are not inside the Fileset itself.
+ /**
+ * Resolves relative symlinks and puts them in the {@code entries} map.
+ *
+ * <p>Note that {@code relativeLinks} should only contain entries in {@link
+ * RelativeSymlinkBehavior#RESOLVE} mode.
+ */
+ private static void resolveRelativeSymlinks(
+ Map<PathFragment, String> entries, Map<PathFragment, String> relativeLinks) {
for (Map.Entry<PathFragment, String> e : relativeLinks.entrySet()) {
PathFragment location = e.getKey();
String value = e.getValue();
@@ -142,22 +149,37 @@
entries.put(location, entries.get(actualLocation));
}
}
- return new FilesetManifest(entries, artifactValues);
}
private final Map<PathFragment, String> entries;
private final Map<String, FileArtifactValue> artifactValues;
- private FilesetManifest(Map<PathFragment, String> entries,
- Map<String, FileArtifactValue> artifactValues) {
+ private FilesetManifest(
+ Map<PathFragment, String> entries, Map<String, FileArtifactValue> artifactValues) {
this.entries = Collections.unmodifiableMap(entries);
this.artifactValues = artifactValues;
}
+ /**
+ * Returns a mapping of symlink name to its target path.
+ *
+ * <p>Values in this map can be:
+ *
+ * <ul>
+ * <li>An absolute path.
+ * <li>A relative path, which should be considered relative to the exec root.
+ * <li>{@code null}, which represents an empty file.
+ * </ul>
+ */
public Map<PathFragment, String> getEntries() {
return entries;
}
+ /**
+ * Returns a mapping of target path to {@link FileArtifactValue}.
+ *
+ * <p>The keyset of this map is a subset of the values in the map returned by {@link #getEntries}.
+ */
public Map<String, FileArtifactValue> getArtifactValues() {
return artifactValues;
}
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 cf53a85..81a8170 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
@@ -163,11 +163,14 @@
ImmutableList<FilesetOutputSymlink> outputSymlinks = filesetMappings.get(fileset);
FilesetManifest filesetManifest =
FilesetManifest.constructFilesetManifest(
- outputSymlinks, fileset.getExecPath(), relSymlinkBehavior, execRoot.asFragment());
+ outputSymlinks, fileset.getExecPath(), relSymlinkBehavior);
for (Map.Entry<PathFragment, String> mapping : filesetManifest.getEntries().entrySet()) {
String value = mapping.getValue();
- ActionInput artifact = value == null ? EMPTY_FILE : ActionInputHelper.fromPath(value);
+ ActionInput artifact =
+ value == null
+ ? EMPTY_FILE
+ : ActionInputHelper.fromPath(execRoot.getRelative(value).getPathString());
addMapping(inputMappings, mapping.getKey(), artifact);
}
}
