Skip to content

Commit 3e2fb54

Browse files
committed
Include unresolved symlink target into the source directory hash
This makes it so that changes to the targets of unresolved symlinks invalidate actions that depend on the source directory containing them.
1 parent 1aaf251 commit 3e2fb54

File tree

4 files changed

+24
-10
lines changed

4 files changed

+24
-10
lines changed

src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,7 @@ public RecursiveFilesystemTraversalValue compute(SkyKey skyKey, Environment env)
187187
if (rootInfo.type.isSymlink()) {
188188
return RecursiveFilesystemTraversalValue.of(
189189
ResolvedFileFactory.danglingSymlink(
190-
traversal.root().asRootedPath(),
191-
rootInfo.unresolvedSymlinkTarget,
192-
rootInfo.metadata));
190+
traversal.root().asRootedPath(), rootInfo.unresolvedSymlinkTarget));
193191
} else {
194192
return RecursiveFilesystemTraversalValue.EMPTY;
195193
}

src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java

+11-3
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@
1414
package com.google.devtools.build.lib.skyframe;
1515

1616
import static com.google.common.base.Preconditions.checkNotNull;
17+
import static java.nio.charset.StandardCharsets.ISO_8859_1;
1718

1819
import com.google.common.base.Objects;
1920
import com.google.devtools.build.lib.actions.HasDigest;
2021
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
2122
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
2223
import com.google.devtools.build.lib.collect.nestedset.Order;
24+
import com.google.devtools.build.lib.vfs.DigestHashFunction;
2325
import com.google.devtools.build.lib.vfs.PathFragment;
2426
import com.google.devtools.build.lib.vfs.RootedPath;
2527
import com.google.devtools.build.skyframe.SkyValue;
@@ -574,9 +576,15 @@ public static ResolvedFile symlinkToDirectory(
574576
return new SymlinkToDirectory(targetPath, linkNamePath, linkValue, metadata);
575577
}
576578

577-
public static ResolvedFile danglingSymlink(
578-
RootedPath linkNamePath, PathFragment linkValue, HasDigest metadata) {
579-
return new DanglingSymlink(linkNamePath, linkValue, metadata);
579+
public static ResolvedFile danglingSymlink(RootedPath linkNamePath, PathFragment linkValue) {
580+
byte[] digest =
581+
DigestHashFunction.SHA256
582+
.getHashFunction()
583+
.hashString(linkValue.getPathString(), ISO_8859_1)
584+
.asBytes();
585+
// Ensure that the digest does not collide with that of a regular file.
586+
digest[0] ^= 1;
587+
return new DanglingSymlink(linkNamePath, linkValue, new HasDigest.ByteStringDigest(digest));
580588
}
581589
}
582590

src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -830,8 +830,7 @@ public void testTraversalOfDanglingSymlink() throws Exception {
830830
PathFragment linkTarget = PathFragment.create("non_existent");
831831
parentOf(link).asPath().createDirectory();
832832
link.asPath().createSymbolicLink(linkTarget);
833-
traverseAndAssertFiles(
834-
fileLikeRoot(linkArtifact), danglingSymlink(link, linkTarget, EMPTY_METADATA));
833+
traverseAndAssertFiles(fileLikeRoot(linkArtifact), danglingSymlink(link, linkTarget));
835834
}
836835

837836
@Test
@@ -845,7 +844,7 @@ public void testTraversalOfDanglingSymlinkInADirectory() throws Exception {
845844
traverseAndAssertFiles(
846845
fileLikeRoot(dirArtifact),
847846
regularFile(file, EMPTY_METADATA),
848-
danglingSymlink(link, linkTarget, EMPTY_METADATA));
847+
danglingSymlink(link, linkTarget));
849848
}
850849

851850
@Test

src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.skyframe;
1515

16+
import static java.nio.charset.StandardCharsets.ISO_8859_1;
1617
import static org.junit.Assert.assertThrows;
1718

1819
import com.google.common.collect.ImmutableSet;
@@ -195,13 +196,21 @@ public void emptyDirReplacedWithDifferentSymlink_invalidatesAction() throws Exce
195196
}
196197

197198
@Test
198-
@Ignore("TODO(#25834)")
199199
public void danglingSymlinkModified_invalidatesAction() throws Exception {
200200
FileSystemUtils.ensureSymbolicLink(
201201
sourceDir.getRelative("dangling_symlink"), PathFragment.create("still_does_not_exist"));
202202
assertInvalidatedByBuild();
203203
}
204204

205+
@Test
206+
public void danglingSymlinkReplacedWithFile_invalidatesAction() throws Exception {
207+
Path danglingSymlink = sourceDir.getRelative("dangling_symlink");
208+
String target = danglingSymlink.readSymbolicLink().getPathString();
209+
danglingSymlink.delete();
210+
FileSystemUtils.writeContent(danglingSymlink, ISO_8859_1, target);
211+
assertInvalidatedByBuild();
212+
}
213+
205214
@Test
206215
public void crossingPackageBoundary_fails() throws Exception {
207216
FileSystemUtils.touchFile(sourceDir.getRelative("subdir/BUILD"));

0 commit comments

Comments
 (0)