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