Make Bazel not raise an error when a symlink points to its own ancestor.
This is only an error when the symlink is to be traversed recursively. E.g. `a/a/a/b` is perfectly acceptable if `a/a` is a symlink to `.`, except if we want to recursively traverse into it during a glob or recursive target pattern expansion.
RELNOTES: Blaze now allows symbolic links that point to their own ancestor unless they are traversed recursively by e.g. a //... recursive target pattern or a recursive glob.
PiperOrigin-RevId: 334982640
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
index 7c2f416..55c5ab4 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
@@ -102,6 +102,18 @@
public abstract ImmutableList<RootedPath> logicalChainDuringResolution();
/**
+ * If a symlink pointing back to its own ancestor was encountered during the resolution of this
+ * {@link FileValue}, returns the path to it. Otherwise, returns null.
+ */
+ public abstract ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain();
+
+ /**
+ * If a symlink pointing back to its own ancestor was encountered during the resolution of this
+ * {@link FileValue}, returns the symlinks in the cycle. Otherwise, returns null.
+ */
+ public abstract ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain();
+
+ /**
* Returns the real rooted path of the file, taking ancestor symlinks into account. For example,
* the rooted path ['root']/['a/b'] is really ['root']/['c/b'] if 'a' is a symlink to 'c'. Note
* that ancestor symlinks outside the root boundary are not taken into consideration.
@@ -165,6 +177,8 @@
*/
public static FileValue value(
ImmutableList<RootedPath> logicalChainDuringResolution,
+ ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain,
+ ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain,
RootedPath originalRootedPath,
FileStateValue fileStateValueFromAncestors,
RootedPath realRootedPath,
@@ -205,17 +219,36 @@
if (fileStateValueFromAncestors.getType() == FileStateType.SYMLINK) {
PathFragment symlinkTarget = fileStateValueFromAncestors.getSymlinkTarget();
- return shouldStoreChain
- ? new SymlinkFileValueWithStoredChain(
- realRootedPath, realFileStateValue, logicalChainDuringResolution, symlinkTarget)
- : new SymlinkFileValueWithoutStoredChain(
- realRootedPath, realFileStateValue, symlinkTarget);
+ if (pathToUnboundedAncestorSymlinkExpansionChain != null) {
+ return new SymlinkFileValueWithSymlinkCycle(
+ realRootedPath,
+ realFileStateValue,
+ logicalChainDuringResolution,
+ symlinkTarget,
+ pathToUnboundedAncestorSymlinkExpansionChain,
+ unboundedAncestorSymlinkExpansionChain);
+ } else if (shouldStoreChain) {
+ return new SymlinkFileValueWithStoredChain(
+ realRootedPath, realFileStateValue, logicalChainDuringResolution, symlinkTarget);
+ } else {
+ return new SymlinkFileValueWithoutStoredChain(
+ realRootedPath, realFileStateValue, symlinkTarget);
+ }
+ } else {
+ if (pathToUnboundedAncestorSymlinkExpansionChain != null) {
+ return new DifferentRealPathFileValueWithSymlinkCycle(
+ realRootedPath,
+ realFileStateValue,
+ logicalChainDuringResolution,
+ pathToUnboundedAncestorSymlinkExpansionChain,
+ unboundedAncestorSymlinkExpansionChain);
+ } else if (shouldStoreChain) {
+ return new DifferentRealPathFileValueWithStoredChain(
+ realRootedPath, realFileStateValue, logicalChainDuringResolution);
+ } else {
+ return new DifferentRealPathFileValueWithoutStoredChain(realRootedPath, realFileStateValue);
+ }
}
-
- return shouldStoreChain
- ? new DifferentRealPathFileValueWithStoredChain(
- realRootedPath, realFileStateValue, logicalChainDuringResolution)
- : new DifferentRealPathFileValueWithoutStoredChain(realRootedPath, realFileStateValue);
}
/**
@@ -241,6 +274,16 @@
}
@Override
+ public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
+ return null;
+ }
+
+ @Override
+ public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
+ return null;
+ }
+
+ @Override
public RootedPath realRootedPath() {
return rootedPath;
}
@@ -274,6 +317,84 @@
}
/**
+ * A {@link FileValue} whose resolution required traversing a symlink chain caused by a symlink
+ * pointing to its own ancestor but which eventually points to a real file.
+ */
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec
+ public static class DifferentRealPathFileValueWithSymlinkCycle
+ extends DifferentRealPathFileValueWithStoredChain {
+ // We can't store an exception here because this needs to be serialized, AutoCodec chokes on
+ // object cycles and FilesystemInfiniteSymlinkCycleException somehow sets its cause to itself
+ protected final ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain;
+ protected final ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain;
+
+ public DifferentRealPathFileValueWithSymlinkCycle(
+ RootedPath realRootedPath,
+ FileStateValue realFileStateValue,
+ ImmutableList<RootedPath> logicalChainDuringResolution,
+ ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain,
+ ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain) {
+ super(realRootedPath, realFileStateValue, logicalChainDuringResolution);
+ this.pathToUnboundedAncestorSymlinkExpansionChain =
+ pathToUnboundedAncestorSymlinkExpansionChain;
+ this.unboundedAncestorSymlinkExpansionChain = unboundedAncestorSymlinkExpansionChain;
+ }
+
+ @Override
+ public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
+ return pathToUnboundedAncestorSymlinkExpansionChain;
+ }
+
+ @Override
+ public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
+ return unboundedAncestorSymlinkExpansionChain;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ realRootedPath,
+ realFileStateValue,
+ logicalChainDuringResolution,
+ pathToUnboundedAncestorSymlinkExpansionChain,
+ unboundedAncestorSymlinkExpansionChain);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null) {
+ return false;
+ }
+
+ if (obj.getClass() != DifferentRealPathFileValueWithSymlinkCycle.class) {
+ return false;
+ }
+
+ DifferentRealPathFileValueWithSymlinkCycle other =
+ (DifferentRealPathFileValueWithSymlinkCycle) obj;
+ return realRootedPath.equals(other.realRootedPath)
+ && realFileStateValue.equals(other.realFileStateValue)
+ && logicalChainDuringResolution.equals(other.logicalChainDuringResolution)
+ && pathToUnboundedAncestorSymlinkExpansionChain.equals(
+ other.pathToUnboundedAncestorSymlinkExpansionChain)
+ && unboundedAncestorSymlinkExpansionChain.equals(
+ other.unboundedAncestorSymlinkExpansionChain);
+ }
+
+ @Override
+ public String toString() {
+ return String.format(
+ "symlink ancestor (real_path=%s, real_state=%s, chain=%s, path=%s, cycle=%s)",
+ realRootedPath,
+ realFileStateValue,
+ logicalChainDuringResolution,
+ pathToUnboundedAncestorSymlinkExpansionChain,
+ unboundedAncestorSymlinkExpansionChain);
+ }
+ }
+
+ /**
* Implementation of {@link FileValue} for paths whose fully resolved path is different than the
* requested path, but the path itself is not a symlink. For example, this is the case for the
* path "foo/bar/baz" if at least one of {'foo', 'foo/bar'} is a symlink but 'foo/bar/baz' not.
@@ -310,6 +431,16 @@
}
@Override
+ public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
+ return null;
+ }
+
+ @Override
+ public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
+ return null;
+ }
+
+ @Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
@@ -370,6 +501,16 @@
}
@Override
+ public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
+ return null;
+ }
+
+ @Override
+ public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
+ return null;
+ }
+
+ @Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
@@ -396,12 +537,93 @@
}
}
+ /**
+ * A {@link FileValue} whose resolution required traversing a symlink chain caused by a symlink
+ * pointing to its own ancestor and which eventually points to a symlink.
+ */
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec
+ public static final class SymlinkFileValueWithSymlinkCycle
+ extends SymlinkFileValueWithStoredChain {
+ // We can't store an exception here because this needs to be serialized, AutoCodec chokes on
+ // object cycles and FilesystemInfiniteSymlinkCycleException somehow sets its cause to itself
+ private final ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain;
+ private final ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain;
+
+ public SymlinkFileValueWithSymlinkCycle(
+ RootedPath realRootedPath,
+ FileStateValue realFileStateValue,
+ ImmutableList<RootedPath> logicalChainDuringResolution,
+ PathFragment linkTarget,
+ ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain,
+ ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain) {
+ super(realRootedPath, realFileStateValue, logicalChainDuringResolution, linkTarget);
+ this.pathToUnboundedAncestorSymlinkExpansionChain =
+ pathToUnboundedAncestorSymlinkExpansionChain;
+ this.unboundedAncestorSymlinkExpansionChain = unboundedAncestorSymlinkExpansionChain;
+ }
+
+ @Override
+ public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
+ return pathToUnboundedAncestorSymlinkExpansionChain;
+ }
+
+ @Override
+ public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
+ return unboundedAncestorSymlinkExpansionChain;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ realRootedPath,
+ realFileStateValue,
+ logicalChainDuringResolution,
+ linkTarget,
+ pathToUnboundedAncestorSymlinkExpansionChain,
+ unboundedAncestorSymlinkExpansionChain);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null) {
+ return false;
+ }
+
+ if (obj.getClass() != SymlinkFileValueWithSymlinkCycle.class) {
+ return false;
+ }
+
+ SymlinkFileValueWithSymlinkCycle other = (SymlinkFileValueWithSymlinkCycle) obj;
+ return realRootedPath.equals(other.realRootedPath)
+ && realFileStateValue.equals(other.realFileStateValue)
+ && logicalChainDuringResolution.equals(other.logicalChainDuringResolution)
+ && linkTarget.equals(other.linkTarget)
+ && pathToUnboundedAncestorSymlinkExpansionChain.equals(
+ other.pathToUnboundedAncestorSymlinkExpansionChain)
+ && unboundedAncestorSymlinkExpansionChain.equals(
+ other.unboundedAncestorSymlinkExpansionChain);
+ }
+
+ @Override
+ public String toString() {
+ return String.format(
+ "symlink ancestor (real_path=%s, real_state=%s, target=%s, chain=%s, path=%s, cycle=%s)",
+ realRootedPath,
+ realFileStateValue,
+ linkTarget,
+ logicalChainDuringResolution,
+ pathToUnboundedAncestorSymlinkExpansionChain,
+ unboundedAncestorSymlinkExpansionChain);
+ }
+ }
+
/** Implementation of {@link FileValue} for paths that are themselves symlinks. */
@AutoCodec.VisibleForSerialization
@AutoCodec
- public static final class SymlinkFileValueWithStoredChain
+ public static class SymlinkFileValueWithStoredChain
extends DifferentRealPathFileValueWithStoredChain {
- private final PathFragment linkTarget;
+ protected final PathFragment linkTarget;
@VisibleForTesting
public SymlinkFileValueWithStoredChain(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index fa674c4..8062c16 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -1057,6 +1057,7 @@
name = "collect_packages_under_directory_function",
srcs = ["CollectPackagesUnderDirectoryFunction.java"],
deps = [
+ "process_package_directory",
":collect_packages_under_directory_value",
":recursive_directory_traversal_function",
":recursive_pkg_key",
@@ -1524,6 +1525,8 @@
srcs = ["GlobFunction.java"],
deps = [
":directory_listing_value",
+ ":file_symlink_infinite_expansion_exception",
+ ":file_symlink_infinite_expansion_uniqueness_function",
":glob_descriptor",
":glob_value",
":ignored_package_prefixes_value",
@@ -2020,6 +2023,8 @@
":directory_listing_value",
":dirents",
":file_symlink_exception",
+ ":file_symlink_infinite_expansion_exception",
+ ":file_symlink_infinite_expansion_uniqueness_function",
":package_lookup_value",
":process_package_directory_result",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
@@ -2095,6 +2100,8 @@
deps = [
":action_execution_value",
":directory_listing_value",
+ ":file_symlink_infinite_expansion_exception",
+ ":file_symlink_infinite_expansion_uniqueness_function",
":package_lookup_value",
":sky_functions",
":tree_artifact_value",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
index bae2c18..7e0668a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
@@ -66,11 +66,7 @@
this.nonexistentFileReceiver = nonexistentFileReceiver;
}
- @Override
- public FileValue compute(SkyKey skyKey, Environment env)
- throws FileFunctionException, InterruptedException {
- RootedPath rootedPath = (RootedPath) skyKey.argument();
-
+ private static class SymlinkResolutionState {
// Suppose we have a path p. One of the goals of FileFunction is to resolve the "real path", if
// any, of p. The basic algorithm is to use the fully resolved path of p's parent directory to
// determine the fully resolved path of p. This is complicated when symlinks are involved, and
@@ -91,6 +87,18 @@
// See the usage in checkPathSeenDuringPartialResolutionInternal.
TreeSet<Path> sortedLogicalChain = Sets.newTreeSet();
+ ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain = null;
+ ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain = null;
+
+ private SymlinkResolutionState() {}
+ }
+
+ @Override
+ public FileValue compute(SkyKey skyKey, Environment env)
+ throws FileFunctionException, InterruptedException {
+ RootedPath rootedPath = (RootedPath) skyKey.argument();
+ SymlinkResolutionState symlinkResolutionState = new SymlinkResolutionState();
+
// Fully resolve the path of the parent directory, but only if the current file is not the
// filesystem root (has no parent) or a package path root (treated opaquely and handled by
// skyframe's DiffAwareness interface).
@@ -99,7 +107,7 @@
// an ancestor is part of a symlink cycle, we want to detect that quickly as it gives a more
// informative error message than we'd get doing bogus filesystem operations.
PartialResolutionResult resolveFromAncestorsResult =
- resolveFromAncestors(rootedPath, sortedLogicalChain, logicalChain, env);
+ resolveFromAncestors(rootedPath, symlinkResolutionState, env);
if (resolveFromAncestorsResult == null) {
return null;
}
@@ -107,7 +115,9 @@
FileStateValue fileStateValueFromAncestors = resolveFromAncestorsResult.fileStateValue;
if (fileStateValueFromAncestors.getType() == FileStateType.NONEXISTENT) {
return FileValue.value(
- ImmutableList.copyOf(logicalChain),
+ ImmutableList.copyOf(symlinkResolutionState.logicalChain),
+ symlinkResolutionState.pathToUnboundedAncestorSymlinkExpansionChain,
+ symlinkResolutionState.unboundedAncestorSymlinkExpansionChain,
rootedPath,
FileStateValue.NONEXISTENT_FILE_STATE_NODE,
rootedPathFromAncestors,
@@ -120,11 +130,7 @@
while (realFileStateValue.getType().isSymlink()) {
PartialResolutionResult getSymlinkTargetRootedPathResult =
getSymlinkTargetRootedPath(
- realRootedPath,
- realFileStateValue.getSymlinkTarget(),
- sortedLogicalChain,
- logicalChain,
- env);
+ realRootedPath, realFileStateValue.getSymlinkTarget(), symlinkResolutionState, env);
if (getSymlinkTargetRootedPathResult == null) {
return null;
}
@@ -133,7 +139,9 @@
}
return FileValue.value(
- ImmutableList.copyOf(logicalChain),
+ ImmutableList.copyOf(symlinkResolutionState.logicalChain),
+ symlinkResolutionState.pathToUnboundedAncestorSymlinkExpansionChain,
+ symlinkResolutionState.unboundedAncestorSymlinkExpansionChain,
rootedPath,
// TODO(b/123922036): This is a bug. Should be 'fileStateValueFromAncestors'.
fileStateValueFromAncestors,
@@ -156,24 +164,19 @@
*/
@Nullable
private PartialResolutionResult resolveFromAncestors(
- RootedPath rootedPath,
- TreeSet<Path> sortedLogicalChain,
- ArrayList<RootedPath> logicalChain,
- Environment env)
+ RootedPath rootedPath, SymlinkResolutionState symlinkResolutionState, Environment env)
throws InterruptedException, FileFunctionException {
RootedPath parentRootedPath = rootedPath.getParentDirectory();
return parentRootedPath != null
- ? resolveFromAncestorsWithParent(
- rootedPath, parentRootedPath, sortedLogicalChain, logicalChain, env)
- : resolveFromAncestorsNoParent(rootedPath, sortedLogicalChain, logicalChain, env);
+ ? resolveFromAncestorsWithParent(rootedPath, parentRootedPath, symlinkResolutionState, env)
+ : resolveFromAncestorsNoParent(rootedPath, symlinkResolutionState, env);
}
@Nullable
private PartialResolutionResult resolveFromAncestorsWithParent(
RootedPath rootedPath,
RootedPath parentRootedPath,
- TreeSet<Path> sortedLogicalChain,
- ArrayList<RootedPath> logicalChain,
+ SymlinkResolutionState symlinkResolutionState,
Environment env)
throws InterruptedException, FileFunctionException {
PathFragment relativePath = rootedPath.getRootRelativePath();
@@ -197,7 +200,7 @@
for (RootedPath parentPartialRootedPath : parentFileValue.logicalChainDuringResolution()) {
checkAndNotePathSeenDuringPartialResolution(
- getChild(parentPartialRootedPath, baseName), sortedLogicalChain, logicalChain, env);
+ getChild(parentPartialRootedPath, baseName), symlinkResolutionState, env);
if (env.valuesMissing()) {
return null;
}
@@ -214,12 +217,9 @@
@Nullable
private PartialResolutionResult resolveFromAncestorsNoParent(
- RootedPath rootedPath,
- TreeSet<Path> sortedLogicalChain,
- ArrayList<RootedPath> logicalChain,
- Environment env)
+ RootedPath rootedPath, SymlinkResolutionState symlinkResolutionState, Environment env)
throws InterruptedException, FileFunctionException {
- checkAndNotePathSeenDuringPartialResolution(rootedPath, sortedLogicalChain, logicalChain, env);
+ checkAndNotePathSeenDuringPartialResolution(rootedPath, symlinkResolutionState, env);
if (env.valuesMissing()) {
return null;
}
@@ -249,8 +249,7 @@
private PartialResolutionResult getSymlinkTargetRootedPath(
RootedPath rootedPath,
PathFragment symlinkTarget,
- TreeSet<Path> sortedLogicalChain,
- ArrayList<RootedPath> logicalChain,
+ SymlinkResolutionState symlinkResolutionState,
Environment env)
throws FileFunctionException, InterruptedException {
Path path = rootedPath.asPath();
@@ -265,44 +264,35 @@
: path.getRelative(symlinkTarget);
}
RootedPath symlinkTargetRootedPath = toRootedPath(symlinkTargetPath);
- checkPathSeenDuringPartialResolution(
- symlinkTargetRootedPath, sortedLogicalChain, logicalChain, env);
+ checkPathSeenDuringPartialResolution(symlinkTargetRootedPath, symlinkResolutionState, env);
if (env.valuesMissing()) {
return null;
}
// The symlink target could have a different parent directory, which itself could be a directory
// symlink (or have an ancestor directory symlink)!
- return resolveFromAncestors(symlinkTargetRootedPath, sortedLogicalChain, logicalChain, env);
+ return resolveFromAncestors(symlinkTargetRootedPath, symlinkResolutionState, env);
}
private void checkAndNotePathSeenDuringPartialResolution(
- RootedPath rootedPath,
- TreeSet<Path> sortedLogicalChain,
- ArrayList<RootedPath> logicalChain,
- Environment env)
+ RootedPath rootedPath, SymlinkResolutionState symlinkResolutionState, Environment env)
throws FileFunctionException, InterruptedException {
Path path = rootedPath.asPath();
- checkPathSeenDuringPartialResolutionInternal(
- rootedPath, path, sortedLogicalChain, logicalChain, env);
- sortedLogicalChain.add(path);
- logicalChain.add(rootedPath);
+ checkPathSeenDuringPartialResolutionInternal(rootedPath, path, symlinkResolutionState, env);
+ symlinkResolutionState.sortedLogicalChain.add(path);
+ symlinkResolutionState.logicalChain.add(rootedPath);
}
private void checkPathSeenDuringPartialResolution(
- RootedPath rootedPath,
- TreeSet<Path> sortedLogicalChain,
- ArrayList<RootedPath> logicalChain,
- Environment env)
+ RootedPath rootedPath, SymlinkResolutionState symlinkResolutionState, Environment env)
throws FileFunctionException, InterruptedException {
checkPathSeenDuringPartialResolutionInternal(
- rootedPath, rootedPath.asPath(), sortedLogicalChain, logicalChain, env);
+ rootedPath, rootedPath.asPath(), symlinkResolutionState, env);
}
private void checkPathSeenDuringPartialResolutionInternal(
RootedPath rootedPath,
Path path,
- TreeSet<Path> sortedLogicalChain,
- ArrayList<RootedPath> logicalChain,
+ SymlinkResolutionState symlinkResolutionState,
Environment env)
throws FileFunctionException, InterruptedException {
// We are about to perform another step of partial real path resolution. 'logicalChain' is the
@@ -329,12 +319,13 @@
// candidate p for (ii) and (iii).
SkyKey uniquenessKey = null;
FileSymlinkException fse = null;
- Path seenFloorPath = sortedLogicalChain.floor(path);
- Path seenCeilingPath = sortedLogicalChain.ceiling(path);
- if (sortedLogicalChain.contains(path)) {
+ Path seenFloorPath = symlinkResolutionState.sortedLogicalChain.floor(path);
+ Path seenCeilingPath = symlinkResolutionState.sortedLogicalChain.ceiling(path);
+ if (symlinkResolutionState.sortedLogicalChain.contains(path)) {
// 'rootedPath' is [transitively] a symlink to a previous element in the symlink chain (i).
Pair<ImmutableList<RootedPath>, ImmutableList<RootedPath>> pathAndChain =
- CycleUtils.splitIntoPathAndChain(isPathPredicate(path), logicalChain);
+ CycleUtils.splitIntoPathAndChain(
+ isPathPredicate(path), symlinkResolutionState.logicalChain);
FileSymlinkCycleException fsce =
new FileSymlinkCycleException(pathAndChain.getFirst(), pathAndChain.getSecond());
uniquenessKey = FileSymlinkCycleUniquenessFunction.key(fsce.getCycle());
@@ -345,21 +336,26 @@
Pair<ImmutableList<RootedPath>, ImmutableList<RootedPath>> pathAndChain =
CycleUtils.splitIntoPathAndChain(
isPathPredicate(seenFloorPath),
- ImmutableList.copyOf(Iterables.concat(logicalChain, ImmutableList.of(rootedPath))));
+ ImmutableList.copyOf(
+ Iterables.concat(
+ symlinkResolutionState.logicalChain, ImmutableList.of(rootedPath))));
uniquenessKey = FileSymlinkInfiniteExpansionUniquenessFunction.key(pathAndChain.getSecond());
fse = new FileSymlinkInfiniteExpansionException(
pathAndChain.getFirst(), pathAndChain.getSecond());
} else if (seenCeilingPath != null && seenCeilingPath.startsWith(path)) {
// 'rootedPath' is [transitively] a symlink to an ancestor of a previous element in the
// symlink chain (iii).
- Pair<ImmutableList<RootedPath>, ImmutableList<RootedPath>> pathAndChain =
- CycleUtils.splitIntoPathAndChain(
- isPathPredicate(seenCeilingPath),
- ImmutableList.copyOf(Iterables.concat(logicalChain, ImmutableList.of(rootedPath))));
- uniquenessKey = FileSymlinkInfiniteExpansionUniquenessFunction.key(pathAndChain.getSecond());
- fse =
- new FileSymlinkInfiniteExpansionException(
- pathAndChain.getFirst(), pathAndChain.getSecond());
+ if (symlinkResolutionState.unboundedAncestorSymlinkExpansionChain == null) {
+ Pair<ImmutableList<RootedPath>, ImmutableList<RootedPath>> pathAndChain =
+ CycleUtils.splitIntoPathAndChain(
+ isPathPredicate(seenCeilingPath),
+ ImmutableList.copyOf(
+ Iterables.concat(
+ symlinkResolutionState.logicalChain, ImmutableList.of(rootedPath))));
+ symlinkResolutionState.pathToUnboundedAncestorSymlinkExpansionChain =
+ pathAndChain.getFirst();
+ symlinkResolutionState.unboundedAncestorSymlinkExpansionChain = pathAndChain.getSecond();
+ }
}
if (uniquenessKey != null) {
// Note that this dependency is merely to ensure that each unique symlink error gets
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
index b832485..8aee295 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
@@ -235,6 +235,28 @@
if (!symlinkFileValue.exists()) {
continue;
}
+
+ // This check is more strict than necessary: we raise an error if globbing traverses into
+ // a directory for any reason, even though it's only necessary if that reason was the
+ // resolution of a recursive glob ("**"). Fixing this would require plumbing the ancestor
+ // symlink information through DirectoryListingValue.
+ if (symlinkFileValue.isDirectory()
+ && symlinkFileValue.unboundedAncestorSymlinkExpansionChain() != null) {
+ SkyKey uniquenessKey =
+ FileSymlinkInfiniteExpansionUniquenessFunction.key(
+ symlinkFileValue.unboundedAncestorSymlinkExpansionChain());
+ env.getValue(uniquenessKey);
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ FileSymlinkInfiniteExpansionException symlinkException =
+ new FileSymlinkInfiniteExpansionException(
+ symlinkFileValue.pathToUnboundedAncestorSymlinkExpansionChain(),
+ symlinkFileValue.unboundedAncestorSymlinkExpansionChain());
+ throw new GlobFunctionException(symlinkException, Transience.PERSISTENT);
+ }
+
Dirent dirent = symlinkFileMap.get(lookedUpKeyAndValue.getKey());
String fileName = dirent.getName();
if (symlinkFileValue.isDirectory()) {
@@ -414,5 +436,9 @@
public GlobFunctionException(InconsistentFilesystemException e, Transience transience) {
super(e, transience);
}
+
+ public GlobFunctionException(FileSymlinkInfiniteExpansionException e, Transience transience) {
+ super(e, transience);
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ProcessPackageDirectory.java b/src/main/java/com/google/devtools/build/lib/skyframe/ProcessPackageDirectory.java
index 0b93eea..c614c18 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ProcessPackageDirectory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ProcessPackageDirectory.java
@@ -96,6 +96,23 @@
return ProcessPackageDirectoryResult.EMPTY_RESULT;
}
+ if (fileValue.unboundedAncestorSymlinkExpansionChain() != null) {
+ SkyKey uniquenessKey =
+ FileSymlinkInfiniteExpansionUniquenessFunction.key(
+ fileValue.unboundedAncestorSymlinkExpansionChain());
+ env.getValue(uniquenessKey);
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ FileSymlinkInfiniteExpansionException symlinkException =
+ new FileSymlinkInfiniteExpansionException(
+ fileValue.pathToUnboundedAncestorSymlinkExpansionChain(),
+ fileValue.unboundedAncestorSymlinkExpansionChain());
+ return reportErrorAndReturn(
+ symlinkException.getMessage(), symlinkException, rootRelativePath, env.getListener());
+ }
+
PackageIdentifier packageId = PackageIdentifier.create(repositoryName, rootRelativePath);
if ((packageId.getRepository().isDefault() || packageId.getRepository().isMain())
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
index 8b27387..3bba228 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
@@ -338,6 +338,19 @@
if (env.valuesMissing()) {
throw new MissingDepException();
}
+ if (fileValue.unboundedAncestorSymlinkExpansionChain() != null) {
+ SkyKey uniquenessKey =
+ FileSymlinkInfiniteExpansionUniquenessFunction.key(
+ fileValue.unboundedAncestorSymlinkExpansionChain());
+ env.getValue(uniquenessKey);
+ if (env.valuesMissing()) {
+ throw new MissingDepException();
+ }
+
+ throw new FileSymlinkInfiniteExpansionException(
+ fileValue.pathToUnboundedAncestorSymlinkExpansionChain(),
+ fileValue.unboundedAncestorSymlinkExpansionChain());
+ }
if (fileValue.exists()) {
// If it exists, it may either be a symlink or a file/directory.
PathFragment unresolvedLinkTarget = null;
