diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java index b248a14722bb07..2d341fa985e82c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java @@ -187,9 +187,7 @@ public RecursiveFilesystemTraversalValue compute(SkyKey skyKey, Environment env) if (rootInfo.type.isSymlink()) { return RecursiveFilesystemTraversalValue.of( ResolvedFileFactory.danglingSymlink( - traversal.root().asRootedPath(), - rootInfo.unresolvedSymlinkTarget, - rootInfo.metadata)); + traversal.root().asRootedPath(), rootInfo.unresolvedSymlinkTarget)); } else { return RecursiveFilesystemTraversalValue.EMPTY; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java index 72a1aba0ac2c21..ce9fdf833308f0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java @@ -14,12 +14,14 @@ package com.google.devtools.build.lib.skyframe; import static com.google.common.base.Preconditions.checkNotNull; +import static java.nio.charset.StandardCharsets.ISO_8859_1; import com.google.common.base.Objects; import com.google.devtools.build.lib.actions.HasDigest; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyValue; @@ -574,9 +576,15 @@ public static ResolvedFile symlinkToDirectory( return new SymlinkToDirectory(targetPath, linkNamePath, linkValue, metadata); } - public static ResolvedFile danglingSymlink( - RootedPath linkNamePath, PathFragment linkValue, HasDigest metadata) { - return new DanglingSymlink(linkNamePath, linkValue, metadata); + public static ResolvedFile danglingSymlink(RootedPath linkNamePath, PathFragment linkValue) { + byte[] digest = + DigestHashFunction.SHA256 + .getHashFunction() + .hashString(linkValue.getPathString(), ISO_8859_1) + .asBytes(); + // Ensure that the digest does not collide with that of a regular file. + digest[0] ^= 1; + return new DanglingSymlink(linkNamePath, linkValue, new HasDigest.ByteStringDigest(digest)); } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD index 32b758740e9a83..501281fc73c2fc 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD @@ -1921,6 +1921,26 @@ java_test( ], ) +java_test( + name = "SourceDirectoryIntegrationTest", + srcs = ["SourceDirectoryIntegrationTest.java"], + jvm_flags = [ + "-DBAZEL_TRACK_SOURCE_DIRECTORIES=1", + ], + deps = [ + "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/bugreport", + "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/test/java/com/google/devtools/build/lib/buildtool/util", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) + java_test( name = "FileArtifactValueTest", timeout = "short", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java index 85b42897f4d7d4..3ba84d85ac1eda 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java @@ -830,8 +830,7 @@ public void testTraversalOfDanglingSymlink() throws Exception { PathFragment linkTarget = PathFragment.create("non_existent"); parentOf(link).asPath().createDirectory(); link.asPath().createSymbolicLink(linkTarget); - traverseAndAssertFiles( - fileLikeRoot(linkArtifact), danglingSymlink(link, linkTarget, EMPTY_METADATA)); + traverseAndAssertFiles(fileLikeRoot(linkArtifact), danglingSymlink(link, linkTarget)); } @Test @@ -845,7 +844,7 @@ public void testTraversalOfDanglingSymlinkInADirectory() throws Exception { traverseAndAssertFiles( fileLikeRoot(dirArtifact), regularFile(file, EMPTY_METADATA), - danglingSymlink(link, linkTarget, EMPTY_METADATA)); + danglingSymlink(link, linkTarget)); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java new file mode 100644 index 00000000000000..5ccecbc3d8834a --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java @@ -0,0 +1,245 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe; + +import static java.nio.charset.StandardCharsets.ISO_8859_1; +import static org.junit.Assert.assertThrows; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.BuildFailedException; +import com.google.devtools.build.lib.bugreport.BugReport; +import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase; +import com.google.devtools.build.lib.events.EventKind; +import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Integration test for invalidation of actions that consume source directories. */ +@RunWith(JUnit4.class) +public final class SourceDirectoryIntegrationTest extends BuildIntegrationTestCase { + + private Path sourceDir; + + @Override + protected ImmutableSet additionalEventsToCollect() { + return ImmutableSet.of(EventKind.FINISH); + } + + @Before + public void setUpGenrule() throws Exception { + write( + "foo/BUILD", + """ + genrule( + name = "foo", + srcs = ["dir"], + outs = ["foo.out"], + cmd = "touch $@", + ) + """); + + sourceDir = getWorkspace().getRelative("foo/dir"); + sourceDir.createDirectoryAndParents(); + FileSystemUtils.writeIsoLatin1(sourceDir.getRelative("file1"), "content"); + FileSystemUtils.writeIsoLatin1(sourceDir.getRelative("file2"), "content"); + FileSystemUtils.writeIsoLatin1(sourceDir.getRelative("file3"), "other content"); + sourceDir.getRelative("symlink").createSymbolicLink(PathFragment.create("file3")); + sourceDir + .getRelative("dangling_symlink") + .createSymbolicLink(PathFragment.create("does_not_exist")); + + Path subDir = sourceDir.getRelative("subdir"); + subDir.createDirectory(); + FileSystemUtils.writeIsoLatin1(subDir.getRelative("file1"), "content"); + FileSystemUtils.writeIsoLatin1(subDir.getRelative("file2"), "content"); + FileSystemUtils.writeIsoLatin1(subDir.getRelative("file3"), "other content"); + subDir.getRelative("symlink").createSymbolicLink(PathFragment.create("file3")); + subDir + .getRelative("dangling_symlink") + .createSymbolicLink(PathFragment.create("does_not_exist")); + + subDir.getRelative("nested").createDirectory(); + subDir.getRelative("nested2").createDirectory(); + subDir.getRelative("nested_non_empty").createDirectory(); + FileSystemUtils.writeIsoLatin1(subDir.getRelative("nested_non_empty/file1"), "content"); + + buildTarget("//foo"); + assertContainsEvent(events.collector(), "Executing genrule //foo:foo"); + + events.collector().clear(); + } + + @Test + public void nothingModified_doesNotInvalidateAction() throws Exception { + assertNotInvalidatedByBuild(); + } + + @Test + public void touched_doesNotInvalidateAction() throws Exception { + sourceDir.setLastModifiedTime(Path.NOW_SENTINEL_TIME); + assertNotInvalidatedByBuild(); + } + + @Test + public void topLevelFileTouched_doesNotInvalidateAction() throws Exception { + sourceDir.getRelative("file1").setLastModifiedTime(Path.NOW_SENTINEL_TIME); + assertNotInvalidatedByBuild(); + } + + @Test + public void topLevelDirTouched_doesNotInvalidateAction() throws Exception { + sourceDir.getRelative("subdir").setLastModifiedTime(Path.NOW_SENTINEL_TIME); + assertNotInvalidatedByBuild(); + } + + @Test + public void nestedFileTouched_doesNotInvalidateAction() throws Exception { + sourceDir.getRelative("subdir/file1").setLastModifiedTime(Path.NOW_SENTINEL_TIME); + assertNotInvalidatedByBuild(); + } + + @Test + public void nestedDirTouched_doesNotInvalidateAction() throws Exception { + sourceDir.getRelative("subdir/nested").setLastModifiedTime(Path.NOW_SENTINEL_TIME); + assertNotInvalidatedByBuild(); + } + + @Test + public void topLevelFileDeleted_invalidatesAction() throws Exception { + sourceDir.getRelative("file1").delete(); + assertInvalidatedByBuild(); + } + + @Test + public void nestedFileDeleted_invalidatesAction() throws Exception { + sourceDir.getRelative("subdir/file1").delete(); + assertInvalidatedByBuild(); + } + + @Test + public void topLevelFileModified_invalidatesAction() throws Exception { + FileSystemUtils.writeIsoLatin1(sourceDir.getRelative("file1"), "modified content"); + assertInvalidatedByBuild(); + } + + @Test + public void nestedFileModified_invalidatesAction() throws Exception { + FileSystemUtils.writeIsoLatin1(sourceDir.getRelative("subdir/file1"), "modified content"); + assertInvalidatedByBuild(); + } + + @Test + public void topLevelFileAdded_invalidatesAction() throws Exception { + FileSystemUtils.writeIsoLatin1(sourceDir.getRelative("new_file"), "modified content"); + assertInvalidatedByBuild(); + } + + @Test + public void nestedFileAdded_invalidatesAction() throws Exception { + FileSystemUtils.writeIsoLatin1(sourceDir.getRelative("subdir/new_file"), "modified content"); + assertInvalidatedByBuild(); + } + + @Test + public void emptyDirDeleted_invalidatesAction() throws Exception { + sourceDir.getRelative("subdir/nested").delete(); + assertInvalidatedByBuild(); + } + + @Test + public void fileReplacedByIdenticalSymlink_doesNotInvalidateAction() throws Exception { + Path file = sourceDir.getRelative("file1"); + file.delete(); + file.createSymbolicLink(sourceDir.getRelative("file2")); + assertNotInvalidatedByBuild(); + } + + @Test + public void fileReplacedByDifferentSymlink_invalidatesAction() throws Exception { + Path file = sourceDir.getRelative("file1"); + file.delete(); + file.createSymbolicLink(sourceDir.getRelative("file3")); + assertInvalidatedByBuild(); + } + + @Test + @Ignore("TODO(#25834)") + public void emptyDirReplacedWithIdenticalSymlink_doesNotInvalidateAction() throws Exception { + Path dir = sourceDir.getRelative("subdir/nested2"); + dir.delete(); + dir.createSymbolicLink(PathFragment.create("nested")); + assertNotInvalidatedByBuild(); + } + + @Test + public void emptyDirReplacedWithDifferentSymlink_invalidatesAction() throws Exception { + Path dir = sourceDir.getRelative("subdir/nested2"); + dir.delete(); + dir.createSymbolicLink(PathFragment.create("nested_non_empty")); + assertInvalidatedByBuild(); + } + + @Test + public void danglingSymlinkModified_invalidatesAction() throws Exception { + FileSystemUtils.ensureSymbolicLink( + sourceDir.getRelative("dangling_symlink"), PathFragment.create("still_does_not_exist")); + assertInvalidatedByBuild(); + } + + @Test + public void danglingSymlinkReplacedWithFile_invalidatesAction() throws Exception { + Path danglingSymlink = sourceDir.getRelative("dangling_symlink"); + String target = danglingSymlink.readSymbolicLink().getPathString(); + danglingSymlink.delete(); + FileSystemUtils.writeContent(danglingSymlink, ISO_8859_1, target); + assertInvalidatedByBuild(); + } + + @Test + public void crossingPackageBoundary_fails() throws Exception { + FileSystemUtils.touchFile(sourceDir.getRelative("subdir/BUILD")); + // TODO(#25834): This should not crash Bazel. + assertThrows(IllegalStateException.class, () -> buildTarget("//foo")); + BugReport.getAndResetLastCrashingThrowableIfInTest(); + assertContainsEvent( + "Directory artifact foo/dir crosses package boundary into package rooted at foo/dir/subdir"); + } + + @Test + public void infiniteSymlinkExpansion_fails() throws Exception { + Path dir = sourceDir.getRelative("subdir/nested2"); + dir.delete(); + dir.createSymbolicLink(PathFragment.create("..")); + assertThrows(BuildFailedException.class, () -> buildTarget("//foo")); + assertContainsEvent("infinite symlink expansion detected"); + assertContainsEvent("foo/dir/subdir/nested2"); + } + + private static final String GENRULE_EVENT = "Executing genrule //foo:foo"; + + private void assertInvalidatedByBuild() throws Exception { + buildTarget("//foo"); + assertContainsEvent(events.collector(), GENRULE_EVENT); + } + + private void assertNotInvalidatedByBuild() throws Exception { + buildTarget("//foo"); + assertDoesNotContainEvent(events.collector(), GENRULE_EVENT); + } +}