diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 9dc1bca45b00f4..efb330238f1a22 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -95,10 +95,8 @@ java_library( "ExecException.java", "Executor.java", "FailAction.java", - "FilesetManifest.java", "FilesetTraversalParams.java", "FilesetTraversalParamsFactory.java", - "ForbiddenActionInputException.java", "FullSpawnMetrics.java", "HasDigest.java", "InputFileErrorException.java", @@ -154,7 +152,6 @@ java_library( ":commandline_item", ":execution_requirements", ":file_metadata", - ":fileset_output_symlink", ":fileset_output_tree", ":localhost_capacity", ":middleman_type", @@ -205,7 +202,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", - "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/com/google/devtools/common/options", "//src/main/java/net/starlark/java/eval", @@ -403,7 +399,13 @@ java_library( name = "fileset_output_tree", srcs = ["FilesetOutputTree.java"], deps = [ + ":file_metadata", ":fileset_output_symlink", + ":forbidden_action_input_exception", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", + "//third_party:flogger", "//third_party:guava", ], ) @@ -421,6 +423,11 @@ java_library( ], ) +java_library( + name = "forbidden_action_input_exception", + srcs = ["ForbiddenActionInputException.java"], +) + java_library( name = "runfiles_metadata", srcs = ["RunfilesArtifactValue.java"], diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java index 39c2bd21d59dde..871ee2db550971 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java @@ -18,13 +18,13 @@ import static com.google.common.base.Preconditions.checkState; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; -import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError; +import com.google.devtools.build.lib.actions.FilesetOutputTree.FilesetManifest; +import com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehaviorWithoutError; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; @@ -174,10 +174,9 @@ public void visitArtifacts(Iterable artifacts, ArtifactReceiver receiv } private void visitFileset(Artifact filesetArtifact, ArtifactReceiver receiver) { - ImmutableList links = filesets.get(filesetArtifact).symlinks(); + FilesetOutputTree filesetOutput = filesets.get(filesetArtifact); FilesetManifest filesetManifest = - FilesetManifest.constructFilesetManifestWithoutError( - links, + filesetOutput.constructFilesetManifestWithoutError( PathFragment.EMPTY_FRAGMENT, fullyResolveFilesetLinks ? RelativeSymlinkBehaviorWithoutError.RESOLVE_FULLY diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetManifest.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetManifest.java deleted file mode 100644 index c8eb1a3653e4a9..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/actions/FilesetManifest.java +++ /dev/null @@ -1,298 +0,0 @@ -// Copyright 2017 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.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; - -import com.google.common.flogger.GoogleLogger; -import com.google.devtools.build.lib.vfs.DigestHashFunction; -import com.google.devtools.build.lib.vfs.FileSystemUtils; -import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; -import java.io.IOException; -import java.util.Collections; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; - -/** - * Representation of a Fileset manifest. - */ -public final class FilesetManifest { - private static final int MAX_SYMLINK_TRAVERSALS = 256; - private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); - - /** Mode that determines how to handle relative target paths. */ - public enum RelativeSymlinkBehavior { - /** Ignore any relative target paths. */ - IGNORE, - - /** Give an error if a relative target path is encountered. */ - ERROR, - - /** Resolve all relative target paths. */ - RESOLVE, - - /** Fully resolve all relative paths, even those pointing to internal directories. */ - RESOLVE_FULLY - } - - /** - * Shadow of {@link RelativeSymlinkBehavior} without the {@link RelativeSymlinkBehavior#ERROR} - * value for callers who know there won't be an error thrown when constructing the manifest. - */ - public enum RelativeSymlinkBehaviorWithoutError { - /** Ignore any relative target paths. */ - IGNORE(RelativeSymlinkBehavior.IGNORE), - - /** Resolve all relative target paths. */ - RESOLVE(RelativeSymlinkBehavior.RESOLVE), - - /** Fully resolve all relative paths, even those pointing to internal directories. */ - RESOLVE_FULLY(RelativeSymlinkBehavior.RESOLVE_FULLY); - - private final RelativeSymlinkBehavior target; - - RelativeSymlinkBehaviorWithoutError(RelativeSymlinkBehavior target) { - this.target = target; - } - } - - /** - * Constructs a FilesetManifest from the given {@code outputSymlinks}, processing relative - * symlinks according to {@code relSymlinkBehavior}. Use when {@link - * RelativeSymlinkBehavior#ERROR} is guaranteed not to be the behavior. - */ - public static FilesetManifest constructFilesetManifestWithoutError( - List outputSymlinks, - PathFragment targetPrefix, - RelativeSymlinkBehaviorWithoutError relSymlinkBehavior) { - try { - return constructFilesetManifest(outputSymlinks, targetPrefix, relSymlinkBehavior.target); - } catch (ForbiddenRelativeSymlinkException e) { - throw new IllegalStateException( - "Can't throw forbidden symlink exception unless behavior is ERROR: " - + relSymlinkBehavior - + ", " - + targetPrefix - + ", " - + outputSymlinks, - e); - } - } - - public static FilesetManifest constructFilesetManifest( - List outputSymlinks, - PathFragment targetPrefix, - RelativeSymlinkBehavior relSymlinkBehavior) - throws ForbiddenRelativeSymlinkException { - LinkedHashMap entries = new LinkedHashMap<>(); - Map relativeLinks = new HashMap<>(); - Map artifactValues = new HashMap<>(); - for (FilesetOutputSymlink outputSymlink : outputSymlinks) { - PathFragment fullLocation = targetPrefix.getRelative(outputSymlink.getName()); - String targetPath = outputSymlink.getTargetPath().getPathString(); - if (isRelativeSymlink(outputSymlink)) { - addRelativeSymlinkEntry(targetPath, fullLocation, relSymlinkBehavior, relativeLinks); - } else { - // Symlinks are already deduplicated by name in SkyframeFilesetManifestAction. - checkState( - entries.put(fullLocation, targetPath) == null, - "Duplicate fileset entry at %s", - fullLocation); - } - if (outputSymlink.getMetadata() instanceof FileArtifactValue) { - artifactValues.put(targetPath, (FileArtifactValue) outputSymlink.getMetadata()); - } - } - resolveRelativeSymlinks(entries, relativeLinks, targetPrefix.isAbsolute(), relSymlinkBehavior); - return new FilesetManifest(entries, artifactValues); - } - - private static boolean isRelativeSymlink(FilesetOutputSymlink symlink) { - return !symlink.getTargetPath().isAbsolute() && !symlink.isRelativeToExecRoot(); - } - - /** Potentially adds the relative symlink to the map, depending on {@code relSymlinkBehavior}. */ - private static void addRelativeSymlinkEntry( - String targetPath, - PathFragment fullLocation, - RelativeSymlinkBehavior relSymlinkBehavior, - Map relativeLinks) - throws ForbiddenRelativeSymlinkException { - switch (relSymlinkBehavior) { - case ERROR -> throw new ForbiddenRelativeSymlinkException(targetPath); - case RESOLVE, RESOLVE_FULLY -> - checkState( - relativeLinks.put(fullLocation, targetPath) == null, - "Duplicate fileset entry at %s", - fullLocation); - case IGNORE -> { - // Do nothing. - } - } - } - - /** Fully resolve relative symlinks including internal directory symlinks. */ - private static void fullyResolveRelativeSymlinks( - Map entries, - Map relativeLinks, - boolean absolute) { - try { - // Construct an in-memory Filesystem containing all the non-relative-symlink entries in the - // Fileset. Treat these as regular files in the filesystem whose contents are the "real" - // symlink - // pointing out of the Fileset. For relative symlinks, we encode these as symlinks in the - // in-memory Filesystem. This allows us to then crawl the filesystem for files. Any readable - // file is a valid part of the FilesetManifest. Dangling internal links or symlink cycles will - // be discovered by the in-memory filesystem. - // (Choice of digest function is irrelevant). - InMemoryFileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256); - Path root = fs.getPath("/"); - for (Map.Entry e : entries.entrySet()) { - PathFragment location = e.getKey(); - Path locationPath = root.getRelative(location); - locationPath.getParentDirectory().createDirectoryAndParents(); - FileSystemUtils.writeContentAsLatin1(locationPath, e.getValue()); - } - for (Map.Entry e : relativeLinks.entrySet()) { - PathFragment location = e.getKey(); - Path locationPath = fs.getPath("/").getRelative(location); - PathFragment value = PathFragment.create(checkNotNull(e.getValue(), e)); - checkState(!value.isAbsolute(), e); - - locationPath.getParentDirectory().createDirectoryAndParents(); - locationPath.createSymbolicLink(value); - } - - addSymlinks(root, entries, absolute); - } catch (IOException e) { - throw new IllegalStateException("InMemoryFileSystem can't throw", e); - } - } - - private static void addSymlinks(Path root, Map entries, boolean absolute) - throws IOException { - for (Path path : root.getDirectoryEntries()) { - try { - if (path.isDirectory()) { - addSymlinks(path, entries, absolute); - } else { - String contents = new String(FileSystemUtils.readContentAsLatin1(path)); - entries.put(absolute ? path.asFragment() : path.asFragment().toRelative(), contents); - } - } catch (IOException e) { - logger.atWarning().log("Symlink %s is dangling or cyclic: %s", path, e.getMessage()); - } - } - } - - /** - * Resolves relative symlinks and puts them in the {@code entries} map. - * - *

Note that {@code relativeLinks} should only contain entries in {@link - * RelativeSymlinkBehavior#RESOLVE} or {@link RelativeSymlinkBehavior#RESOLVE_FULLY} mode. - */ - private static void resolveRelativeSymlinks( - Map entries, - Map relativeLinks, - boolean absolute, - RelativeSymlinkBehavior relSymlinkBehavior) { - if (relSymlinkBehavior == RelativeSymlinkBehavior.RESOLVE_FULLY && !relativeLinks.isEmpty()) { - fullyResolveRelativeSymlinks(entries, relativeLinks, absolute); - } else if (relSymlinkBehavior == RelativeSymlinkBehavior.RESOLVE) { - for (Map.Entry e : relativeLinks.entrySet()) { - PathFragment location = e.getKey(); - String value = e.getValue(); - String actual = checkNotNull(value, e); - checkState(!actual.startsWith("/"), e); - PathFragment actualLocation = location; - - // Recursively resolve relative symlinks. - LinkedHashSet seen = new LinkedHashSet<>(); - int traversals = 0; - do { - actualLocation = actualLocation.getParentDirectory().getRelative(actual); - actual = relativeLinks.get(actualLocation); - } while (++traversals <= MAX_SYMLINK_TRAVERSALS && actual != null && seen.add(actual)); - - if (traversals >= MAX_SYMLINK_TRAVERSALS) { - logger.atWarning().log( - "Symlink %s is part of a chain of length at least %d" - + " which exceeds Blaze's maximum allowable symlink chain length", - location, traversals); - } else if (actual != null) { - // TODO(b/113128395): throw here. - logger.atWarning().log("Symlink %s forms a symlink cycle: %s", location, seen); - } else if (!entries.containsKey(actualLocation)) { - // We've found a relative symlink that points out of the fileset. We should really always - // throw here, but current behavior is that we tolerate such symlinks when they occur in - // runfiles, which is the only time this code is hit. - // TODO(b/113128395): throw here. - logger.atWarning().log( - "Symlink %s (transitively) points to %s" - + " that is not in this fileset (or was pruned because of a cycle)", - location, actualLocation); - } else { - // We have successfully resolved the symlink. - entries.put(location, entries.get(actualLocation)); - } - } - } - } - - private final Map entries; - private final Map artifactValues; - - private FilesetManifest( - Map entries, Map artifactValues) { - this.entries = Collections.unmodifiableMap(entries); - this.artifactValues = artifactValues; - } - - /** - * Returns a mapping of symlink name to its target path. - * - *

Values in this map can be: - * - *

    - *
  • An absolute path. - *
  • A relative path, which should be considered relative to the exec root. - *
- */ - public Map getEntries() { - return entries; - } - - /** - * Returns a mapping of target path to {@link FileArtifactValue}. - * - *

The keyset of this map is a subset of the values in the map returned by {@link #getEntries}. - */ - public Map getArtifactValues() { - return artifactValues; - } - - /** Exception indicating that a relative symlink was encountered but not permitted. */ - public static final class ForbiddenRelativeSymlinkException - extends ForbiddenActionInputException { - private ForbiddenRelativeSymlinkException(String symlinkTarget) { - super("Fileset symlink " + symlinkTarget + " is not absolute"); - } - } -} diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputTree.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputTree.java index 9cdffebbfcf451..ba5b229bf77c57 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputTree.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputTree.java @@ -14,14 +14,30 @@ package com.google.devtools.build.lib.actions; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; +import com.google.common.flogger.GoogleLogger; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; /** A collection of {@link FilesetOutputSymlink}s comprising the output tree of a fileset. */ public final class FilesetOutputTree { + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + public static final FilesetOutputTree EMPTY = new FilesetOutputTree(ImmutableList.of()); + private static final int MAX_SYMLINK_TRAVERSALS = 256; public static FilesetOutputTree create(ImmutableList symlinks) { return symlinks.isEmpty() ? EMPTY : new FilesetOutputTree(symlinks); @@ -33,6 +49,57 @@ private FilesetOutputTree(ImmutableList symlinks) { this.symlinks = checkNotNull(symlinks); } + /** + * Constructs a {@link FilesetManifest} for this fileset tree, processing relative symlinks + * according to {@code relSymlinkBehavior}. Use when {@link RelativeSymlinkBehavior#ERROR} is + * guaranteed not to be the behavior. + */ + public FilesetManifest constructFilesetManifestWithoutError( + PathFragment targetPrefix, RelativeSymlinkBehaviorWithoutError relSymlinkBehavior) { + try { + return constructFilesetManifest(targetPrefix, relSymlinkBehavior.target); + } catch (ForbiddenRelativeSymlinkException e) { + throw new IllegalStateException( + "Can't throw forbidden symlink exception unless behavior is ERROR: " + + relSymlinkBehavior + + ", " + + targetPrefix + + ", " + + symlinks, + e); + } + } + + /** + * Constructs a {@link FilesetManifest} for this fileset tree, processing relative symlinks + * according to {@code relSymlinkBehavior}. + */ + public FilesetManifest constructFilesetManifest( + PathFragment targetPrefix, RelativeSymlinkBehavior relSymlinkBehavior) + throws ForbiddenRelativeSymlinkException { + LinkedHashMap entries = new LinkedHashMap<>(); + Map relativeLinks = new HashMap<>(); + Map artifactValues = new HashMap<>(); + for (FilesetOutputSymlink outputSymlink : symlinks) { + PathFragment fullLocation = targetPrefix.getRelative(outputSymlink.getName()); + String targetPath = outputSymlink.getTargetPath().getPathString(); + if (isRelativeSymlink(outputSymlink)) { + addRelativeSymlinkEntry(targetPath, fullLocation, relSymlinkBehavior, relativeLinks); + } else { + // Symlinks are already deduplicated by name in SkyframeFilesetManifestAction. + checkState( + entries.put(fullLocation, targetPath) == null, + "Duplicate fileset entry at %s", + fullLocation); + } + if (outputSymlink.getMetadata() instanceof FileArtifactValue) { + artifactValues.put(targetPath, (FileArtifactValue) outputSymlink.getMetadata()); + } + } + resolveRelativeSymlinks(entries, relativeLinks, targetPrefix.isAbsolute(), relSymlinkBehavior); + return new FilesetManifest(entries, artifactValues); + } + public ImmutableList symlinks() { return symlinks; } @@ -65,4 +132,218 @@ public boolean equals(Object o) { public String toString() { return MoreObjects.toStringHelper(this).add("symlinks", symlinks).toString(); } + + /** Mode that determines how to handle relative target paths. */ + public enum RelativeSymlinkBehavior { + /** Ignore any relative target paths. */ + IGNORE, + + /** Give an error if a relative target path is encountered. */ + ERROR, + + /** Resolve all relative target paths. */ + RESOLVE, + + /** Fully resolve all relative paths, even those pointing to internal directories. */ + RESOLVE_FULLY + } + + /** + * Shadow of {@link RelativeSymlinkBehavior} without the {@link RelativeSymlinkBehavior#ERROR} + * value for callers who know there won't be an error thrown when constructing the manifest. + */ + public enum RelativeSymlinkBehaviorWithoutError { + /** Ignore any relative target paths. */ + IGNORE(RelativeSymlinkBehavior.IGNORE), + + /** Resolve all relative target paths. */ + RESOLVE(RelativeSymlinkBehavior.RESOLVE), + + /** Fully resolve all relative paths, even those pointing to internal directories. */ + RESOLVE_FULLY(RelativeSymlinkBehavior.RESOLVE_FULLY); + + private final RelativeSymlinkBehavior target; + + RelativeSymlinkBehaviorWithoutError(RelativeSymlinkBehavior target) { + this.target = target; + } + } + + private static boolean isRelativeSymlink(FilesetOutputSymlink symlink) { + return !symlink.getTargetPath().isAbsolute() && !symlink.isRelativeToExecRoot(); + } + + /** Potentially adds the relative symlink to the map, depending on {@code relSymlinkBehavior}. */ + private static void addRelativeSymlinkEntry( + String targetPath, + PathFragment fullLocation, + RelativeSymlinkBehavior relSymlinkBehavior, + Map relativeLinks) + throws ForbiddenRelativeSymlinkException { + switch (relSymlinkBehavior) { + case ERROR -> throw new ForbiddenRelativeSymlinkException(targetPath); + case RESOLVE, RESOLVE_FULLY -> + checkState( + relativeLinks.put(fullLocation, targetPath) == null, + "Duplicate fileset entry at %s", + fullLocation); + case IGNORE -> {} + } + } + + /** Fully resolve relative symlinks including internal directory symlinks. */ + private static void fullyResolveRelativeSymlinks( + Map entries, + Map relativeLinks, + boolean absolute) { + try { + // Construct an in-memory Filesystem containing all the non-relative-symlink entries in the + // Fileset. Treat these as regular files in the filesystem whose contents are the "real" + // symlink pointing out of the Fileset. For relative symlinks, we encode these as symlinks in + // the in-memory Filesystem. This allows us to then crawl the filesystem for files. Any + // readable file is a valid part of the FilesetManifest. Dangling internal links or symlink + // cycles will be discovered by the in-memory filesystem. + // (Choice of digest function is irrelevant). + InMemoryFileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256); + Path root = fs.getPath("/"); + for (Map.Entry e : entries.entrySet()) { + PathFragment location = e.getKey(); + Path locationPath = root.getRelative(location); + locationPath.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContentAsLatin1(locationPath, e.getValue()); + } + for (Map.Entry e : relativeLinks.entrySet()) { + PathFragment location = e.getKey(); + Path locationPath = fs.getPath("/").getRelative(location); + PathFragment value = PathFragment.create(checkNotNull(e.getValue(), e)); + checkState(!value.isAbsolute(), e); + + locationPath.getParentDirectory().createDirectoryAndParents(); + locationPath.createSymbolicLink(value); + } + + addSymlinks(root, entries, absolute); + } catch (IOException e) { + throw new IllegalStateException("InMemoryFileSystem can't throw", e); + } + } + + private static void addSymlinks(Path root, Map entries, boolean absolute) + throws IOException { + for (Path path : root.getDirectoryEntries()) { + try { + if (path.isDirectory()) { + addSymlinks(path, entries, absolute); + } else { + String contents = new String(FileSystemUtils.readContentAsLatin1(path)); + entries.put(absolute ? path.asFragment() : path.asFragment().toRelative(), contents); + } + } catch (IOException e) { + logger.atWarning().log("Symlink %s is dangling or cyclic: %s", path, e.getMessage()); + } + } + } + + /** + * Resolves relative symlinks and puts them in the {@code entries} map. + * + *

Note that {@code relativeLinks} should only contain entries in {@link + * RelativeSymlinkBehavior#RESOLVE} or {@link RelativeSymlinkBehavior#RESOLVE_FULLY} mode. + */ + private static void resolveRelativeSymlinks( + Map entries, + Map relativeLinks, + boolean absolute, + RelativeSymlinkBehavior relSymlinkBehavior) { + if (relativeLinks.isEmpty()) { + return; + } + if (relSymlinkBehavior == RelativeSymlinkBehavior.RESOLVE_FULLY) { + fullyResolveRelativeSymlinks(entries, relativeLinks, absolute); + } else if (relSymlinkBehavior == RelativeSymlinkBehavior.RESOLVE) { + for (Map.Entry e : relativeLinks.entrySet()) { + PathFragment location = e.getKey(); + String actual = e.getValue(); + checkState(!actual.startsWith("/"), e); + PathFragment actualLocation = location; + + // Recursively resolve relative symlinks. + LinkedHashSet seen = new LinkedHashSet<>(); + int traversals = 0; + do { + actualLocation = actualLocation.getParentDirectory().getRelative(actual); + actual = relativeLinks.get(actualLocation); + } while (++traversals <= MAX_SYMLINK_TRAVERSALS && actual != null && seen.add(actual)); + + if (traversals >= MAX_SYMLINK_TRAVERSALS) { + logger.atWarning().log( + "Symlink %s is part of a chain of length at least %d" + + " which exceeds Blaze's maximum allowable symlink chain length", + location, traversals); + } else if (actual != null) { + // TODO(b/113128395): throw here. + logger.atWarning().log("Symlink %s forms a symlink cycle: %s", location, seen); + } else { + String resolvedTarget = entries.get(actualLocation); + if (resolvedTarget == null) { + // We've found a relative symlink that points out of the fileset. We should really + // always throw here, but current behavior is that we tolerate such symlinks when they + // occur in runfiles, which is the only time this code is hit. + // TODO(b/113128395): throw here. + logger.atWarning().log( + "Symlink %s (transitively) points to %s that is not in this fileset (or was pruned" + + " because of a cycle)", + location, actualLocation); + } else { + // We have successfully resolved the symlink. + entries.put(location, resolvedTarget); + } + } + } + } + } + + /** Representation of a Fileset manifest. */ + public static final class FilesetManifest { + private final Map entries; + private final Map artifactValues; + + private FilesetManifest( + Map entries, Map artifactValues) { + this.entries = Collections.unmodifiableMap(entries); + this.artifactValues = artifactValues; + } + + /** + * Returns a mapping of symlink name to its target path. + * + *

Values in this map can be: + * + *

    + *
  • An absolute path. + *
  • A relative path, which should be considered relative to the exec root. + *
+ */ + public Map getEntries() { + return entries; + } + + /** + * Returns a mapping of target path to {@link FileArtifactValue}. + * + *

The keyset of this map is a subset of the values in the map returned by {@link + * #getEntries}. + */ + public Map getArtifactValues() { + return artifactValues; + } + } + + /** Exception indicating that a relative symlink was encountered but not permitted. */ + public static final class ForbiddenRelativeSymlinkException + extends ForbiddenActionInputException { + private ForbiddenRelativeSymlinkException(String symlinkTarget) { + super("Fileset symlink " + symlinkTarget + " is not absolute"); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index aed3d9a30245e5..0ea9f816b18128 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -2563,7 +2563,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java index c7d6c028857949..5697cb61f33120 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java @@ -37,9 +37,9 @@ import com.google.devtools.build.lib.actions.CommandLineLimits; import com.google.devtools.build.lib.actions.CommandLines; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; -import com.google.devtools.build.lib.actions.FilesetManifest; -import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; +import com.google.devtools.build.lib.actions.FilesetOutputTree.FilesetManifest; +import com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehaviorWithoutError; import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.SingleStringArgFormatter; import com.google.devtools.build.lib.analysis.actions.PathMappers; @@ -488,20 +488,19 @@ private static void expandFileset( List expandedValues, PathMapper pathMapper) throws CommandLineExpansionException { - ImmutableList expandedFileSet; + FilesetOutputTree filesetOutput; try { - expandedFileSet = artifactExpander.expandFileset(fileset).symlinks(); + filesetOutput = artifactExpander.expandFileset(fileset); } catch (MissingExpansionException e) { throw new CommandLineExpansionException( String.format( - "Could not expand fileset: %s. Did you forget to add it as an input of the" - + " action?", + "Could not expand fileset: %s. Did you forget to add it as an input of the action?", fileset), e); } FilesetManifest filesetManifest = - FilesetManifest.constructFilesetManifestWithoutError( - expandedFileSet, fileset.getExecPath(), RelativeSymlinkBehaviorWithoutError.IGNORE); + filesetOutput.constructFilesetManifestWithoutError( + fileset.getExecPath(), RelativeSymlinkBehaviorWithoutError.IGNORE); for (PathFragment relativePath : filesetManifest.getEntries().keySet()) { PathFragment mappedRelativePath = pathMapper.map(relativePath); expandedValues.add(new FilesetSymlinkFile(fileset, mappedRelativePath)); diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD index ac6bc13fb9fbec..04abf3062b5926 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -26,6 +26,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/remote/common:bulk_transfer_exception", @@ -231,6 +232,7 @@ java_library( deps = [ ":spawn_runner", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/profiler", ], ) @@ -255,6 +257,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/vfs", @@ -330,6 +333,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/util/io", 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 9f145a4a234942..d274cda9b4c831 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 @@ -27,10 +27,10 @@ import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactExpander; import com.google.devtools.build.lib.actions.ArtifactExpander.MissingExpansionException; -import com.google.devtools.build.lib.actions.FilesetManifest; -import com.google.devtools.build.lib.actions.FilesetManifest.ForbiddenRelativeSymlinkException; -import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior; import com.google.devtools.build.lib.actions.FilesetOutputTree; +import com.google.devtools.build.lib.actions.FilesetOutputTree.FilesetManifest; +import com.google.devtools.build.lib.actions.FilesetOutputTree.ForbiddenRelativeSymlinkException; +import com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehavior; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.PathMapper; @@ -189,8 +189,7 @@ private void addFilesetManifest( throws ForbiddenRelativeSymlinkException { Preconditions.checkArgument(filesetArtifact.isFileset(), filesetArtifact); FilesetManifest filesetManifest = - FilesetManifest.constructFilesetManifest( - filesetOutput.symlinks(), location, relSymlinkBehavior); + filesetOutput.constructFilesetManifest(location, relSymlinkBehavior); for (Map.Entry mapping : filesetManifest.getEntries().entrySet()) { String value = mapping.getValue(); diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/BUILD b/src/main/java/com/google/devtools/build/lib/exec/local/BUILD index 6a0756132f9c5c..25f740a1044b1f 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/local/BUILD @@ -28,6 +28,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/actions:resource_manager", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", "//src/main/java/com/google/devtools/build/lib/concurrent", diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index c116983b46c4dc..bfcb485938c5b2 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -66,6 +66,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options", diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD index 05085fb0e77b9b..06ce10c17c339b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD @@ -51,14 +51,13 @@ java_library( ), deps = [ "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", - "//third_party:auto_value", "//third_party:guava", "//third_party:jsr305", "//third_party/protobuf:protobuf_java", diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD index 5f28b59fa3bd84..8e91491513c643 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD +++ b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD @@ -103,6 +103,7 @@ java_library( ":windows_sandbox", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/exec:execution_options", "//src/main/java/com/google/devtools/build/lib/exec:runfiles_tree_updater", @@ -152,6 +153,7 @@ java_library( ":sandboxed_spawns", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/actions:resource_manager", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/exec:bin_tools", @@ -166,7 +168,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/protobuf:failure_details_java_proto", "//third_party:guava", - "//third_party:jsr305", ], ) @@ -182,6 +183,7 @@ java_library( ":sandboxed_spawns", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy", "//src/main/java/com/google/devtools/build/lib/exec:execution_options", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", @@ -214,6 +216,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy", "//src/main/java/com/google/devtools/build/lib/exec:execution_options", @@ -274,6 +277,7 @@ java_library( ":sandboxed_spawns", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy", "//src/main/java/com/google/devtools/build/lib/exec:execution_options", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", @@ -303,6 +307,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy", "//src/main/java/com/google/devtools/build/lib/exec:execution_options", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", @@ -330,6 +335,7 @@ java_library( ":sandboxed_spawns", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/analysis/platform:platform_utils", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java index 2edc4660e42f55..a334dad6ff783d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java @@ -23,9 +23,9 @@ import com.google.devtools.build.lib.actions.ActionInputMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.FilesetManifest; -import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError; import com.google.devtools.build.lib.actions.FilesetOutputTree; +import com.google.devtools.build.lib.actions.FilesetOutputTree.FilesetManifest; +import com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehaviorWithoutError; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.RunfilesArtifactValue; import com.google.devtools.build.lib.actions.RunfilesTree; @@ -72,8 +72,8 @@ private static ImmutableMap createFilesetMapping( Map filesetMap = new HashMap<>(); for (FilesetOutputTree filesetOutput : filesets.values()) { FilesetManifest manifest = - FilesetManifest.constructFilesetManifestWithoutError( - filesetOutput.symlinks(), execRoot, RelativeSymlinkBehaviorWithoutError.RESOLVE); + filesetOutput.constructFilesetManifestWithoutError( + execRoot, RelativeSymlinkBehaviorWithoutError.RESOLVE); manifest .getArtifactValues() .forEach( diff --git a/src/main/java/com/google/devtools/build/lib/worker/BUILD b/src/main/java/com/google/devtools/build/lib/worker/BUILD index f72809c88fb02c..5396c07b066feb 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/BUILD +++ b/src/main/java/com/google/devtools/build/lib/worker/BUILD @@ -63,6 +63,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:resource_manager", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", diff --git a/src/test/java/com/google/devtools/build/lib/actions/FilesetOutputTreeTest.java b/src/test/java/com/google/devtools/build/lib/actions/FilesetOutputTreeTest.java new file mode 100644 index 00000000000000..21b4f04f8fe8e0 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/actions/FilesetOutputTreeTest.java @@ -0,0 +1,278 @@ +// Copyright 2017 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 static com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehavior.ERROR; +import static com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehavior.IGNORE; +import static com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehavior.RESOLVE_FULLY; +import static org.junit.Assert.assertThrows; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.actions.FilesetOutputTree.FilesetManifest; +import com.google.devtools.build.lib.actions.FilesetOutputTree.ForbiddenRelativeSymlinkException; +import com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehavior; +import com.google.devtools.build.lib.actions.FilesetOutputTreeTest.ManifestCommonTests; +import com.google.devtools.build.lib.actions.FilesetOutputTreeTest.OneOffManifestTests; +import com.google.devtools.build.lib.actions.FilesetOutputTreeTest.ResolvingManifestTests; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.testing.junit.testparameterinjector.TestParameter; +import com.google.testing.junit.testparameterinjector.TestParameterInjector; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.junit.runners.Suite; + +/** Tests for {@link FilesetOutputTree}. */ +@RunWith(Suite.class) +@Suite.SuiteClasses({ + ManifestCommonTests.class, + OneOffManifestTests.class, + ResolvingManifestTests.class +}) +public final class FilesetOutputTreeTest { + + private static final PathFragment EXEC_ROOT = PathFragment.create("/root"); + + private static FilesetOutputSymlink symlink(String from, String to) { + return FilesetOutputSymlink.createForTesting( + PathFragment.create(from), PathFragment.create(to), EXEC_ROOT); + } + + private static FilesetOutputTree createTree(FilesetOutputSymlink... symlinks) { + return FilesetOutputTree.create(ImmutableList.copyOf(symlinks)); + } + + /** Manifest tests that apply to all relative symlink behavior. */ + @RunWith(TestParameterInjector.class) + public static final class ManifestCommonTests { + + @TestParameter private RelativeSymlinkBehavior behavior; + + @Test + public void emptyManifest() throws Exception { + FilesetManifest manifest = + FilesetOutputTree.EMPTY.constructFilesetManifest( + PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()).isEmpty(); + assertThat(manifest.getArtifactValues()).isEmpty(); + } + + @Test + public void manifestWithSingleFile() throws Exception { + FilesetOutputTree filesetOutput = createTree(symlink("bar", "/dir/file")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()) + .containsExactly(PathFragment.create("out/foo/bar"), "/dir/file"); + } + + @Test + public void manifestWithTwoFiles() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("bar", "/dir/file"), symlink("baz", "/dir/file")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()) + .containsExactly( + PathFragment.create("out/foo/bar"), "/dir/file", + PathFragment.create("out/foo/baz"), "/dir/file"); + } + + @Test + public void manifestWithDirectory() throws Exception { + FilesetOutputTree filesetOutput = createTree(symlink("bar", "/some")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()) + .containsExactly(PathFragment.create("out/foo/bar"), "/some"); + } + + @Test + public void manifestWithExecRootRelativePath() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("bar", EXEC_ROOT.getRelative("foo/bar").getPathString())); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()) + .containsExactly(PathFragment.create("out/foo/bar"), "foo/bar"); + } + } + + /** Manifest tests that apply to a specific relative symlink behavior. */ + @RunWith(JUnit4.class) + public static final class OneOffManifestTests { + + @Test + public void canonicalEmptyInstance() { + assertThat(createTree()).isSameInstanceAs(FilesetOutputTree.EMPTY); + } + + @Test + public void manifestWithErrorOnRelativeSymlink() { + FilesetOutputTree filesetOutput = + createTree(symlink("bar", "foo"), symlink("foo", "/foo/bar")); + + var e = + assertThrows( + ForbiddenRelativeSymlinkException.class, + () -> filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), ERROR)); + assertThat(e).hasMessageThat().contains("Fileset symlink foo is not absolute"); + } + + @Test + public void manifestWithIgnoredRelativeSymlink() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("bar", "foo"), symlink("foo", "/foo/bar")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), IGNORE); + + assertThat(manifest.getEntries()) + .containsExactly(PathFragment.create("out/foo/foo"), "/foo/bar"); + } + + @Test + public void manifestWithResolvedRelativeDirectorySymlink() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("foo/subdir/f1", "/foo/subdir/f1"), symlink("foo/bar", "subdir")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out"), RESOLVE_FULLY); + + assertThat(manifest.getEntries()) + .containsExactly( + PathFragment.create("out/foo/subdir/f1"), "/foo/subdir/f1", + PathFragment.create("out/foo/bar/f1"), "/foo/subdir/f1"); + } + } + + /** Manifest tests that apply resolving relative symlink behavior. */ + @RunWith(TestParameterInjector.class) + public static final class ResolvingManifestTests { + + @TestParameter({"RESOLVE", "RESOLVE_FULLY"}) + private RelativeSymlinkBehavior behavior; + + @Test + public void manifestWithResolvedRelativeSymlink() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("bar", "foo"), symlink("foo", "/foo/bar")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()) + .containsExactly( + PathFragment.create("out/foo/bar"), "/foo/bar", + PathFragment.create("out/foo/foo"), "/foo/bar"); + } + + @Test + public void manifestWithResolvedRelativeSymlinkWithDotSlash() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("bar", "./foo"), symlink("foo", "/foo/bar")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()) + .containsExactly( + PathFragment.create("out/foo/bar"), "/foo/bar", + PathFragment.create("out/foo/foo"), "/foo/bar"); + } + + @Test + public void manifestWithSymlinkCycle() throws Exception { + FilesetOutputTree filesetOutput = + createTree( + symlink("bar", "foo"), + symlink("foo", "biz"), + symlink("biz", "bar"), + symlink("reg", "/file")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out"), behavior); + + assertThat(manifest.getEntries()).containsExactly(PathFragment.create("out/reg"), "/file"); + } + + @Test + public void unboundedSymlinkDescendant() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("p", "a/b"), symlink("a/b", "../b/c"), symlink("reg", "/file")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out"), behavior); + + assertThat(manifest.getEntries()).containsExactly(PathFragment.create("out/reg"), "/file"); + } + + @Test + public void unboundedSymlinkAncestor() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("a/b", "c/d"), symlink("a/c/d", ".././a"), symlink("reg", "/file")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out"), behavior); + + assertThat(manifest.getEntries()).containsExactly(PathFragment.create("out/reg"), "/file"); + } + + @Test + public void manifestWithResolvedRelativeSymlinkWithDotDotSlash() throws Exception { + FilesetOutputTree filesetOutput = + createTree(symlink("bar/bar", "../foo/foo"), symlink("foo/foo", "/foo/bar")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()) + .containsExactly( + PathFragment.create("out/foo/bar/bar"), "/foo/bar", + PathFragment.create("out/foo/foo/foo"), "/foo/bar"); + } + + @Test + public void manifestWithUnresolvableRelativeSymlink() throws Exception { + FilesetOutputTree filesetOutput = createTree(symlink("bar", "foo")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()).isEmpty(); + assertThat(manifest.getArtifactValues()).isEmpty(); + } + + @Test + public void manifestWithUnresolvableRelativeSymlinkToRelativeSymlink() throws Exception { + FilesetOutputTree filesetOutput = createTree(symlink("bar", "foo"), symlink("foo", "baz")); + + FilesetManifest manifest = + filesetOutput.constructFilesetManifest(PathFragment.create("out/foo"), behavior); + + assertThat(manifest.getEntries()).isEmpty(); + assertThat(manifest.getArtifactValues()).isEmpty(); + } + } +} diff --git a/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java b/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java deleted file mode 100644 index f7632d240e1c93..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java +++ /dev/null @@ -1,294 +0,0 @@ -// Copyright 2017 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.exec; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior.ERROR; -import static com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior.IGNORE; -import static com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior.RESOLVE_FULLY; -import static org.junit.Assert.assertThrows; - -import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.actions.FilesetManifest; -import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; -import com.google.devtools.build.lib.exec.FilesetManifestTest.ManifestCommonTests; -import com.google.devtools.build.lib.exec.FilesetManifestTest.OneOffManifestTests; -import com.google.devtools.build.lib.exec.FilesetManifestTest.ResolvingManifestTests; -import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.testing.junit.testparameterinjector.TestParameter; -import com.google.testing.junit.testparameterinjector.TestParameterInjector; -import java.util.List; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.junit.runners.Suite; - -/** Tests for {@link FilesetManifest}. */ -@RunWith(Suite.class) -@Suite.SuiteClasses({ - ManifestCommonTests.class, - OneOffManifestTests.class, - ResolvingManifestTests.class -}) -public final class FilesetManifestTest { - - private static final PathFragment EXEC_ROOT = PathFragment.create("/root"); - - private static FilesetOutputSymlink filesetSymlink(String from, String to) { - return FilesetOutputSymlink.createForTesting( - PathFragment.create(from), PathFragment.create(to), EXEC_ROOT); - } - - /** Manifest tests that apply to all relative symlink behavior. */ - @RunWith(TestParameterInjector.class) - public static final class ManifestCommonTests { - - @TestParameter private RelativeSymlinkBehavior behavior; - - @Test - public void testEmptyManifest() throws Exception { - List symlinks = ImmutableList.of(); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()).isEmpty(); - } - - @Test - public void testManifestWithSingleFile() throws Exception { - List symlinks = ImmutableList.of(filesetSymlink("bar", "/dir/file")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()) - .containsExactly(PathFragment.create("out/foo/bar"), "/dir/file"); - } - - @Test - public void testManifestWithTwoFiles() throws Exception { - List symlinks = - ImmutableList.of(filesetSymlink("bar", "/dir/file"), filesetSymlink("baz", "/dir/file")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()) - .containsExactly( - PathFragment.create("out/foo/bar"), "/dir/file", - PathFragment.create("out/foo/baz"), "/dir/file"); - } - - @Test - public void testManifestWithDirectory() throws Exception { - List symlinks = ImmutableList.of(filesetSymlink("bar", "/some")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()) - .containsExactly(PathFragment.create("out/foo/bar"), "/some"); - } - - @Test - public void testManifestWithExecRootRelativePath() throws Exception { - List symlinks = - ImmutableList.of(filesetSymlink("bar", EXEC_ROOT.getRelative("foo/bar").getPathString())); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()) - .containsExactly(PathFragment.create("out/foo/bar"), "foo/bar"); - } - } - - /** Manifest tests that apply to a specific relative symlink behavior. */ - @RunWith(JUnit4.class) - public static final class OneOffManifestTests { - - @Test - public void testManifestWithErrorOnRelativeSymlink() { - List symlinks = - ImmutableList.of(filesetSymlink("bar", "foo"), filesetSymlink("foo", "/foo/bar")); - - FilesetManifest.ForbiddenRelativeSymlinkException e = - assertThrows( - FilesetManifest.ForbiddenRelativeSymlinkException.class, - () -> - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), ERROR)); - assertThat(e).hasMessageThat().contains("Fileset symlink foo is not absolute"); - } - - @Test - public void testManifestWithIgnoredRelativeSymlink() throws Exception { - List symlinks = - ImmutableList.of(filesetSymlink("bar", "foo"), filesetSymlink("foo", "/foo/bar")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), IGNORE); - - assertThat(manifest.getEntries()) - .containsExactly(PathFragment.create("out/foo/foo"), "/foo/bar"); - } - - @Test - public void testManifestWithResolvedRelativeDirectorySymlink() throws Exception { - List symlinks = - ImmutableList.of( - filesetSymlink("foo/subdir/f1", "/foo/subdir/f1"), - filesetSymlink("foo/bar", "subdir")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out"), RESOLVE_FULLY); - - assertThat(manifest.getEntries()) - .containsExactly( - PathFragment.create("out/foo/subdir/f1"), "/foo/subdir/f1", - PathFragment.create("out/foo/bar/f1"), "/foo/subdir/f1"); - } - } - - /** Manifest tests that apply resolving relative symlink behavior. */ - @RunWith(TestParameterInjector.class) - public static final class ResolvingManifestTests { - - @TestParameter({"RESOLVE", "RESOLVE_FULLY"}) - private RelativeSymlinkBehavior behavior; - - @Test - public void testManifestWithResolvedRelativeSymlink() throws Exception { - List symlinks = - ImmutableList.of(filesetSymlink("bar", "foo"), filesetSymlink("foo", "/foo/bar")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()) - .containsExactly( - PathFragment.create("out/foo/bar"), "/foo/bar", - PathFragment.create("out/foo/foo"), "/foo/bar"); - } - - @Test - public void testManifestWithResolvedRelativeSymlinkWithDotSlash() throws Exception { - List symlinks = - ImmutableList.of(filesetSymlink("bar", "./foo"), filesetSymlink("foo", "/foo/bar")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()) - .containsExactly( - PathFragment.create("out/foo/bar"), "/foo/bar", - PathFragment.create("out/foo/foo"), "/foo/bar"); - } - - @Test - public void testManifestWithSymlinkCycle() throws Exception { - List symlinks = - ImmutableList.of( - filesetSymlink("bar", "foo"), - filesetSymlink("foo", "biz"), - filesetSymlink("biz", "bar"), - filesetSymlink("reg", "/file")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest(symlinks, PathFragment.create("out"), behavior); - - assertThat(manifest.getEntries()).containsExactly(PathFragment.create("out/reg"), "/file"); - } - - @Test - public void testUnboundedSymlinkDescendant() throws Exception { - List symlinks = - ImmutableList.of( - filesetSymlink("p", "a/b"), - filesetSymlink("a/b", "../b/c"), - filesetSymlink("reg", "/file")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest(symlinks, PathFragment.create("out"), behavior); - - assertThat(manifest.getEntries()).containsExactly(PathFragment.create("out/reg"), "/file"); - } - - @Test - public void testUnboundedSymlinkAncestor() throws Exception { - List symlinks = - ImmutableList.of( - filesetSymlink("a/b", "c/d"), - filesetSymlink("a/c/d", ".././a"), - filesetSymlink("reg", "/file")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest(symlinks, PathFragment.create("out"), behavior); - - assertThat(manifest.getEntries()).containsExactly(PathFragment.create("out/reg"), "/file"); - } - - @Test - public void testManifestWithResolvedRelativeSymlinkWithDotDotSlash() throws Exception { - List symlinks = - ImmutableList.of( - filesetSymlink("bar/bar", "../foo/foo"), filesetSymlink("foo/foo", "/foo/bar")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()) - .containsExactly( - PathFragment.create("out/foo/bar/bar"), "/foo/bar", - PathFragment.create("out/foo/foo/foo"), "/foo/bar"); - } - - @Test - public void testManifestWithUnresolvableRelativeSymlink() throws Exception { - List symlinks = ImmutableList.of(filesetSymlink("bar", "foo")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()).isEmpty(); - assertThat(manifest.getArtifactValues()).isEmpty(); - } - - @Test - public void testManifestWithUnresolvableRelativeSymlinkToRelativeSymlink() throws Exception { - List symlinks = - ImmutableList.of(filesetSymlink("bar", "foo"), filesetSymlink("foo", "baz")); - - FilesetManifest manifest = - FilesetManifest.constructFilesetManifest( - symlinks, PathFragment.create("out/foo"), behavior); - - assertThat(manifest.getEntries()).isEmpty(); - assertThat(manifest.getArtifactValues()).isEmpty(); - } - } -} 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 931a504333cf4c..c0f6b41bf40792 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 @@ -14,9 +14,9 @@ package com.google.devtools.build.lib.exec; import static com.google.common.truth.Truth.assertThat; -import static com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior.ERROR; -import static com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior.IGNORE; -import static com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior.RESOLVE; +import static com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehavior.ERROR; +import static com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehavior.IGNORE; +import static com.google.devtools.build.lib.actions.FilesetOutputTree.RelativeSymlinkBehavior.RESOLVE; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertThrows; @@ -33,9 +33,9 @@ import com.google.devtools.build.lib.actions.ArtifactExpander; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; -import com.google.devtools.build.lib.actions.FilesetManifest; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.FilesetOutputTree; +import com.google.devtools.build.lib.actions.FilesetOutputTree.ForbiddenRelativeSymlinkException; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.RunfilesTree; @@ -508,9 +508,9 @@ public void testManifestWithErrorOnRelativeSymlink() { filesetSymlink("workspace/bar", "foo"), filesetSymlink("workspace/foo", "/root/bar")))); - FilesetManifest.ForbiddenRelativeSymlinkException e = + var e = assertThrows( - FilesetManifest.ForbiddenRelativeSymlinkException.class, + ForbiddenRelativeSymlinkException.class, () -> expander.addFilesetManifests( filesetMappings, inputMap, PathFragment.EMPTY_FRAGMENT)); diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index 8ccb518aa06d2a..df814d74a3c1b6 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -76,6 +76,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_metadata", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/BUILD b/src/test/java/com/google/devtools/build/lib/remote/util/BUILD index 7b4a0ef65af258..376071c288351c 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/util/BUILD @@ -27,6 +27,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:forbidden_action_input_exception", "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/remote",