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..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 @@ -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; @@ -270,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( @@ -280,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 @@ -297,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; } @@ -311,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/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); } 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'])