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