From 3aae02447b36dd2292348b4f1d699fb6bbaac901 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 15 Apr 2025 14:19:42 +0200 Subject: [PATCH 1/2] Add test for source directory invalidation --- .../google/devtools/build/lib/skyframe/BUILD | 19 ++ .../SourceDirectoryIntegrationTest.java | 232 ++++++++++++++++++ 2 files changed, 251 insertions(+) create mode 100644 src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java 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..58d734feacb8dc 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,25 @@ 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/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/SourceDirectoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java new file mode 100644 index 00000000000000..f6b101be45d21c --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java @@ -0,0 +1,232 @@ +// 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 org.junit.Assert.assertThrows; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.BuildFailedException; +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 infiniteSymlinkExpansion() 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"); + } + + @Test + @Ignore("TODO(#25834)") + public void danglingSymlinkModified_invalidatesAction() throws Exception { + FileSystemUtils.ensureSymbolicLink( + sourceDir.getRelative("dangling_symlink"), PathFragment.create("still_does_not_exist")); + assertInvalidatedByBuild(); + } + + @Test + @Ignore("TODO(#25834)") + public void subPackageAdded_invalidatesAction() throws Exception { + FileSystemUtils.touchFile(sourceDir.getRelative("subdir/BUILD")); + assertInvalidatedByBuild(); + } + + 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); + } +} From 1aaf2517effd8cd031c0abb482d6127697125702 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 15 Apr 2025 19:13:14 +0200 Subject: [PATCH 2/2] Prohibit subpackage traversals --- .../google/devtools/build/lib/skyframe/BUILD | 1 + .../SourceDirectoryIntegrationTest.java | 30 +++++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) 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 58d734feacb8dc..501281fc73c2fc 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD @@ -1930,6 +1930,7 @@ java_test( 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", 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 index f6b101be45d21c..20d6b0132a2863 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java @@ -17,6 +17,7 @@ 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; @@ -193,16 +194,6 @@ public void emptyDirReplacedWithDifferentSymlink_invalidatesAction() throws Exce assertInvalidatedByBuild(); } - @Test - public void infiniteSymlinkExpansion() 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"); - } - @Test @Ignore("TODO(#25834)") public void danglingSymlinkModified_invalidatesAction() throws Exception { @@ -212,10 +203,23 @@ public void danglingSymlinkModified_invalidatesAction() throws Exception { } @Test - @Ignore("TODO(#25834)") - public void subPackageAdded_invalidatesAction() throws Exception { + public void crossingPackageBoundary_fails() throws Exception { FileSystemUtils.touchFile(sourceDir.getRelative("subdir/BUILD")); - assertInvalidatedByBuild(); + // 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";