From 3aae02447b36dd2292348b4f1d699fb6bbaac901 Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
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<EventKind> 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 <fabian@meumertzhe.im>
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";