From 4674c7dc07aaf61a3fc0385826e1787671060f2d Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 18 Nov 2025 02:11:25 -0800 Subject: [PATCH 1/4] [8.8.0] Fix NPE with remote repo contents cache I haven't been able to reproduce this in a test, but this should fix the following crash observed while running `bazel info`: ``` FATAL: bazel crashed due to an internal error. Printing stack trace: java.lang.NullPointerException: Cannot invoke "java.util.concurrent.ExecutorService.shutdownNow()" because "this.materializationExecutor" is null at com.google.devtools.build.lib.remote.RemoteExternalOverlayFileSystem.afterCommand(RemoteExternalOverlayFileSystem.java:145) at com.google.devtools.build.lib.remote.RemoteModule.afterCommand(RemoteModule.java:1034) at com.google.devtools.build.lib.runtime.BlazeRuntime.afterCommand(BlazeRuntime.java:787) at com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.execExclusively(BlazeCommandDispatcher.java:807) at com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.exec(BlazeCommandDispatcher.java:266) at com.google.devtools.build.lib.server.GrpcServerImpl.executeCommand(GrpcServerImpl.java:608) at com.google.devtools.build.lib.server.GrpcServerImpl.lambda$run$0(GrpcServerImpl.java:679) at io.grpc.Context$1.run(Context.java:566) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) at java.base/java.lang.Thread.run(Unknown Source) ``` Closes #27690. PiperOrigin-RevId: 833722608 Change-Id: I88c485a01e5967657ec3b5529a47639b743b18e6 (cherry picked from commit a7d0e91dce87cde6f9bd8c1e0139db2cd07df4d9) --- .../build/lib/remote/RemoteExternalOverlayFileSystem.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java index 89c7b61f75b256..3425f500a697e6 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java @@ -145,6 +145,11 @@ public void beforeCommand( } public void afterCommand() { + if (cache == null) { + // Not all commands cause beforeCommand to be called, but afterCommand is called + // unconditionally. + return; + } this.cache = null; this.inputPrefetcher = null; this.reporter = null; From 4e4919b1296f22de02a9dc01c5dea9889dec8401 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 18 Nov 2025 20:53:30 -0800 Subject: [PATCH 2/4] [8.8.0] Make remote repo contents cache less spammy Don't print a message when it's successful. Users can always look under `external` to verify which repo came from the cache. Closes #27699. PiperOrigin-RevId: 834096735 Change-Id: I3916fb240218a6b68ecf48417142b998ca281598 (cherry picked from commit 3ca9ce13423393b90564710670b09486955383af) --- .../lib/rules/repository/RepositoryDelegatorFunction.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java index 57e40d770200f8..0f7e84d82ff90d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -268,10 +268,6 @@ public SkyValue compute(SkyKey skyKey, Environment env) try { if (remoteRepoContentsCache.lookupCache( repositoryName, repoRoot, digestWriter.predeclaredInputHash, env.getListener())) { - env.getListener() - .handle( - Event.debug( - "Got %s from the remote repo contents cache".formatted(repositoryName))); return new RepositoryDirectoryValue.Success( repoRoot, /* isFetchingDelayed= */ false, excludeRepoFromVendoring); } From 7545af3373c5b2677d612fa85d9eb5e7f7067c35 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 24 Nov 2025 01:46:40 -0800 Subject: [PATCH 3/4] Fix materialization edge cases in the remote repo contents cache Fixes the creation of empty directories and also contains a speculative fix for the following issue observed during a sequence of real builds: ``` Error in path: Failed to materialize remote repo @@protoc-gen-validate+: [unix_jni.cc:302] /home/ubuntu/.cache/bazel/_bazel_ubuntu/123/external/protoc-gen-validate+/example-workspace/.bazelrc (File exists) ERROR: //:foo :: Error loading option //:foo: error evaluating module extension @@gazelle+//:extensions.bzl%go_deps ``` The mentioned file is a symlink. Closes #27711. PiperOrigin-RevId: 836122472 Change-Id: I8becd8c3640a659d28dc433340db962c18563d9f (cherry picked from commit b27ea05b2ce0864284b38bf3539f8c2fe019ef64) --- .../remote/RemoteExternalOverlayFileSystem.java | 17 +++++++++++------ .../bzlmod/remote_repo_contents_cache_test.py | 9 +++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java index 3425f500a697e6..1b02d22eb34255 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java @@ -275,6 +275,9 @@ private void doMaterialize(RepositoryName repo, ExtendedEventHandler reporter) var repoPath = externalDirectory.getChild(repo.getName()); var remoteRepo = externalFs.getPath(repoPath); var walkResult = walk(remoteRepo); + for (var directory : walkResult.directories()) { + nativeFs.getPath(directory.asFragment()).createDirectory(); + } var unused = getFromFuture( inputPrefetcher.prefetchFiles( @@ -285,11 +288,10 @@ private void doMaterialize(RepositoryName repo, ExtendedEventHandler reporter) ActionInputPrefetcher.Priority.CRITICAL, ActionInputPrefetcher.Reason.INPUTS)); // Create symlinks last as some platforms don't allow creating a symlink to a non-existent - // target. + // target. A symlink may have already been created as an input to an action. for (var remoteSymlink : walkResult.symlinks()) { var nativeSymlink = nativeFs.getPath(remoteSymlink.asFragment()); - nativeSymlink.getParentDirectory().createDirectoryAndParents(); - nativeSymlink.createSymbolicLink(remoteSymlink.readSymbolicLink()); + FileSystemUtils.ensureSymbolicLink(nativeSymlink, remoteSymlink.readSymbolicLink()); } // After the repo has been copied, atomically materialize the marker file. This ensures that the @@ -302,10 +304,10 @@ private void doMaterialize(RepositoryName repo, ExtendedEventHandler reporter) markerFileSibling.renameTo(markerFile); } - private record WalkResult(List files, List symlinks) {} + private record WalkResult(List files, List symlinks, List directories) {} private static WalkResult walk(Path root) throws IOException { - var result = new WalkResult(new ArrayList<>(), new ArrayList<>()); + var result = new WalkResult(new ArrayList<>(), new ArrayList<>(), new ArrayList<>()); walk(root, result); return result; } @@ -316,7 +318,10 @@ private static void walk(Path root, WalkResult result) throws IOException { switch (dirent.getType()) { case FILE -> result.files.add(fromChild); case SYMLINK -> result.symlinks.add(fromChild); - case DIRECTORY -> walk(fromChild, result); + case DIRECTORY -> { + result.directories.add(fromChild); + walk(fromChild, result); + } default -> throw new IOException("Unsupported file type: " + dirent); } } diff --git a/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py b/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py index 57a941e824fa9b..12bf9e29486280 100644 --- a/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py +++ b/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py @@ -314,6 +314,10 @@ def testAccessFromOtherRepo_read(self): [ 'def _repo_impl(rctx):', ' rctx.file("BUILD", "filegroup(name=\'haha\')")', + # Verify that directories are materialized correctly. + ' rctx.file("subdir/file.txt", "hello")', + ' rctx.file("subdir/empty_dir/.keep")', + ' rctx.delete("subdir/empty_dir/.keep")', ' print("JUST FETCHED")', ' return rctx.repo_metadata(reproducible=True)', 'repo = repository_rule(_repo_impl)', @@ -345,12 +349,17 @@ def testAccessFromOtherRepo_read(self): _, _, stderr = self.RunBazel(['build', '@my_repo//:haha']) self.assertNotIn('JUST FETCHED', '\n'.join(stderr)) self.assertFalse(os.path.exists(os.path.join(repo_dir, 'BUILD'))) + self.assertFalse(os.path.exists(os.path.join(repo_dir, 'subdir'))) # Fetch other: my_repo materialized _, _, stderr = self.RunBazel(['build', '@other//:haha']) self.assertNotIn('JUST FETCHED', '\n'.join(stderr)) self.assertTrue(os.path.exists(os.path.join(repo_dir, 'BUILD'))) self.assertTrue(os.path.exists(os.path.join(other_repo_dir, 'BUILD'))) + self.assertTrue(os.path.exists(os.path.join(repo_dir, 'subdir/file.txt'))) + with open(os.path.join(repo_dir, 'subdir/file.txt')) as f: + self.assertEqual(f.read(), 'hello') + self.assertTrue(os.path.exists(os.path.join(repo_dir, 'subdir/empty_dir'))) # Materialized repo is not refetched after a shutdown self.RunBazel(['shutdown']) From 96f4080ffa016652aeb4888e1bad4c0f6694af46 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 28 Nov 2025 06:48:24 -0800 Subject: [PATCH 4/4] [8.7.0] Fix `RemoteExternalOverlayFileSystem#resolveSymbolicLinks` Ensures that the returned `Path` is still in the overlay file system. Also make the error message emitted by `Path#checkSameFileSystem` more informative. This is motivated by and helped discover the above as the fix for the following crash observed when using the remote repo contents cache with an explicit `--sandbox_base`: ``` Caused by: java.lang.IllegalArgumentException: Files are on different filesystems: /dev/shm/bazel-sandbox.b10976335efa519b0184f3091ac8e21f7beefb92142303f9ab2c3341f45a2f28/linux-sandbox/18/execroot/_main/external/c-ares+/configs/ares_build.h (on com.google.devtools.build.lib.unix.UnixFileSystem@5e0a8154), /home/ubuntu/.cache/bazel/_bazel_ubuntu/123/execroot/_main/external/c-ares+/configs/ares_build.h (on com.google.devtools.build.lib.remote.RemoteExternalOverlayFileSystem@6cd9bfda) at com.google.devtools.build.lib.vfs.Path.checkSameFileSystem(Path.java:964) at com.google.devtools.build.lib.vfs.Path.createSymbolicLink(Path.java:523) at com.google.devtools.build.lib.vfs.Path.createSymbolicLink(Path.java:535) at com.google.devtools.build.lib.sandbox.SymlinkedSandboxedSpawn.copyFile(SymlinkedSandboxedSpawn.java:129) ``` Alternative to https://github.com/bazelbuild/bazel/pull/27721 Closes #27802. PiperOrigin-RevId: 837832265 Change-Id: I3b73167496b011aef66954d59ca3804b4b64996f (cherry picked from commit 8eaf6a90db1d0e95fbade4dd2b7e3b8c57d03629) --- .../RemoteExternalOverlayFileSystem.java | 3 ++- .../google/devtools/build/lib/vfs/Path.java | 3 ++- .../bzlmod/remote_repo_contents_cache_test.py | 21 ++++++++++++++++--- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java index 1b02d22eb34255..39fdaf35d58744 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java @@ -525,7 +525,8 @@ public byte[] getxattr(PathFragment path, String name, boolean followSymlinks) @Override public Path resolveSymbolicLinks(PathFragment path) throws IOException { - return fsForPath(path).resolveSymbolicLinks(path); + // Ensure that the return value doesn't leave the overlay file system. + return getPath(fsForPath(path).resolveSymbolicLinks(path).asFragment()); } @Nullable diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java index b48d483e7c487a..d62a3da4784456 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java @@ -948,7 +948,8 @@ public void prefetchPackageAsync(int maxDirs) { private void checkSameFileSystem(Path that) { if (this.fileSystem != that.fileSystem) { throw new IllegalArgumentException( - "Files are on different filesystems: " + this + ", " + that); + "Files are on different filesystems: %s (on %s), %s (on %s)" + .formatted(this, this.fileSystem, that, that.fileSystem)); } } } diff --git a/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py b/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py index 12bf9e29486280..c901e9b216c900 100644 --- a/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py +++ b/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py @@ -482,7 +482,11 @@ def testUseRepoFileInBuildRule_actionUsesCache(self): with open(self.Path('bazel-bin/main/out.txt')) as f: self.assertEqual(f.read(), 'hello') - def testUseRepoFileInBuildRule_actionDoesNotUseCache(self): + def do_testUseRepoFileInBuildRule_actionDoesNotUseCache( + self, extra_flags=None + ): + if extra_flags is None: + extra_flags = [] self.ScratchFile( 'MODULE.bazel', [ @@ -518,7 +522,7 @@ def testUseRepoFileInBuildRule_actionDoesNotUseCache(self): repo_dir = self.RepoDir('my_repo') # First fetch: not cached - _, _, stderr = self.RunBazel(['build', '//main:use_data']) + _, _, stderr = self.RunBazel(['build', '//main:use_data'] + extra_flags) self.assertIn('JUST FETCHED', '\n'.join(stderr)) self.assertTrue(os.path.exists(os.path.join(repo_dir, 'BUILD'))) self.assertTrue(os.path.exists(os.path.join(repo_dir, 'data.txt'))) @@ -528,7 +532,7 @@ def testUseRepoFileInBuildRule_actionDoesNotUseCache(self): # After expunging: repo and build action cached self.RunBazel(['clean', '--expunge']) - _, _, stderr = self.RunBazel(['build', '//main:use_data']) + _, _, stderr = self.RunBazel(['build', '//main:use_data'] + extra_flags) self.assertNotIn('JUST FETCHED', '\n'.join(stderr)) self.assertFalse(os.path.exists(os.path.join(repo_dir, 'BUILD'))) self.assertTrue(os.path.exists(os.path.join(repo_dir, 'data.txt'))) @@ -536,6 +540,17 @@ def testUseRepoFileInBuildRule_actionDoesNotUseCache(self): with open(self.Path('bazel-bin/main/out.txt')) as f: self.assertEqual(f.read(), 'hello') + def testUseRepoFileInBuildRule_actionDoesNotUseCache(self): + self.do_testUseRepoFileInBuildRule_actionDoesNotUseCache() + + def testUseRepoFileInBuildRule_actionDoesNotUseCache_withExplicitSandboxBase( + self, + ): + tmpdir = self.ScratchDir('sandbox_base') + self.do_testUseRepoFileInBuildRule_actionDoesNotUseCache( + extra_flags=['--sandbox_base=' + tmpdir] + ) + def testLostRemoteFile_build(self): # Create a repo with two BUILD files (one in a subpackage), build a target # from one to cause it to be cached, then build that target again after