Fix long-standing corner case with infinite symlink expansion detection. See the added unit tests for more details.
The approach is to have FileFunction consider the entire logical chain of paths encountered during realpath resolution, not just the physical symlink chain. In order to implement this, we have FileValue instances embed the full logical chain of paths encountered during their own real path resolution. This leads to a natural recursive algorithm.
In addition to the new code being less buggy, I hope it's also either to reason about since the algorithm is more natural. The added code comments should help as well.
Another cool thing is that the error message printed to the user on infinite symlink expansion is much more useful since it shows them the full chain of paths.
While there is a theoretical memory concern (consider a path like 'a/b/c/d/e/f/g' where 'a' is a symlink to 'a1/b/c/d/e/f/g', 'b' is a symlink to 'b1/c/d/e/f/g', etc), in practice there's no noticeable memory increase for many use cases.
RELNOTES: None
PiperOrigin-RevId: 234606705
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 73362d3..f03268a 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
@@ -15,7 +15,9 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Interner;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -88,6 +90,18 @@
}
/**
+ * If {@code !isFile() && exists()}, returns an ordered list of the {@link RootedPath}s that were
+ * considered when determining {@code realRootedPath()}.
+ *
+ * <p>This information is used to detect unbounded symlink expansions.
+ *
+ * <p>As a memory optimization, we don't store this information when {@code isFile() || !exists()}
+ * -- this information is only needed for resolving ancestors, and an existing file or a
+ * non-existent directory has no descendants, by definition.
+ */
+ public abstract ImmutableList<RootedPath> logicalChainDuringResolution();
+
+ /**
* 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.
@@ -150,28 +164,62 @@
* not be used for symlink cycles.
*/
public static FileValue value(
- RootedPath rootedPath,
+ ImmutableList<RootedPath> logicalChainDuringResolution,
+ RootedPath originalRootedPath,
FileStateValue fileStateValue,
RootedPath realRootedPath,
FileStateValue realFileStateValue) {
- if (rootedPath.equals(realRootedPath)) {
- Preconditions.checkState(fileStateValue.getType() != FileStateType.SYMLINK,
- "rootedPath: %s, fileStateValue: %s, realRootedPath: %s, realFileStateValue: %s",
- rootedPath, fileStateValue, realRootedPath, realFileStateValue);
- return new RegularFileValue(rootedPath, fileStateValue);
- } else {
- if (fileStateValue.getType() == FileStateType.SYMLINK) {
- return new SymlinkFileValue(realRootedPath, realFileStateValue,
- fileStateValue.getSymlinkTarget());
- } else {
- return new DifferentRealPathFileValue(
- realRootedPath, realFileStateValue);
- }
+ if (originalRootedPath.equals(realRootedPath)) {
+ Preconditions.checkState(
+ fileStateValue.getType() != FileStateType.SYMLINK,
+ "originalRootedPath: %s, fileStateValue: %s, realRootedPath: %s, "
+ + "fileStateValueFromAncestors: %s",
+ originalRootedPath,
+ fileStateValue,
+ realRootedPath,
+ realFileStateValue);
+ Preconditions.checkState(
+ !realFileStateValue.getType().exists()
+ || realFileStateValue.getType().isFile()
+ || Iterables.getOnlyElement(logicalChainDuringResolution).equals(originalRootedPath),
+ "logicalChainDuringResolution: %s, originalRootedPath: %s",
+ logicalChainDuringResolution,
+ originalRootedPath);
+ return new RegularFileValue(originalRootedPath, fileStateValue);
}
+
+ boolean shouldStoreChain;
+ switch (realFileStateValue.getType()) {
+ case REGULAR_FILE:
+ case SPECIAL_FILE:
+ case NONEXISTENT:
+ shouldStoreChain = false;
+ break;
+ case SYMLINK:
+ case DIRECTORY:
+ shouldStoreChain = true;
+ break;
+ default:
+ throw new IllegalStateException(fileStateValue.getType().toString());
+ }
+
+ if (fileStateValue.getType() == FileStateType.SYMLINK) {
+ PathFragment symlinkTarget = fileStateValue.getSymlinkTarget();
+ return shouldStoreChain
+ ? new SymlinkFileValueWithStoredChain(
+ realRootedPath, realFileStateValue, logicalChainDuringResolution, symlinkTarget)
+ : new SymlinkFileValueWithoutStoredChain(
+ realRootedPath, realFileStateValue, symlinkTarget);
+ }
+
+ return shouldStoreChain
+ ? new DifferentRealPathFileValueWithStoredChain(
+ realRootedPath, realFileStateValue, logicalChainDuringResolution)
+ : new DifferentRealPathFileValueWithoutStoredChain(realRootedPath, realFileStateValue);
}
/**
- * Implementation of {@link FileValue} for files whose fully resolved path is the same as the
+ * Implementation of {@link FileValue} for paths whose fully resolved path is the same as the
* requested path. For example, this is the case for the path "foo/bar/baz" if neither 'foo' nor
* 'foo/bar' nor 'foo/bar/baz' are symlinks.
*/
@@ -188,6 +236,11 @@
}
@Override
+ public ImmutableList<RootedPath> logicalChainDuringResolution() {
+ return ImmutableList.of(rootedPath);
+ }
+
+ @Override
public RootedPath realRootedPath() {
return rootedPath;
}
@@ -202,7 +255,7 @@
if (obj == null) {
return false;
}
- if (obj.getClass() != RegularFileValue.class) {
+ if (!(obj instanceof RegularFileValue)) {
return false;
}
RegularFileValue other = (RegularFileValue) obj;
@@ -216,23 +269,86 @@
@Override
public String toString() {
- return rootedPath + ", " + fileStateValue;
+ return String.format("non-symlink (path=%s, state=%s)", rootedPath, fileStateValue);
}
}
/**
- * Base class for {@link FileValue}s for files whose fully resolved path is different than the
- * requested path. For example, this is the case for the path "foo/bar/baz" if at least one of
- * 'foo', 'foo/bar', or 'foo/bar/baz' is a symlink.
+ * 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.
*/
@AutoCodec.VisibleForSerialization
@AutoCodec
- public static class DifferentRealPathFileValue extends FileValue {
+ public static class DifferentRealPathFileValueWithStoredChain extends FileValue {
+ protected final RootedPath realRootedPath;
+ protected final FileStateValue realFileStateValue;
+ protected final ImmutableList<RootedPath> logicalChainDuringResolution;
+ public DifferentRealPathFileValueWithStoredChain(
+ RootedPath realRootedPath,
+ FileStateValue realFileStateValue,
+ ImmutableList<RootedPath> logicalChainDuringResolution) {
+ this.realRootedPath = Preconditions.checkNotNull(realRootedPath);
+ this.realFileStateValue = Preconditions.checkNotNull(realFileStateValue);
+ this.logicalChainDuringResolution = logicalChainDuringResolution;
+ }
+
+ @Override
+ public RootedPath realRootedPath() {
+ return realRootedPath;
+ }
+
+ @Override
+ public FileStateValue realFileStateValue() {
+ return realFileStateValue;
+ }
+
+ @Override
+ public ImmutableList<RootedPath> logicalChainDuringResolution() {
+ return logicalChainDuringResolution;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null) {
+ return false;
+ }
+ // Note that we can't use 'instanceof' because this class has a subclass.
+ if (obj.getClass() != DifferentRealPathFileValueWithStoredChain.class) {
+ return false;
+ }
+ DifferentRealPathFileValueWithStoredChain other =
+ (DifferentRealPathFileValueWithStoredChain) obj;
+ return realRootedPath.equals(other.realRootedPath)
+ && realFileStateValue.equals(other.realFileStateValue)
+ && logicalChainDuringResolution.equals(other.logicalChainDuringResolution);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(realRootedPath, realFileStateValue, logicalChainDuringResolution);
+ }
+
+ @Override
+ public String toString() {
+ return String.format(
+ "symlink ancestor (real_path=%s, real_state=%s, chain=%s)",
+ realRootedPath, realFileStateValue, logicalChainDuringResolution);
+ }
+ }
+
+ /**
+ * Same as {@link DifferentRealPathFileValueWithStoredChain}, except without {@link
+ * #logicalChainDuringResolution}.
+ */
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec
+ public static class DifferentRealPathFileValueWithoutStoredChain extends FileValue {
protected final RootedPath realRootedPath;
protected final FileStateValue realFileStateValue;
- public DifferentRealPathFileValue(
+ public DifferentRealPathFileValueWithoutStoredChain(
RootedPath realRootedPath, FileStateValue realFileStateValue) {
this.realRootedPath = Preconditions.checkNotNull(realRootedPath);
this.realFileStateValue = Preconditions.checkNotNull(realFileStateValue);
@@ -249,14 +365,21 @@
}
@Override
+ public ImmutableList<RootedPath> logicalChainDuringResolution() {
+ throw new IllegalStateException(this.toString());
+ }
+
+ @Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
- if (obj.getClass() != DifferentRealPathFileValue.class) {
+ // Note that we can't use 'instanceof' because this class has a subclass.
+ if (obj.getClass() != DifferentRealPathFileValueWithoutStoredChain.class) {
return false;
}
- DifferentRealPathFileValue other = (DifferentRealPathFileValue) obj;
+ DifferentRealPathFileValueWithoutStoredChain other =
+ (DifferentRealPathFileValueWithoutStoredChain) obj;
return realRootedPath.equals(other.realRootedPath)
&& realFileStateValue.equals(other.realFileStateValue);
}
@@ -268,18 +391,79 @@
@Override
public String toString() {
- return realRootedPath + ", " + realFileStateValue + " (symlink ancestor)";
+ return String.format(
+ "symlink ancestor (real_path=%s, real_state=%s)", realRootedPath, realFileStateValue);
}
}
- /** Implementation of {@link FileValue} for files that are symlinks. */
- @VisibleForTesting
+ /** Implementation of {@link FileValue} for paths that are themselves symlinks. */
+ @AutoCodec.VisibleForSerialization
@AutoCodec
- public static final class SymlinkFileValue extends DifferentRealPathFileValue {
+ public static final class SymlinkFileValueWithStoredChain
+ extends DifferentRealPathFileValueWithStoredChain {
private final PathFragment linkTarget;
@VisibleForTesting
- public SymlinkFileValue(
+ public SymlinkFileValueWithStoredChain(
+ RootedPath realRootedPath,
+ FileStateValue realFileStateValue,
+ ImmutableList<RootedPath> logicalChainDuringResolution,
+ PathFragment linkTarget) {
+ super(realRootedPath, realFileStateValue, logicalChainDuringResolution);
+ this.linkTarget = linkTarget;
+ }
+
+ @Override
+ public boolean isSymlink() {
+ return true;
+ }
+
+ @Override
+ public PathFragment getUnresolvedLinkTarget() {
+ return linkTarget;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null) {
+ return false;
+ }
+ if (!(obj instanceof SymlinkFileValueWithStoredChain)) {
+ return false;
+ }
+ SymlinkFileValueWithStoredChain other = (SymlinkFileValueWithStoredChain) obj;
+ return realRootedPath.equals(other.realRootedPath)
+ && realFileStateValue.equals(other.realFileStateValue)
+ && logicalChainDuringResolution.equals(other.logicalChainDuringResolution)
+ && linkTarget.equals(other.linkTarget);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ realRootedPath, realFileStateValue, logicalChainDuringResolution, linkTarget);
+ }
+
+ @Override
+ public String toString() {
+ return String.format(
+ "symlink (real_path=%s, real_state=%s, link_value=%s, chain=%s)",
+ realRootedPath, realFileStateValue, linkTarget, logicalChainDuringResolution);
+ }
+ }
+
+ /**
+ * Same as {@link SymlinkFileValueWithStoredChain}, except without {@link
+ * #logicalChainDuringResolution}.
+ */
+ @VisibleForTesting
+ @AutoCodec
+ public static final class SymlinkFileValueWithoutStoredChain
+ extends DifferentRealPathFileValueWithoutStoredChain {
+ private final PathFragment linkTarget;
+
+ @VisibleForTesting
+ public SymlinkFileValueWithoutStoredChain(
RootedPath realRootedPath, FileStateValue realFileStateValue, PathFragment linkTarget) {
super(realRootedPath, realFileStateValue);
this.linkTarget = linkTarget;
@@ -300,10 +484,10 @@
if (obj == null) {
return false;
}
- if (obj.getClass() != SymlinkFileValue.class) {
+ if (!(obj instanceof SymlinkFileValueWithoutStoredChain)) {
return false;
}
- SymlinkFileValue other = (SymlinkFileValue) obj;
+ SymlinkFileValueWithoutStoredChain other = (SymlinkFileValueWithoutStoredChain) obj;
return realRootedPath.equals(other.realRootedPath)
&& realFileStateValue.equals(other.realFileStateValue)
&& linkTarget.equals(other.linkTarget);
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 a008859..0f0aaa6 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
@@ -39,9 +39,8 @@
/**
* A {@link SkyFunction} for {@link FileValue}s.
*
- * <p>Most of the complexity in the implementation is associated to handling symlinks. Namely,
- * this class makes sure that {@code FileValue}s corresponding to symlinks are correctly invalidated
- * if the destination of the symlink is invalidated. Directory symlinks are also covered.
+ * <p>Most of the complexity in the implementation results from wanting incremental correctness in
+ * the presence of symlinks, esp. ancestor directory symlinks.
*/
public class FileFunction implements SkyFunction {
private final AtomicReference<PathPackageLocator> pkgLocator;
@@ -71,60 +70,100 @@
public FileValue compute(SkyKey skyKey, Environment env)
throws FileFunctionException, InterruptedException {
RootedPath rootedPath = (RootedPath) skyKey.argument();
- RootedPath realRootedPath = null;
- FileStateValue realFileStateValue = null;
- PathFragment relativePath = rootedPath.getRootRelativePath();
- // Resolve ancestor symlinks, 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). Note that this is the first thing we do - if 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.
- if (!relativePath.equals(PathFragment.EMPTY_FRAGMENT)) {
- Pair<RootedPath, FileStateValue> resolvedState = resolveFromAncestors(rootedPath, env);
- if (resolvedState == null) {
- return null;
- }
- realRootedPath = resolvedState.getFirst();
- realFileStateValue = resolvedState.getSecond();
- if (realFileStateValue.getType() == FileStateType.NONEXISTENT) {
- return FileValue.value(
- rootedPath,
- FileStateValue.NONEXISTENT_FILE_STATE_NODE,
- realRootedPath,
- realFileStateValue);
- }
+ // 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
+ // is especially complicated when ancestor directory symlinks are involved.
+ //
+ // Since FileStateValues are the roots of invalidation, care has to be taken to ensuring we
+ // declare the proper FileStateValue deps. As a concrete example, let p = a/b and imagine (i) a
+ // is a direct symlink to c and also (ii) c/b is an existing file. Among other direct deps, we
+ // want to have a direct dep on FileStateValue(c/b), since that's the node that will be changed
+ // if the actual contents of a/b (aka c/b) changes. To rephrase: a dep on FileStateValue(a/b)
+ // won't do anything productive since that path will never be in the Skyframe diff.
+ //
+ // In the course of resolving the real path of p, there will be a logical chain of paths we
+ // consider. Going with the example from above, the full chain of paths we consider is
+ // [a/b, c/b].
+ ArrayList<RootedPath> logicalChain = new ArrayList<>();
+ // Same contents as 'logicalChain', except stored as an sorted TreeSet for efficiency reasons.
+ // See the usage in checkPathSeenDuringPartialResolutionInternal.
+ TreeSet<Path> sortedLogicalChain = Sets.newTreeSet();
+
+ // 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).
+ //
+ // This entails resolving ancestor symlinks fully. Note that this is the first thing we do - if
+ // 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);
+ if (resolveFromAncestorsResult == null) {
+ return null;
}
+ RootedPath rootedPathFromAncestors = resolveFromAncestorsResult.rootedPath;
+ FileStateValue fileStateValueFromAncestors = resolveFromAncestorsResult.fileStateValue;
+ if (fileStateValueFromAncestors.getType() == FileStateType.NONEXISTENT) {
+ return FileValue.value(
+ ImmutableList.copyOf(logicalChain),
+ rootedPath,
+ FileStateValue.NONEXISTENT_FILE_STATE_NODE,
+ rootedPathFromAncestors,
+ fileStateValueFromAncestors);
+ }
+
+ RootedPath realRootedPath = rootedPathFromAncestors;
+ FileStateValue realFileStateValue = fileStateValueFromAncestors;
FileStateValue fileStateValue;
if (rootedPath.equals(realRootedPath)) {
fileStateValue = Preconditions.checkNotNull(realFileStateValue, rootedPath);
} else {
+ // TODO(b/123922036): Yep, this is useless & wasted work.
fileStateValue = (FileStateValue) env.getValue(FileStateValue.key(rootedPath));
if (fileStateValue == null) {
return null;
}
}
- if (realFileStateValue == null) {
- realRootedPath = rootedPath;
- realFileStateValue = fileStateValue;
- }
-
- ArrayList<RootedPath> symlinkChain = new ArrayList<>();
- TreeSet<Path> orderedSeenPaths = Sets.newTreeSet();
while (realFileStateValue.getType().isSymlink()) {
- symlinkChain.add(realRootedPath);
- orderedSeenPaths.add(realRootedPath.asPath());
- Pair<RootedPath, FileStateValue> resolvedState = getSymlinkTargetRootedPath(realRootedPath,
- realFileStateValue.getSymlinkTarget(), orderedSeenPaths, symlinkChain, env);
- if (resolvedState == null) {
+ PartialResolutionResult getSymlinkTargetRootedPathResult =
+ getSymlinkTargetRootedPath(
+ realRootedPath,
+ realFileStateValue.getSymlinkTarget(),
+ sortedLogicalChain,
+ logicalChain,
+ env);
+ if (getSymlinkTargetRootedPathResult == null) {
return null;
}
- realRootedPath = resolvedState.getFirst();
- realFileStateValue = resolvedState.getSecond();
+ realRootedPath = getSymlinkTargetRootedPathResult.rootedPath;
+ realFileStateValue = getSymlinkTargetRootedPathResult.fileStateValue;
}
- return FileValue.value(rootedPath, fileStateValue, realRootedPath, realFileStateValue);
+
+ return FileValue.value(
+ ImmutableList.copyOf(logicalChain),
+ rootedPath,
+ // TODO(b/123922036): This is a bug. Should be 'fileStateValueFromAncestors'.
+ fileStateValue,
+ realRootedPath,
+ realFileStateValue);
+ }
+
+ private static RootedPath getParent(RootedPath childRootedPath) {
+ return RootedPath.toRootedPath(
+ childRootedPath.getRoot(), childRootedPath.getRootRelativePath().getParentDirectory());
+ }
+
+ private static RootedPath getChild(RootedPath parentRootedPath, String baseName) {
+ return RootedPath.toRootedPath(
+ parentRootedPath.getRoot(), parentRootedPath.getRootRelativePath().getChild(baseName));
+ }
+
+ private RootedPath toRootedPath(Path path) {
+ return RootedPath.toRootedPathMaybeUnderRoot(path, pkgLocator.get().getPathEntries());
}
/**
@@ -132,42 +171,91 @@
* {@code null} if there was a missing dep.
*/
@Nullable
- private Pair<RootedPath, FileStateValue> resolveFromAncestors(
- RootedPath rootedPath, Environment env) throws InterruptedException {
- PathFragment relativePath = rootedPath.getRootRelativePath();
- RootedPath realRootedPath = rootedPath;
- FileValue parentFileValue = null;
- PathFragment parentDirectory = relativePath.getParentDirectory();
- if (parentDirectory != null) {
- RootedPath parentRootedPath = RootedPath.toRootedPath(rootedPath.getRoot(), parentDirectory);
+ private PartialResolutionResult resolveFromAncestors(
+ RootedPath rootedPath,
+ TreeSet<Path> sortedLogicalChain,
+ ArrayList<RootedPath> logicalChain,
+ Environment env)
+ throws InterruptedException, FileFunctionException {
+ PathFragment parentDirectory = rootedPath.getRootRelativePath().getParentDirectory();
+ return parentDirectory != null
+ ? resolveFromAncestorsWithParent(
+ rootedPath, parentDirectory, sortedLogicalChain, logicalChain, env)
+ : resolveFromAncestorsNoParent(rootedPath, sortedLogicalChain, logicalChain, env);
+ }
- parentFileValue = (FileValue) env.getValue(FileValue.key(parentRootedPath));
- if (parentFileValue == null) {
+ @Nullable
+ private PartialResolutionResult resolveFromAncestorsWithParent(
+ RootedPath rootedPath,
+ PathFragment parentDirectory,
+ TreeSet<Path> sortedLogicalChain,
+ ArrayList<RootedPath> logicalChain,
+ Environment env)
+ throws InterruptedException, FileFunctionException {
+ PathFragment relativePath = rootedPath.getRootRelativePath();
+ RootedPath rootedPathFromAncestors;
+ String baseName = relativePath.getBaseName();
+ RootedPath parentRootedPath = RootedPath.toRootedPath(rootedPath.getRoot(), parentDirectory);
+
+ FileValue parentFileValue = (FileValue) env.getValue(FileValue.key(parentRootedPath));
+ if (parentFileValue == null) {
+ return null;
+ }
+ rootedPathFromAncestors = getChild(parentFileValue.realRootedPath(), baseName);
+
+ if (!parentFileValue.exists() || !parentFileValue.isDirectory()) {
+ if (nonexistentFileReceiver != null) {
+ nonexistentFileReceiver.accept(
+ rootedPath, rootedPathFromAncestors, parentRootedPath, parentFileValue);
+ }
+ return new PartialResolutionResult(
+ rootedPathFromAncestors, FileStateValue.NONEXISTENT_FILE_STATE_NODE);
+ }
+
+ for (RootedPath parentPartialRootedPath : parentFileValue.logicalChainDuringResolution()) {
+ checkAndNotePathSeenDuringPartialResolution(
+ getChild(parentPartialRootedPath, baseName), sortedLogicalChain, logicalChain, env);
+ if (env.valuesMissing()) {
return null;
}
- PathFragment baseName = PathFragment.create(relativePath.getBaseName());
- RootedPath parentRealRootedPath = parentFileValue.realRootedPath();
- realRootedPath =
- RootedPath.toRootedPath(
- parentRealRootedPath.getRoot(),
- parentRealRootedPath.getRootRelativePath().getRelative(baseName));
+ }
- if (!parentFileValue.exists() || !parentFileValue.isDirectory()) {
- if (nonexistentFileReceiver != null) {
- nonexistentFileReceiver.accept(
- rootedPath, realRootedPath, parentRootedPath, parentFileValue);
- }
- return Pair.of(realRootedPath, FileStateValue.NONEXISTENT_FILE_STATE_NODE);
- }
+ FileStateValue fileStateValueFromAncestors =
+ (FileStateValue) env.getValue(FileStateValue.key(rootedPathFromAncestors));
+ if (fileStateValueFromAncestors == null) {
+ return null;
+ }
+
+ return new PartialResolutionResult(rootedPathFromAncestors, fileStateValueFromAncestors);
+ }
+
+ @Nullable
+ private PartialResolutionResult resolveFromAncestorsNoParent(
+ RootedPath rootedPath,
+ TreeSet<Path> sortedLogicalChain,
+ ArrayList<RootedPath> logicalChain,
+ Environment env)
+ throws InterruptedException, FileFunctionException {
+ checkAndNotePathSeenDuringPartialResolution(rootedPath, sortedLogicalChain, logicalChain, env);
+ if (env.valuesMissing()) {
+ return null;
}
FileStateValue realFileStateValue =
- (FileStateValue)
- env.getValue(FileStateValue.key(realRootedPath));
-
+ (FileStateValue) env.getValue(FileStateValue.key(rootedPath));
if (realFileStateValue == null) {
return null;
}
- return Pair.of(realRootedPath, realFileStateValue);
+ return new PartialResolutionResult(rootedPath, realFileStateValue);
+ }
+
+ private static final class PartialResolutionResult {
+ private final RootedPath rootedPath;
+ private final FileStateValue fileStateValue;
+
+ private PartialResolutionResult(RootedPath rootedPath, FileStateValue fileStateValue) {
+ this.rootedPath = rootedPath;
+ this.fileStateValue = fileStateValue;
+ }
}
/**
@@ -175,102 +263,132 @@
* symlinkTarget}, accounting for ancestor symlinks, or {@code null} if there was a missing dep.
*/
@Nullable
- private Pair<RootedPath, FileStateValue> getSymlinkTargetRootedPath(
+ private PartialResolutionResult getSymlinkTargetRootedPath(
RootedPath rootedPath,
PathFragment symlinkTarget,
- TreeSet<Path> orderedSeenPaths,
- Iterable<RootedPath> symlinkChain,
+ TreeSet<Path> sortedLogicalChain,
+ ArrayList<RootedPath> logicalChain,
Environment env)
throws FileFunctionException, InterruptedException {
- RootedPath symlinkTargetRootedPath;
+ Path path = rootedPath.asPath();
+ Path symlinkTargetPath;
if (symlinkTarget.isAbsolute()) {
- Path path = rootedPath.asPath().getFileSystem().getPath(symlinkTarget);
- symlinkTargetRootedPath =
- RootedPath.toRootedPathMaybeUnderRoot(path, pkgLocator.get().getPathEntries());
+ symlinkTargetPath = path.getRelative(symlinkTarget);
} else {
- Path path = rootedPath.asPath();
- Path symlinkTargetPath;
- if (path.getParentDirectory() != null) {
- RootedPath parentRootedPath = RootedPath.toRootedPathMaybeUnderRoot(
- path.getParentDirectory(), pkgLocator.get().getPathEntries());
- FileValue parentFileValue = (FileValue) env.getValue(FileValue.key(parentRootedPath));
- if (parentFileValue == null) {
- return null;
- }
- symlinkTargetPath = parentFileValue.realRootedPath().asPath().getRelative(symlinkTarget);
- } else {
- // This means '/' is a symlink to 'symlinkTarget'.
- symlinkTargetPath = path.getRelative(symlinkTarget);
- }
- symlinkTargetRootedPath = RootedPath.toRootedPathMaybeUnderRoot(symlinkTargetPath,
- pkgLocator.get().getPathEntries());
+ Path parentPath = path.getParentDirectory();
+ symlinkTargetPath =
+ parentPath != null
+ ? parentPath.getRelative(symlinkTarget)
+ : path.getRelative(symlinkTarget);
}
- // Suppose we have a symlink chain p -> p1 -> p2 -> ... pK. We want to determine the fully
- // resolved path, if any, of p. This entails following the chain and noticing if there's a
- // symlink issue. There three sorts of issues:
- // (i) Symlink cycle:
- // p -> p1 -> p2 -> p1
- // (ii) Unbounded expansion caused by a symlink to a descendant of a member of the chain:
- // p -> a/b -> c/d -> a/b/e
- // (iii) Unbounded expansion caused by a symlink to an ancestor of a member of the chain:
- // p -> a/b -> c/d -> a
+ RootedPath symlinkTargetRootedPath = toRootedPath(symlinkTargetPath);
+ checkPathSeenDuringPartialResolution(
+ symlinkTargetRootedPath, sortedLogicalChain, logicalChain, 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);
+ }
+
+ private void checkAndNotePathSeenDuringPartialResolution(
+ RootedPath rootedPath,
+ TreeSet<Path> sortedLogicalChain,
+ ArrayList<RootedPath> logicalChain,
+ Environment env)
+ throws FileFunctionException, InterruptedException {
+ Path path = rootedPath.asPath();
+ checkPathSeenDuringPartialResolutionInternal(
+ rootedPath, path, sortedLogicalChain, logicalChain, env);
+ sortedLogicalChain.add(path);
+ logicalChain.add(rootedPath);
+ }
+
+ private void checkPathSeenDuringPartialResolution(
+ RootedPath rootedPath,
+ TreeSet<Path> sortedLogicalChain,
+ ArrayList<RootedPath> logicalChain,
+ Environment env)
+ throws FileFunctionException, InterruptedException {
+ checkPathSeenDuringPartialResolutionInternal(
+ rootedPath, rootedPath.asPath(), sortedLogicalChain, logicalChain, env);
+ }
+
+ private void checkPathSeenDuringPartialResolutionInternal(
+ RootedPath rootedPath,
+ Path path,
+ TreeSet<Path> sortedLogicalChain,
+ ArrayList<RootedPath> logicalChain,
+ Environment env)
+ throws FileFunctionException, InterruptedException {
+ // We are about to perform another step of partial real path resolution. 'logicalChain' is the
+ // chain of paths we've considered so far, and 'rootedPath' / 'path' is the proposed next path
+ // we consider.
//
- // We can detect all three of these symlink issues by following the chain and deciding if each
- // new element is problematic. Here is our incremental algorithm:
+ // Before we proceed with 'rootedPath', we need to ensure there won't be a problem. There are
+ // three sorts of issues, all stemming from symlinks:
+ // (i) Symlink cycle:
+ // p -> p1 -> p2 -> p1
+ // (ii) Unbounded expansion caused by a symlink to a descendant of a member of the chain:
+ // p -> a/b -> c/d -> a/b/e
+ // (iii) Unbounded expansion caused by a symlink to an ancestor of a member of the chain:
+ // p -> a/b -> c/d -> a
//
- // Suppose we encounter the symlink target p and we have already encountered all the paths in P:
- // If p is in P then we have a found a cycle (i).
- // If p is a descendant of any path p' in P then we have unbounded expansion (ii).
- // If p is an ancestor of any path p' in P then we have unbounded expansion (iii).
+ // We can detect all three of these symlink issues inspection of the proposed new element. Here
+ // is our incremental algorithm:
+ //
+ // If 'path' is in 'sortedLogicalChain' then we have a found a cycle (i).
+ // If 'path' is a descendant of any path p in 'sortedLogicalChain' then we have unbounded
+ // expansion (ii).
+ // If 'path' is an ancestor of any path p in 'sortedLogicalChain' then we have unbounded
+ // expansion (iii).
// We can check for these cases efficiently (read: sublinear time) by finding the extremal
- // candidate p' for (ii) and (iii).
- Path symlinkTargetPath = symlinkTargetRootedPath.asPath();
+ // candidate p for (ii) and (iii).
SkyKey uniquenessKey = null;
FileSymlinkException fse = null;
- Path seenFloorPath = orderedSeenPaths.floor(symlinkTargetPath);
- Path seenCeilingPath = orderedSeenPaths.ceiling(symlinkTargetPath);
- if (orderedSeenPaths.contains(symlinkTargetPath)) {
- // 'rootedPath' is a symlink to a previous element in the symlink chain (i).
+ Path seenFloorPath = sortedLogicalChain.floor(path);
+ Path seenCeilingPath = sortedLogicalChain.ceiling(path);
+ if (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(symlinkTargetRootedPath.asPath()), symlinkChain);
+ CycleUtils.splitIntoPathAndChain(isPathPredicate(path), logicalChain);
FileSymlinkCycleException fsce =
new FileSymlinkCycleException(pathAndChain.getFirst(), pathAndChain.getSecond());
uniquenessKey = FileSymlinkCycleUniquenessFunction.key(fsce.getCycle());
fse = fsce;
- } else if (seenFloorPath != null && symlinkTargetPath.startsWith(seenFloorPath)) {
- // 'rootedPath' is a symlink to a descendant of a previous element in the symlink chain (ii).
+ } else if (seenFloorPath != null && path.startsWith(seenFloorPath)) {
+ // 'rootedPath' is [transitively] a symlink to a descendant of a previous element in the
+ // symlink chain (ii).
Pair<ImmutableList<RootedPath>, ImmutableList<RootedPath>> pathAndChain =
CycleUtils.splitIntoPathAndChain(
isPathPredicate(seenFloorPath),
- ImmutableList.copyOf(
- Iterables.concat(symlinkChain, ImmutableList.of(symlinkTargetRootedPath))));
+ ImmutableList.copyOf(Iterables.concat(logicalChain, ImmutableList.of(rootedPath))));
uniquenessKey = FileSymlinkInfiniteExpansionUniquenessFunction.key(pathAndChain.getSecond());
fse = new FileSymlinkInfiniteExpansionException(
pathAndChain.getFirst(), pathAndChain.getSecond());
- } else if (seenCeilingPath != null && seenCeilingPath.startsWith(symlinkTargetPath)) {
- // 'rootedPath' is a symlink to an ancestor of a previous element in the symlink chain (iii).
+ } 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(symlinkChain, ImmutableList.of(symlinkTargetRootedPath))));
+ ImmutableList.copyOf(Iterables.concat(logicalChain, ImmutableList.of(rootedPath))));
uniquenessKey = FileSymlinkInfiniteExpansionUniquenessFunction.key(pathAndChain.getSecond());
fse =
new FileSymlinkInfiniteExpansionException(
pathAndChain.getFirst(), pathAndChain.getSecond());
}
if (uniquenessKey != null) {
- if (env.getValue(uniquenessKey) == null) {
- // Note that this dependency is merely to ensure that each unique symlink error gets
- // reported exactly once.
- return null;
+ // Note that this dependency is merely to ensure that each unique symlink error gets
+ // reported exactly once.
+ env.getValue(uniquenessKey);
+ if (env.valuesMissing()) {
+ return;
}
throw new FileFunctionException(
Preconditions.checkNotNull(fse, rootedPath), Transience.PERSISTENT);
}
-
- return resolveFromAncestors(symlinkTargetRootedPath, env);
}
private static final Predicate<RootedPath> isPathPredicate(final Path path) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index c0be09d..9f68139 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.devtools.build.lib.skyframe.SkyframeExecutor.DEFAULT_THREAD_COUNT;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.assertThatEvaluationResult;
import static org.junit.Assert.fail;
@@ -33,6 +34,10 @@
import com.google.common.testing.EqualsTester;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.actions.FileValue;
+import com.google.devtools.build.lib.actions.FileValue.DifferentRealPathFileValueWithStoredChain;
+import com.google.devtools.build.lib.actions.FileValue.DifferentRealPathFileValueWithoutStoredChain;
+import com.google.devtools.build.lib.actions.FileValue.SymlinkFileValueWithStoredChain;
+import com.google.devtools.build.lib.actions.FileValue.SymlinkFileValueWithoutStoredChain;
import com.google.devtools.build.lib.actions.InconsistentFilesystemException;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ServerDirectories;
@@ -199,17 +204,17 @@
}
private FileValue valueForPath(Path path) throws InterruptedException {
- return valueForPathHelper(pkgRoot, path);
+ return valueForPathHelper(pkgRoot, path, makeDriver());
}
private FileValue valueForPathOutsidePkgRoot(Path path) throws InterruptedException {
- return valueForPathHelper(Root.absoluteRoot(fs), path);
+ return valueForPathHelper(Root.absoluteRoot(fs), path, makeDriver());
}
- private FileValue valueForPathHelper(Root root, Path path) throws InterruptedException {
+ private FileValue valueForPathHelper(Root root, Path path, SequentialBuildDriver driver)
+ throws InterruptedException {
PathFragment pathFragment = root.relativize(path);
RootedPath rootedPath = RootedPath.toRootedPath(root, pathFragment);
- SequentialBuildDriver driver = makeDriver();
SkyKey key = FileValue.key(rootedPath);
EvaluationResult<FileValue> result = driver.evaluate(ImmutableList.of(key), EVALUATION_OPTIONS);
assertThat(result.hasError()).isFalse();
@@ -218,15 +223,38 @@
@Test
public void testFileValueHashCodeAndEqualsContract() throws Exception {
- Path pathA = file(pkgRoot + "a", "a");
- Path pathB = file(pkgRoot + "b", "b");
+ Path pathA = file("a", "a");
+ Path pathB = file("b", "b");
+ Path pathC = symlink("c", "a");
+ Path pathD = directory("d");
+ Path pathDA = file("d/a", "da");
+ Path pathE = symlink("e", "d");
+ Path pathF = symlink("f", "a");
+
FileValue valueA1 = valueForPathOutsidePkgRoot(pathA);
FileValue valueA2 = valueForPathOutsidePkgRoot(pathA);
FileValue valueB1 = valueForPathOutsidePkgRoot(pathB);
FileValue valueB2 = valueForPathOutsidePkgRoot(pathB);
+ FileValue valueC1 = valueForPathOutsidePkgRoot(pathC);
+ FileValue valueC2 = valueForPathOutsidePkgRoot(pathC);
+ FileValue valueD1 = valueForPathOutsidePkgRoot(pathD);
+ FileValue valueD2 = valueForPathOutsidePkgRoot(pathD);
+ FileValue valueDA1 = valueForPathOutsidePkgRoot(pathDA);
+ FileValue valueDA2 = valueForPathOutsidePkgRoot(pathDA);
+ FileValue valueE1 = valueForPathOutsidePkgRoot(pathE);
+ FileValue valueE2 = valueForPathOutsidePkgRoot(pathE);
+ FileValue valueF1 = valueForPathOutsidePkgRoot(pathF);
+ FileValue valueF2 = valueForPathOutsidePkgRoot(pathF);
+
new EqualsTester()
.addEqualityGroup(valueA1, valueA2)
.addEqualityGroup(valueB1, valueB2)
+ // Both 'f' and 'c' are transitively symlinks to 'a', so all of these FileValues ought to be
+ // equal.
+ .addEqualityGroup(valueC1, valueC2, valueF1, valueF2)
+ .addEqualityGroup(valueD1, valueD2)
+ .addEqualityGroup(valueDA1, valueDA2)
+ .addEqualityGroup(valueE1, valueE2)
.testEquals();
}
@@ -288,9 +316,17 @@
}
@Test
+ public void testFileUnderBrokenDirectorySymlink() throws Exception {
+ symlink("a", "b/c");
+ symlink("b", "d");
+ assertValueChangesIfContentsOfDirectoryChanges("b", true, "a/e");
+ }
+
+ @Test
public void testFileUnderDirectorySymlink() throws Exception {
symlink("a", "b/c");
symlink("b", "d");
+ file("d/c/e");
assertValueChangesIfContentsOfDirectoryChanges("b", true, "a/e");
}
@@ -1126,8 +1162,8 @@
assertError("this/is/a/path");
}
- private void runTestInfiniteSymlinkExpansion(boolean symlinkToAncestor, boolean absoluteSymlink)
- throws Exception {
+ private void runTestSimpleInfiniteSymlinkExpansion(
+ boolean symlinkToAncestor, boolean absoluteSymlink) throws Exception {
Path otherPath = path("other");
RootedPath otherRootedPath = RootedPath.toRootedPath(pkgRoot, pkgRoot.relativize(otherPath));
Path ancestorPath = path("a");
@@ -1202,22 +1238,110 @@
@Test
public void testInfiniteSymlinkExpansion_AbsoluteSymlinkToDescendant() throws Exception {
- runTestInfiniteSymlinkExpansion(/* symlinkToAncestor= */ false, /*absoluteSymlink=*/ true);
+ runTestSimpleInfiniteSymlinkExpansion(
+ /* symlinkToAncestor= */ false, /*absoluteSymlink=*/ true);
}
@Test
public void testInfiniteSymlinkExpansion_RelativeSymlinkToDescendant() throws Exception {
- runTestInfiniteSymlinkExpansion(/* symlinkToAncestor= */ false, /*absoluteSymlink=*/ false);
+ runTestSimpleInfiniteSymlinkExpansion(
+ /* symlinkToAncestor= */ false, /*absoluteSymlink=*/ false);
}
@Test
public void testInfiniteSymlinkExpansion_AbsoluteSymlinkToAncestor() throws Exception {
- runTestInfiniteSymlinkExpansion(/* symlinkToAncestor= */ true, /*absoluteSymlink=*/ true);
+ runTestSimpleInfiniteSymlinkExpansion(/* symlinkToAncestor= */ true, /*absoluteSymlink=*/ true);
}
@Test
public void testInfiniteSymlinkExpansion_RelativeSymlinkToAncestor() throws Exception {
- runTestInfiniteSymlinkExpansion(/* symlinkToAncestor= */ true, /*absoluteSymlink=*/ false);
+ runTestSimpleInfiniteSymlinkExpansion(
+ /* symlinkToAncestor= */ true, /*absoluteSymlink=*/ false);
+ }
+
+ @Test
+ public void testInfiniteSymlinkExpansion_SymlinkToReferrerToAncestor() throws Exception {
+ symlink("d", "a");
+ Path abPath = directory("a/b");
+ Path abcPath = abPath.getChild("c");
+ symlink("a/b/c", "../../d/b");
+
+ RootedPath rootedPathABC = RootedPath.toRootedPath(pkgRoot, pkgRoot.relativize(abcPath));
+ RootedPath rootedPathAB = RootedPath.toRootedPath(pkgRoot, pkgRoot.relativize(abPath));
+ RootedPath rootedPathDB = RootedPath.toRootedPath(pkgRoot, pkgRoot.relativize(path("d/b")));
+
+ SkyKey keyABC = FileValue.key(rootedPathABC);
+
+ StoredEventHandler eventHandler = new StoredEventHandler();
+ SequentialBuildDriver driver = makeDriver();
+ EvaluationContext evaluationContext =
+ EvaluationContext.newBuilder()
+ .setKeepGoing(true)
+ .setNumThreads(DEFAULT_THREAD_COUNT)
+ .setEventHander(eventHandler)
+ .build();
+ EvaluationResult<FileValue> result =
+ driver.evaluate(ImmutableList.of(keyABC), evaluationContext);
+
+ assertThatEvaluationResult(result).hasErrorEntryForKeyThat(keyABC).isNotTransient();
+ assertThatEvaluationResult(result)
+ .hasErrorEntryForKeyThat(keyABC)
+ .hasExceptionThat()
+ .isInstanceOf(FileSymlinkInfiniteExpansionException.class);
+ FileSymlinkInfiniteExpansionException fiee =
+ (FileSymlinkInfiniteExpansionException) result.getError(keyABC).getException();
+ assertThat(fiee).hasMessageThat().contains("Infinite symlink expansion");
+ assertThat(fiee.getPathToChain()).isEmpty();
+ assertThat(fiee.getChain())
+ .containsExactly(rootedPathABC, rootedPathDB, rootedPathAB)
+ .inOrder();
+
+ assertThat(eventHandler.getEvents()).hasSize(1);
+ assertThat(Iterables.getOnlyElement(eventHandler.getEvents()).getMessage())
+ .contains("infinite symlink expansion detected");
+ }
+
+ @Test
+ public void testInfiniteSymlinkExpansion_SymlinkToReferrerToAncestor_LevelsOfDirectorySymlinks()
+ throws Exception {
+ symlink("dir1/a", "../dir2");
+ symlink("dir2/b", "../dir1");
+
+ RootedPath rootedPathDir1AB =
+ RootedPath.toRootedPath(pkgRoot, pkgRoot.relativize(path("dir1/a/b")));
+ RootedPath rootedPathDir2B =
+ RootedPath.toRootedPath(pkgRoot, pkgRoot.relativize(path("dir2/b")));
+ RootedPath rootedPathDir1 = RootedPath.toRootedPath(pkgRoot, pkgRoot.relativize(path("dir1")));
+
+ SkyKey keyDir1AB = FileValue.key(rootedPathDir1AB);
+
+ StoredEventHandler eventHandler = new StoredEventHandler();
+ SequentialBuildDriver driver = makeDriver();
+ EvaluationContext evaluationContext =
+ EvaluationContext.newBuilder()
+ .setKeepGoing(true)
+ .setNumThreads(DEFAULT_THREAD_COUNT)
+ .setEventHander(eventHandler)
+ .build();
+ EvaluationResult<FileValue> result =
+ driver.evaluate(ImmutableList.of(keyDir1AB), evaluationContext);
+
+ assertThatEvaluationResult(result).hasErrorEntryForKeyThat(keyDir1AB).isNotTransient();
+ assertThatEvaluationResult(result)
+ .hasErrorEntryForKeyThat(keyDir1AB)
+ .hasExceptionThat()
+ .isInstanceOf(FileSymlinkInfiniteExpansionException.class);
+ FileSymlinkInfiniteExpansionException fiee =
+ (FileSymlinkInfiniteExpansionException) result.getError(keyDir1AB).getException();
+ assertThat(fiee).hasMessageThat().contains("Infinite symlink expansion");
+ assertThat(fiee.getPathToChain()).isEmpty();
+ assertThat(fiee.getChain())
+ .containsExactly(rootedPathDir1AB, rootedPathDir2B, rootedPathDir1)
+ .inOrder();
+
+ assertThat(eventHandler.getEvents()).hasSize(1);
+ assertThat(Iterables.getOnlyElement(eventHandler.getEvents()).getMessage())
+ .contains("infinite symlink expansion detected");
}
@Test
@@ -1278,6 +1402,77 @@
.isEqualTo(pkgRoot.getRelative(expectedRealPathString).toString());
}
+ @Test
+ public void testLogicalChainDuringResolution_Directory_SimpleSymlink() throws Exception {
+ symlink("a", "b");
+ symlink("b", "c");
+ directory("c");
+ FileValue fileValue = valueForPath(path("a"));
+ assertThat(fileValue).isInstanceOf(SymlinkFileValueWithStoredChain.class);
+ assertThat(fileValue.getUnresolvedLinkTarget()).isEqualTo(PathFragment.create("b"));
+ assertThat(fileValue.logicalChainDuringResolution())
+ .isEqualTo(ImmutableList.of(rootedPath("a"), rootedPath("b"), rootedPath("c")));
+ }
+
+ @Test
+ public void testLogicalChainDuringResolution_Directory_SimpleAncestorSymlink() throws Exception {
+ symlink("a", "b");
+ symlink("b", "c");
+ directory("c/d");
+ FileValue fileValue = valueForPath(path("a/d"));
+ assertThat(fileValue).isInstanceOf(DifferentRealPathFileValueWithStoredChain.class);
+ assertThat(fileValue.realRootedPath()).isEqualTo(rootedPath("c/d"));
+ assertThat(fileValue.logicalChainDuringResolution())
+ .containsExactly(rootedPath("a/d"), rootedPath("b/d"), rootedPath("c/d"))
+ .inOrder();
+ }
+
+ @Test
+ public void testLogicalChainDuringResolution_File_SimpleSymlink() throws Exception {
+ symlink("a", "b");
+ symlink("b", "c");
+ file("c");
+ FileValue fileValue = valueForPath(path("a"));
+ assertThat(fileValue).isInstanceOf(SymlinkFileValueWithoutStoredChain.class);
+ assertThat(fileValue.getUnresolvedLinkTarget()).isEqualTo(PathFragment.create("b"));
+ assertThrows(IllegalStateException.class, () -> fileValue.logicalChainDuringResolution());
+ }
+
+ @Test
+ public void testLogicalChainDuringResolution_File_SimpleAncestorSymlink() throws Exception {
+ symlink("a", "b");
+ symlink("b", "c");
+ file("c/d");
+ FileValue fileValue = valueForPath(path("a/d"));
+ assertThat(fileValue).isInstanceOf(DifferentRealPathFileValueWithoutStoredChain.class);
+ assertThat(fileValue.realRootedPath()).isEqualTo(rootedPath("c/d"));
+ assertThrows(IllegalStateException.class, () -> fileValue.logicalChainDuringResolution());
+ }
+
+ @Test
+ public void testLogicalChainDuringResolution_Complicated() throws Exception {
+ symlink("a", "b");
+ symlink("b", "c");
+ directory("c");
+ symlink("c/d", "../e/f");
+ symlink("e", "g");
+ directory("g");
+ symlink("g/f", "../h");
+ directory("h");
+ FileValue fileValue = valueForPath(path("a/d"));
+ assertThat(fileValue).isInstanceOf(DifferentRealPathFileValueWithStoredChain.class);
+ assertThat(fileValue.realRootedPath()).isEqualTo(rootedPath("h"));
+ assertThat(fileValue.logicalChainDuringResolution())
+ .containsExactly(
+ rootedPath("a/d"),
+ rootedPath("b/d"),
+ rootedPath("c/d"),
+ rootedPath("e/f"),
+ rootedPath("g/f"),
+ rootedPath("h"))
+ .inOrder();
+ }
+
/**
* Returns a callback that, when executed, deletes the given path. Not meant to be called directly
* by tests.
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileValueTest.java
index 2b059ab..fad42f0 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileValueTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileValueTest.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
+import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.skyframe.serialization.testutils.FsUtils;
@@ -33,9 +34,19 @@
// Assume we have adequate coverage for FileStateValue serialization.
new FileValue.RegularFileValue(
FsUtils.TEST_ROOT, FileStateValue.NONEXISTENT_FILE_STATE_NODE),
- new FileValue.DifferentRealPathFileValue(
+ new FileValue.DifferentRealPathFileValueWithStoredChain(
+ FsUtils.TEST_ROOT,
+ FileStateValue.DIRECTORY_FILE_STATE_NODE,
+ ImmutableList.of(FsUtils.TEST_ROOT)),
+ new FileValue.DifferentRealPathFileValueWithoutStoredChain(
FsUtils.TEST_ROOT, FileStateValue.DIRECTORY_FILE_STATE_NODE),
- new FileValue.SymlinkFileValue(
+ new FileValue.SymlinkFileValueWithStoredChain(
+ FsUtils.TEST_ROOT,
+ new FileStateValue.RegularFileStateValue(
+ /*size=*/ 100, /*digest=*/ new byte[] {1, 2, 3, 4, 5}, /*contentsProxy=*/ null),
+ ImmutableList.of(FsUtils.TEST_ROOT),
+ PathFragment.create("somewhere/else")),
+ new FileValue.SymlinkFileValueWithoutStoredChain(
FsUtils.TEST_ROOT,
new FileStateValue.RegularFileStateValue(
/*size=*/ 100, /*digest=*/ new byte[] {1, 2, 3, 4, 5}, /*contentsProxy=*/ null),
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
index e599f25..cd6fbec 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
@@ -659,7 +659,12 @@
RootedPath pkgRootedPath = RootedPath.toRootedPath(Root.fromPath(root), pkgPath);
FileStateValue pkgDirFileStateValue = FileStateValue.create(pkgRootedPath, null);
FileValue pkgDirValue =
- FileValue.value(pkgRootedPath, pkgDirFileStateValue, pkgRootedPath, pkgDirFileStateValue);
+ FileValue.value(
+ ImmutableList.of(pkgRootedPath),
+ pkgRootedPath,
+ pkgDirFileStateValue,
+ pkgRootedPath,
+ pkgDirFileStateValue);
differencer.inject(ImmutableMap.of(FileValue.key(pkgRootedPath), pkgDirValue));
String expectedMessage = "/root/workspace/pkg is no longer an existing directory";
SkyKey skyKey =
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
index fd134f1..0ecd2b1 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
@@ -94,6 +94,11 @@
return exists;
}
+ @Override
+ public ImmutableList<RootedPath> logicalChainDuringResolution() {
+ throw new UnsupportedOperationException();
+ }
+
void setExists(boolean exists) {
this.exists = exists;
}
