Strip the execution root from target paths in FilesetOutputSymlink.
RELNOTES: None.
PiperOrigin-RevId: 215754628
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputSymlink.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputSymlink.java
index fd89e58..73a9499 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputSymlink.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputSymlink.java
@@ -26,8 +26,16 @@
public abstract PathFragment getName();
/**
- * Target of the symlink. This may be relative to the target's location if the target itself is a
- * relative symlink. We can override it by using FilesetEntry.symlinks = 'dereference'.
+ * Target of the symlink.
+ *
+ * <p>This path is one of the following:
+ *
+ * <ol>
+ * <li>Relative to the execution root, in which case {@link #isRelativeToExecRoot} will return
+ * {@code true}.
+ * <li>An absolute path to the source tree.
+ * <li>A relative path that should be considered relative to the link.
+ * </ol>
*/
public abstract PathFragment getTargetPath();
@@ -41,8 +49,21 @@
/** true if the target is a generated artifact */
public abstract boolean isGeneratedTarget();
+ /** Returns {@code true} if this symlink is relative to the execution root. */
+ public abstract boolean isRelativeToExecRoot();
+
+ /**
+ * Reconstitutes the original target path of this symlink.
+ *
+ * <p>This method essentially performs the inverse of what is done in {@link #create}. If the
+ * execution root was stripped originally, it is re-prepended.
+ */
+ public final PathFragment reconstituteTargetPath(PathFragment execRoot) {
+ return isRelativeToExecRoot() ? execRoot.getRelative(getTargetPath()) : getTargetPath();
+ }
+
@Override
- public String toString() {
+ public final String toString() {
if (getMetadata() == STRIPPED_METADATA) {
return String.format(
"FilesetOutputSymlink(%s -> %s)",
@@ -55,19 +76,60 @@
}
@VisibleForTesting
- public static FilesetOutputSymlink createForTesting(PathFragment name, PathFragment target) {
- return new AutoValue_FilesetOutputSymlink(name, target, STRIPPED_METADATA, false);
+ public static FilesetOutputSymlink createForTesting(
+ PathFragment name, PathFragment target, PathFragment execRoot) {
+ return create(name, target, STRIPPED_METADATA, false, execRoot);
+ }
+
+ @VisibleForTesting
+ public static FilesetOutputSymlink createAlreadyRelativizedForTesting(
+ PathFragment name, PathFragment target, boolean isRelativeToExecRoot) {
+ return createAlreadyRelativized(name, target, STRIPPED_METADATA, false, isRelativeToExecRoot);
}
/**
+ * Creates a {@link FilesetOutputSymlink}.
+ *
+ * <p>To facilitate cross-device sharing, {@code target} will have the machine-local {@code
+ * execRoot} stripped if necessary. If this happens, {@link #isRelativeToExecRoot} will return
+ * {@code true}.
+ *
* @param name relative path under the Fileset's output directory, including FilesetEntry.destdir
* with and FilesetEntry.strip_prefix applied (if applicable)
* @param target relative or absolute value of the link
* @param metadata metadata corresponding to the target.
* @param isGeneratedTarget true if the target is generated.
+ * @param execRoot the execution root
*/
public static FilesetOutputSymlink create(
- PathFragment name, PathFragment target, Object metadata, boolean isGeneratedTarget) {
- return new AutoValue_FilesetOutputSymlink(name, target, metadata, isGeneratedTarget);
+ PathFragment name,
+ PathFragment target,
+ Object metadata,
+ boolean isGeneratedTarget,
+ PathFragment execRoot) {
+ boolean isRelativeToExecRoot = false;
+ // Check if the target is under the execution root. This is not always the case because the
+ // target may point to a source artifact or it may point to another symlink, in which case the
+ // target path is already relative.
+ if (target.startsWith(execRoot)) {
+ target = target.relativeTo(execRoot);
+ isRelativeToExecRoot = true;
+ }
+ return createAlreadyRelativized(
+ name, target, metadata, isGeneratedTarget, isRelativeToExecRoot);
+ }
+
+ /**
+ * Same as {@link #create}, except assumes that {@code target} already had the execution root
+ * stripped if necessary.
+ */
+ public static FilesetOutputSymlink createAlreadyRelativized(
+ PathFragment name,
+ PathFragment target,
+ Object metadata,
+ boolean isGeneratedTarget,
+ boolean isRelativeToExecRoot) {
+ return new AutoValue_FilesetOutputSymlink(
+ name, target, metadata, isGeneratedTarget, isRelativeToExecRoot);
}
}
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 133aaf3..53bfb09 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,6 +22,7 @@
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;
@@ -358,7 +359,8 @@
FilesetManifest.constructFilesetManifest(
artifactExpander.getFileset(fileset),
fileset.getExecPath(),
- RelativeSymlinkBehavior.IGNORE);
+ RelativeSymlinkBehavior.IGNORE,
+ extractExecRoot(fileset));
for (PathFragment relativePath : filesetManifest.getEntries().keySet()) {
expandedValues.add(new FilesetSymlinkFile(fileset, relativePath));
}
@@ -367,6 +369,14 @@
}
}
+ // 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 40e3842..220beec 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,6 +16,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.LineProcessor;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -75,16 +76,17 @@
public static FilesetManifest constructFilesetManifest(
List<FilesetOutputSymlink> outputSymlinks,
PathFragment targetPrefix,
- RelativeSymlinkBehavior relSymlinkbehavior)
+ RelativeSymlinkBehavior relSymlinkBehavior,
+ PathFragment execRoot)
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());
- String artifact = outputSymlink.getTargetPath().getPathString();
- artifact = artifact.isEmpty() ? null : artifact;
- addSymlinkEntry(artifact, fullLocation, relSymlinkbehavior, entries, relativeLinks);
+ PathFragment linkTarget = outputSymlink.reconstituteTargetPath(execRoot);
+ String artifact = Strings.emptyToNull(linkTarget.getPathString());
+ addSymlinkEntry(artifact, fullLocation, relSymlinkBehavior, entries, relativeLinks);
if (outputSymlink.getMetadata() instanceof FileArtifactValue) {
artifactValues.put(artifact, (FileArtifactValue) outputSymlink.getMetadata());
}
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 113ff88..25d4c81 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
@@ -182,7 +182,7 @@
ImmutableList<FilesetOutputSymlink> outputSymlinks = filesetMappings.get(fileset);
FilesetManifest filesetManifest =
FilesetManifest.constructFilesetManifest(
- outputSymlinks, fileset.getExecPath(), relSymlinkBehavior);
+ outputSymlinks, fileset.getExecPath(), relSymlinkBehavior, execRoot.asFragment());
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/FilesetEntryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
index aa155b1..5312ca9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
@@ -47,6 +47,12 @@
}
}
+ private final PathFragment execRoot;
+
+ public FilesetEntryFunction(PathFragment execRoot) {
+ this.execRoot = execRoot;
+ }
+
@Override
public SkyValue compute(SkyKey key, Environment env)
throws FilesetEntryFunctionException, InterruptedException {
@@ -180,7 +186,7 @@
}
/** Stores an output symlink unless it would overwrite an existing one. */
- private static void maybeStoreSymlink(
+ private void maybeStoreSymlink(
PathFragment linkName,
PathFragment linkTarget,
Object metadata,
@@ -190,7 +196,8 @@
linkName = destPath.getRelative(linkName);
if (!result.containsKey(linkName)) {
result.put(
- linkName, FilesetOutputSymlink.create(linkName, linkTarget, metadata, isGenerated));
+ linkName,
+ FilesetOutputSymlink.create(linkName, linkTarget, metadata, isGenerated, execRoot));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 300cd4e..bf7ae1d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -552,7 +552,9 @@
this.actionExecutionFunction = actionExecutionFunction;
map.put(SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL,
new RecursiveFilesystemTraversalFunction());
- map.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction());
+ map.put(
+ SkyFunctions.FILESET_ENTRY,
+ new FilesetEntryFunction(directories.getExecRoot().asFragment()));
map.put(
SkyFunctions.ACTION_TEMPLATE_EXPANSION,
new ActionTemplateExpansionFunction(actionKeyContext));
diff --git a/src/test/java/com/google/devtools/build/lib/actions/FilesetOutputSymlinkTest.java b/src/test/java/com/google/devtools/build/lib/actions/FilesetOutputSymlinkTest.java
new file mode 100644
index 0000000..bc526c6
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/actions/FilesetOutputSymlinkTest.java
@@ -0,0 +1,61 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.actions;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.vfs.PathFragment;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link FilesetOutputSymlink}. */
+@RunWith(JUnit4.class)
+public final class FilesetOutputSymlinkTest {
+
+ private static final PathFragment EXEC_ROOT = PathFragment.create("/example/execroot");
+
+ private static final FilesetOutputSymlink createSymlinkTo(String path) {
+ return FilesetOutputSymlink.create(
+ PathFragment.create("any/path"), PathFragment.create(path), new Object(), false, EXEC_ROOT);
+ }
+
+ @Test
+ public void stripsExecRootFromTarget() throws Exception {
+ FilesetOutputSymlink symlink = createSymlinkTo("/example/execroot/some/path");
+ PathFragment targetPath = symlink.getTargetPath();
+ assertThat(targetPath.getPathString()).isEqualTo("some/path");
+ }
+
+ @Test
+ public void reconstitutesTargetPath_underExecRoot() throws Exception {
+ FilesetOutputSymlink symlink = createSymlinkTo("/example/execroot/some/path");
+ PathFragment targetPath = symlink.reconstituteTargetPath(EXEC_ROOT);
+ assertThat(targetPath.getPathString()).isEqualTo("/example/execroot/some/path");
+ }
+
+ @Test
+ public void reconstitutesTargetPath_differentExecRoot() throws Exception {
+ FilesetOutputSymlink symlink = createSymlinkTo("/example/execroot/some/path");
+ PathFragment targetPath = symlink.reconstituteTargetPath(PathFragment.create("/new/execroot"));
+ assertThat(targetPath.getPathString()).isEqualTo("/new/execroot/some/path");
+ }
+
+ @Test
+ public void reconstitutesTargetPath_notUnderExecRoot() throws Exception {
+ FilesetOutputSymlink symlink = createSymlinkTo("some/path");
+ PathFragment targetPath = symlink.reconstituteTargetPath(EXEC_ROOT);
+ assertThat(targetPath.getPathString()).isEqualTo("some/path");
+ }
+}
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 b7fd511..5b59a16 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
@@ -388,16 +388,16 @@
.containsEntry(PathFragment.create("out/foo/bar"), ActionInputHelper.fromPath("/some"));
}
- private FilesetOutputSymlink filesetSymlink(String from, String to) {
+ private static FilesetOutputSymlink filesetSymlink(String from, String to) {
return FilesetOutputSymlink.createForTesting(
- PathFragment.create(from), PathFragment.create(to));
+ PathFragment.create(from), PathFragment.create(to), PathFragment.create("/root"));
}
private ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> simpleFilesetManifest() {
return ImmutableMap.of(
createFileset("out"),
ImmutableList.of(
- filesetSymlink("workspace/bar", "foo"), filesetSymlink("workspace/foo", "/foo/bar")));
+ filesetSymlink("workspace/bar", "foo"), filesetSymlink("workspace/foo", "/root/bar")));
}
private SpecialArtifact createFileset(String execPath) {
@@ -426,7 +426,7 @@
expander.addFilesetManifests(simpleFilesetManifest(), entries);
assertThat(entries)
.containsExactly(
- PathFragment.create("out/workspace/foo"), ActionInputHelper.fromPath("/foo/bar"));
+ PathFragment.create("out/workspace/foo"), ActionInputHelper.fromPath("/root/bar"));
}
@Test
@@ -436,7 +436,7 @@
expander.addFilesetManifests(simpleFilesetManifest(), entries);
assertThat(entries)
.containsExactly(
- PathFragment.create("out/workspace/bar"), ActionInputHelper.fromPath("/foo/bar"),
- PathFragment.create("out/workspace/foo"), ActionInputHelper.fromPath("/foo/bar"));
+ PathFragment.create("out/workspace/bar"), ActionInputHelper.fromPath("/root/bar"),
+ PathFragment.create("out/workspace/foo"), ActionInputHelper.fromPath("/root/bar"));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
index 5e5ff14..f82b763 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
@@ -59,10 +59,8 @@
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
-import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
@@ -124,7 +122,8 @@
new BlacklistedPackagePrefixesFunction(
/*hardcodedBlacklistedPackagePrefixes=*/ ImmutableSet.of(),
/*additionalBlacklistedPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT));
- skyFunctions.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction());
+ skyFunctions.put(
+ SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction(rootDirectory.asFragment()));
skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
differencer = new SequencedRecordingDifferencer();
@@ -191,31 +190,32 @@
return result.get(key);
}
- private static FilesetOutputSymlink symlink(String from, Artifact to) {
- return FilesetOutputSymlink.createForTesting(
- PathFragment.create(from), to.getPath().asFragment());
+ private FilesetOutputSymlink symlink(String from, Artifact to) {
+ return symlink(PathFragment.create(from), to.getPath().asFragment());
}
- private static FilesetOutputSymlink symlink(String from, String to) {
- return FilesetOutputSymlink.createForTesting(
- PathFragment.create(from), PathFragment.create(to));
+ private FilesetOutputSymlink symlink(String from, String to) {
+ return symlink(PathFragment.create(from), PathFragment.create(to));
}
- private static FilesetOutputSymlink symlink(String from, RootedPath to) {
- return FilesetOutputSymlink.createForTesting(
- PathFragment.create(from), to.asPath().asFragment());
+ private FilesetOutputSymlink symlink(String from, RootedPath to) {
+ return symlink(PathFragment.create(from), to.asPath().asFragment());
+ }
+
+ private FilesetOutputSymlink symlink(PathFragment from, PathFragment to) {
+ return FilesetOutputSymlink.createForTesting(from, to, rootDirectory.asFragment());
}
private void assertSymlinksCreatedInOrder(
FilesetTraversalParams request, FilesetOutputSymlink... expectedSymlinks) throws Exception {
- List<FilesetOutputSymlink> expected = Arrays.asList(expectedSymlinks);
Collection<FilesetOutputSymlink> actual =
Collections2.transform(
evalFilesetTraversal(request).getSymlinks(),
// Strip the metadata from the actual results.
(input) ->
- FilesetOutputSymlink.createForTesting(input.getName(), input.getTargetPath()));
- assertThat(actual).containsExactlyElementsIn(expected).inOrder();
+ FilesetOutputSymlink.createAlreadyRelativizedForTesting(
+ input.getName(), input.getTargetPath(), input.isRelativeToExecRoot()));
+ assertThat(actual).containsExactlyElementsIn(expectedSymlinks).inOrder();
}
private static Label label(String label) throws Exception {
