[8.8.0] Fix materialization edge cases in the remote repo contents cache

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<Path> files, List<Path> symlinks) {}
+  private record WalkResult(List<Path> files, List<Path> symlinks, List<Path> 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);
       }
     }

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);
             }

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'])