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/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
