Logical rollforward of https://github.com/bazelbuild/bazel/commit/e2e235359ccdc4b6a122586b9d7c99abbddd65f5. That commit was reverted because it tickled an unrelated bug accidentally exercised by a Google-internal test.
The unrelated bug has been fixed separately.
RELNOTES: None
PiperOrigin-RevId: 234620238
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 f03268a..7c2f416 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
@@ -166,16 +166,16 @@
public static FileValue value(
ImmutableList<RootedPath> logicalChainDuringResolution,
RootedPath originalRootedPath,
- FileStateValue fileStateValue,
+ FileStateValue fileStateValueFromAncestors,
RootedPath realRootedPath,
FileStateValue realFileStateValue) {
if (originalRootedPath.equals(realRootedPath)) {
Preconditions.checkState(
- fileStateValue.getType() != FileStateType.SYMLINK,
- "originalRootedPath: %s, fileStateValue: %s, realRootedPath: %s, "
- + "fileStateValueFromAncestors: %s",
+ fileStateValueFromAncestors.getType() != FileStateType.SYMLINK,
+ "originalRootedPath: %s, fileStateValueFromAncestors: %s, "
+ + "realRootedPath: %s, fileStateValueFromAncestors: %s",
originalRootedPath,
- fileStateValue,
+ fileStateValueFromAncestors,
realRootedPath,
realFileStateValue);
Preconditions.checkState(
@@ -185,7 +185,7 @@
"logicalChainDuringResolution: %s, originalRootedPath: %s",
logicalChainDuringResolution,
originalRootedPath);
- return new RegularFileValue(originalRootedPath, fileStateValue);
+ return new RegularFileValue(originalRootedPath, fileStateValueFromAncestors);
}
boolean shouldStoreChain;
@@ -200,11 +200,11 @@
shouldStoreChain = true;
break;
default:
- throw new IllegalStateException(fileStateValue.getType().toString());
+ throw new IllegalStateException(realFileStateValue.getType().toString());
}
- if (fileStateValue.getType() == FileStateType.SYMLINK) {
- PathFragment symlinkTarget = fileStateValue.getSymlinkTarget();
+ if (fileStateValueFromAncestors.getType() == FileStateType.SYMLINK) {
+ PathFragment symlinkTarget = fileStateValueFromAncestors.getSymlinkTarget();
return shouldStoreChain
? new SymlinkFileValueWithStoredChain(
realRootedPath, realFileStateValue, logicalChainDuringResolution, symlinkTarget)
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 0f0aaa6..6f02026 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
@@ -117,17 +117,6 @@
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;
- }
- }
-
while (realFileStateValue.getType().isSymlink()) {
PartialResolutionResult getSymlinkTargetRootedPathResult =
getSymlinkTargetRootedPath(
@@ -147,7 +136,7 @@
ImmutableList.copyOf(logicalChain),
rootedPath,
// TODO(b/123922036): This is a bug. Should be 'fileStateValueFromAncestors'.
- fileStateValue,
+ fileStateValueFromAncestors,
realRootedPath,
realFileStateValue);
}
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 9f68139..103eae8 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
@@ -1385,6 +1385,35 @@
assertThat(result.get(fooKey).exists()).isTrue();
}
+ @Test
+ public void testMultipleLevelsOfDirectorySymlinks_Clean() throws Exception {
+ symlink("a/b/c", "../c");
+ Path abcd = path("a/b/c/d");
+ symlink("a/c/d", "../d");
+ assertThat(valueForPath(abcd).isSymlink()).isTrue();
+ }
+
+ @Test
+ public void testMultipleLevelsOfDirectorySymlinks_Incremental() throws Exception {
+ SequentialBuildDriver driver = makeDriver();
+
+ symlink("a/b/c", "../c");
+ Path acd = directory("a/c/d");
+ Path abcd = path("a/b/c/d");
+
+ FileValue abcdFileValue = valueForPathHelper(pkgRoot, abcd, driver);
+ assertThat(abcdFileValue.isDirectory()).isTrue();
+ assertThat(abcdFileValue.isSymlink()).isFalse();
+
+ acd.delete();
+ symlink("a/c/d", "../d");
+ differencer.invalidate(ImmutableList.of(fileStateSkyKey("a/c/d")));
+
+ abcdFileValue = valueForPathHelper(pkgRoot, abcd, driver);
+
+ assertThat(abcdFileValue.isSymlink()).isTrue();
+ }
+
private void checkRealPath(String pathString) throws Exception {
Path realPath = pkgRoot.getRelative(pathString).resolveSymbolicLinks();
assertRealPath(pathString, pkgRoot.relativize(realPath).toString());
