[8.8.0] Fix remote repo content caching for non-gRPC backends

src/main/java/com/google/devtools/build/lib/BUILD

@@ -288,6 +288,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//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/skyframe:skyframe-objects",
     ],
 )
 

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD

@@ -17,6 +17,7 @@ java_library(
     deps = [
         "//src/main/java/com/google/devtools/build/docgen/annot",
         "//src/main/java/com/google/devtools/build/lib:runtime",
+        "//src/main/java/com/google/devtools/build/lib:runtime/remote_repo_contents_cache",
         "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java

@@ -47,6 +47,7 @@
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
 import com.google.devtools.build.lib.rules.repository.WorkspaceFileHelper;
 import com.google.devtools.build.lib.runtime.ProcessWrapper;
+import com.google.devtools.build.lib.runtime.RemoteRepoContentsCache;
 import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
 import com.google.devtools.build.lib.skyframe.IgnoredSubdirectoriesValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
@@ -125,13 +126,20 @@ public void reportSkyframeRestart(Environment env, RepositoryName repoName) {
 
   private static class State extends WorkerSkyKeyComputeState<FetchResult> {
     @Nullable FetchResult result;
+    final RemoteRepoContentsCache.LookupState repoContentsCacheLookupState =
+        new RemoteRepoContentsCache.LookupState();
   }
 
   @Override
   public boolean wasJustFetched(Environment env) {
     return env.getState(State::new).result != null;
   }
 
+  @Override
+  public RemoteRepoContentsCache.LookupState getRemoteRepoContentsCacheLookupState(Environment env) {
+    return env.getState(State::new).repoContentsCacheLookupState;
+  }
+
   private record FetchArgs(
       Rule rule, Path outputDirectory, BlazeDirectories directories, Environment env, SkyKey key) {
     FetchArgs toWorkerArgs(Environment env) {

src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java

@@ -45,6 +45,7 @@
 import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
 import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValueWithMaterializationData;
 import com.google.devtools.build.lib.actions.FileContentsProxy;
+import com.google.devtools.build.lib.actions.FileStateType;
 import com.google.devtools.build.lib.actions.cache.OutputMetadataStore;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
@@ -247,6 +248,17 @@ protected Path execRoot() {
     return execRootSupplier.get();
   }
 
+  /**
+   * Returns whether the given path lives under the external repository root. Unlike {@link
+   * #execRoot()}, this can be evaluated during external repository materialization, where the exec
+   * root isn't available yet.
+   */
+  private boolean isUnderExternalRepoRoot(Path path) {
+    return path.asFragment()
+        .startsWith(
+            outputBase.getRelative(LabelConstants.EXTERNAL_REPOSITORY_LOCATION).asFragment());
+  }
+
   /**
    * Resolves an exec path to an absolute path, avoiding evaluation of execRoot() if possible as it
    * isn't available during server startup. This logic is unique to Bazel 8.x as it still names the
@@ -412,7 +424,19 @@ private ListenableFuture<Void> prefetchFile(
       PathFragment execPath = input.getExecPath();
 
       FileArtifactValue metadata = metadataSupplier.getMetadata(input);
-      if (metadata == null || !canDownloadFile(resolveExecPath(execPath), metadata)) {
+      if (metadata == null) {
+        return immediateVoidFuture();
+      }
+      Path inputPath = resolveExecPath(execPath);
+      if (metadata.getType() == FileStateType.SYMLINK && isUnderExternalRepoRoot(inputPath)) {
+        return toListenableFuture(
+            plantUnresolvedSymlink(
+                inputPath.forHostFileSystem(),
+                PathFragment.create(
+                    ((FileArtifactValue.UnresolvedSymlinkArtifactValue) metadata)
+                        .getSymlinkTarget())));
+      }
+      if (!canDownloadFile(inputPath, metadata)) {
         return immediateVoidFuture();
       }
 
@@ -752,6 +776,17 @@ private Completable plantSymlink(Symlink symlink) {
         forceRefetch(resolveExecPath(symlink.linkExecPath())));
   }
 
+  private Completable plantUnresolvedSymlink(Path linkPath, PathFragment target) {
+    return downloadCache.executeIfNot(
+        linkPath,
+        Completable.defer(
+            () -> {
+              linkPath.delete();
+              linkPath.createSymbolicLink(target);
+              return Completable.complete();
+            }));
+  }
+
   public ImmutableSet<Path> downloadedFiles() {
     return downloadCache.getFinishedTasks();
   }

src/main/java/com/google/devtools/build/lib/remote/BUILD

@@ -119,6 +119,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/remote/util",
         "//src/main/java/com/google/devtools/build/lib/remote/util:digest_utils",
         "//src/main/java/com/google/devtools/build/lib/remote/zstd",
+        "//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
         "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
         "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",

src/main/java/com/google/devtools/build/lib/remote/CombinedCache.java

@@ -211,6 +211,15 @@ public CachedActionResult downloadActionResult(
       boolean inlineOutErr,
       Set<String> inlineOutputFiles)
       throws IOException, InterruptedException {
+    return getFromFuture(
+        downloadActionResultAsync(context, actionKey, inlineOutErr, inlineOutputFiles));
+  }
+
+  public ListenableFuture<CachedActionResult> downloadActionResultAsync(
+      RemoteActionExecutionContext context,
+      ActionKey actionKey,
+      boolean inlineOutErr,
+      Set<String> inlineOutputFiles) {
     var spawnExecutionContext = context.getSpawnExecutionContext();
 
     ListenableFuture<CachedActionResult> future = immediateFuture(null);
@@ -254,7 +263,7 @@ public CachedActionResult downloadActionResult(
               directExecutor());
     }
 
-    return getFromFuture(future);
+    return future;
   }
 
   private ListenableFuture<ActionResult> downloadActionResultFromRemote(

src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java

@@ -193,11 +193,13 @@ public void afterCommand() {
    */
   public boolean injectRemoteRepo(RepositoryName repo, Tree remoteContents, String markerFile)
       throws IOException, InterruptedException {
+    var repoDir = externalDirectory.getChild(repo.getName());
+    deleteTree(repoDir);
+    var unused = delete(externalDirectory.getChild(repo.getMarkerFileName()));
     var childMap =
         remoteContents.getChildrenList().stream()
             .collect(
                 toImmutableMap(cache.digestUtil::compute, directory -> directory, (a, b) -> a));
-    var repoDir = externalDirectory.getChild(repo.getName());
     var filesToPrefetch = new ArrayList<PathFragment>();
     injectRecursively(
         externalFs,
@@ -220,7 +222,7 @@ public boolean injectRemoteRepo(RepositoryName repo, Tree remoteContents, String
     }
     // Create the repo directory on disk so that readdir reflects the overlaid state of the external
     // directory.
-    nativeFs.createDirectoryAndParents(externalDirectory.getChild(repo.getName()));
+    nativeFs.createDirectoryAndParents(repoDir);
     // Keep the marker file contents in memory so that it can be written out when the repo is
     // materialized. This doubles as a presence marker for the in-memory repo contents.
     markerFileContents.put(repo.getName(), markerFile);
@@ -313,12 +315,8 @@ private void doMaterialize(RepositoryName repo, ExtendedEventHandler reporter)
     }
     prefetch(walkResult.files());
     // Create symlinks last as some platforms don't allow creating a symlink to a non-existent
-    // target. A symlink may have already been created as an input to an action.
-    for (var remoteSymlink : walkResult.symlinks()) {
-      var nativeSymlink = nativeFs.getPath(remoteSymlink);
-      FileSystemUtils.ensureSymbolicLink(
-          nativeSymlink, externalFs.getPath(remoteSymlink).readSymbolicLink());
-    }
+    // target.
+    prefetch(walkResult.symlinks());
 
     // After the repo has been copied, atomically materialize the marker file. This ensures that the
     // repo doesn't have to be refetched after the next server restart.
@@ -654,9 +652,11 @@ private RemoteActionExecutionContext makeRemoteContext(PathFragment relativePath
     }
 
     private FileArtifactValue getMetadata(PathFragment path) throws IOException {
-      var info =
-          (RemoteActionFileSystem.RemoteInMemoryFileInfo) stat(path, /* followSymlinks= */ true);
-      return info.getMetadata();
+      var status = stat(path, /* followSymlinks= */ false);
+      if (!status.isSymbolicLink()) {
+        return ((RemoteActionFileSystem.RemoteInMemoryFileInfo) status).getMetadata();
+      }
+      return FileArtifactValue.createForUnresolvedSymlink(externalFs.getPath(path));
     }
 
     @Override

src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java

@@ -323,6 +323,43 @@ private RemoteActionInputFetcher createActionInputFetcher(@Nullable CombinedCach
         outputPermissions);
   }
 
+  /**
+   * Initializes the repository remote helpers factory and primes the {@link
+   * RemoteExternalOverlayFileSystem} (when one is in use) with the per-build state it needs.
+   */
+  private void initRepoHelpersAndOverlayFs(
+      CommandEnvironment env, String buildRequestId, String invocationId, boolean verboseFailures) {
+    if (actionContextProvider == null) {
+      return;
+    }
+    CombinedCache combinedCache = actionContextProvider.getCombinedCache();
+    if (combinedCache == null) {
+      return;
+    }
+    repositoryRemoteHelpersFactoryDelegate.init(
+        new RepositoryRemoteHelpersFactoryImpl(
+            env.getDirectories(),
+            combinedCache,
+            actionContextProvider.getRemoteExecutionClient(),
+            buildRequestId,
+            invocationId,
+            remoteOptions.remoteInstanceName,
+            remoteOptions.remoteAcceptCached,
+            remoteOptions.remoteUploadLocalResults,
+            verboseFailures));
+    if (env.getDirectories().getOutputBase().getFileSystem()
+        instanceof RemoteExternalOverlayFileSystem remoteFs) {
+      remoteFs.beforeCommand(
+          combinedCache,
+          actionInputFetcher,
+          env.getReporter(),
+          buildRequestId,
+          invocationId,
+          env.getSkyframeExecutor().getEvaluator(),
+          remoteOptions.remoteCacheTtl);
+    }
+  }
+
   @Override
   public void workspaceInit(
       BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) {
@@ -571,6 +608,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
 
     if ((enableHttpCache || enableDiskCache) && !enableGrpcCache) {
       initHttpAndDiskCache(env, credentials, authAndTlsOptions, remoteOptions, digestUtil);
+      initRepoHelpersAndOverlayFs(env, buildRequestId, invocationId, verboseFailures);
       return;
     }
 
@@ -760,27 +798,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
 
     actionInputFetcher = createActionInputFetcher(actionContextProvider.getCombinedCache());
 
-    repositoryRemoteHelpersFactoryDelegate.init(
-        new RepositoryRemoteHelpersFactoryImpl(
-            actionContextProvider.getCombinedCache(),
-            actionContextProvider.getRemoteExecutionClient(),
-            buildRequestId,
-            invocationId,
-            remoteOptions.remoteInstanceName,
-            remoteOptions.remoteAcceptCached,
-            remoteOptions.remoteUploadLocalResults,
-            verboseFailures));
-    if (env.getDirectories().getOutputBase().getFileSystem()
-        instanceof RemoteExternalOverlayFileSystem remoteFs) {
-      remoteFs.beforeCommand(
-          actionContextProvider.getCombinedCache(),
-          actionInputFetcher,
-          env.getReporter(),
-          buildRequestId,
-          invocationId,
-          env.getSkyframeExecutor().getEvaluator(),
-          remoteOptions.remoteCacheTtl);
-    }
+    initRepoHelpersAndOverlayFs(env, buildRequestId, invocationId, verboseFailures);
 
     buildEventArtifactUploaderFactoryDelegate.init(
         new ByteStreamBuildEventArtifactUploaderFactory(
@@ -1231,6 +1249,11 @@ RemoteActionContextProvider getActionContextProvider() {
     return actionContextProvider;
   }
 
+  @VisibleForTesting
+  RepositoryRemoteHelpersFactory getRepositoryRemoteHelpersFactoryDelegate() {
+    return repositoryRemoteHelpersFactoryDelegate;
+  }
+
   @VisibleForTesting
   static Credentials createCredentials(
       CredentialHelperEnvironment credentialHelperEnvironment,