Repo contents cache

This mostly follows the design doc at https://docs.google.com/document/d/1ZScqiIQi9l7_8eikbsGI-rjupdbCI7wgm1RYD76FJfM/edit?pli=1&tab=t.0#heading=h.5mcn15i0e1ch

- Adds a new flag `--repo_contents_cache` to specify the path of the repo contents cache, where we'll store fetched contents of repos that can be cached safely across workspaces.
  - The flag defaults to `<value of --repository_cache>/contents`.
  - If this path ends up being inside the main repo, we throw an error. This is because files inside the source tree are considered immutable during the lifetime of a Bazel invocation, which means writing into the repo contents cache can cause spurious failures.
- A repo rule can indicate readiness for caching by returning `repository_ctx.repo_metadata(reproducible=True)` from its implementation function, similarly to module extensions.
  - Note that reproducibility/cacheability is not per repo rule (like "local"-ness), but per repo. Example: `http_archive` is only cacheable if the checksum is provided.
- Before we fetch a repo, we first check if matching entries exist in the repo contents cache under the key of the hash of all its predeclared inputs (`HVal(Pre(R))` in the doc). If not, fetch as normal.
   - These include repo attrs, repo rule impl .bzl hashes, Starlark semantics, etc.
- If matching entries exist, we go through each of them and examine if it's up-to-date by examining its "recorded inputs file" (analogous to the marker file in `outputBase/external`). If we find an up-to-date entry, we set up a symlink, and declare victory. Otherwise, fetch as normal.
- After fetching, if the repo rule indicates cacheability, we move the fetched contents into the repo contents cache by appending a new entry under the predeclared inputs hash key.

RELNOTES: Added a new flag `--repo_contents_cache` (defaults to the `contents` directory under the `--repository_cache`) where Bazel stores fetched contents of repos that can be safely cached across workspaces. A repo rule can indicate cacheability by returning `repository_ctx.repo_metadata(reproducible=True)` from its implementation function.

Work towards #12227.

Closes #25938.

PiperOrigin-RevId: 758682171
Change-Id: Ie703152a98745f7382c3d095a1dbf4b35c3408eb

Author: Wyverald

scripts/bootstrap/bootstrap.sh

@@ -38,6 +38,7 @@ _BAZEL_ARGS="--spawn_strategy=standalone \
       --strategy=Javac=worker --worker_quit_after_build --ignore_unsupported_sandboxing \
       --compilation_mode=opt \
       --repository_cache=derived/repository_cache \
+      --repo_contents_cache= \
       --repo_env=BAZEL_HTTP_RULES_URLS_AS_DEFAULT_CANONICAL_ID=0 \
       --extra_toolchains=//scripts/bootstrap:all \
       --extra_toolchains=@rules_python//python:autodetecting_toolchain \

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

@@ -356,35 +356,41 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
         starlarkRepositoryFunction.setTimeoutScaling(1.0);
         singleExtensionEvalFunction.setTimeoutScaling(1.0);
       }
-      if (repoOptions.experimentalRepositoryCache != null) {
-        Path repositoryCachePath;
-        if (repoOptions.experimentalRepositoryCache.isEmpty()) {
-          // A set but empty path indicates a request to disable the repository cache.
-          repositoryCachePath = null;
-        } else if (repoOptions.experimentalRepositoryCache.isAbsolute()) {
-          repositoryCachePath = filesystem.getPath(repoOptions.experimentalRepositoryCache);
-        } else {
-          repositoryCachePath =
-              env.getBlazeWorkspace()
-                  .getWorkspace()
-                  .getRelative(repoOptions.experimentalRepositoryCache);
-        }
-        repositoryCache.setPath(repositoryCachePath);
+      if (repoOptions.repositoryCache != null) {
+        repositoryCache.setPath(toPath(repoOptions.repositoryCache, env));
       } else {
-        Path repositoryCachePath =
+        repositoryCache.setPath(
             env.getDirectories()
                 .getServerDirectories()
                 .getOutputUserRoot()
-                .getRelative(DEFAULT_CACHE_LOCATION);
-        try {
-          repositoryCachePath.createDirectoryAndParents();
-          repositoryCache.setPath(repositoryCachePath);
-        } catch (IOException e) {
-          env.getReporter()
-              .handle(
-                  Event.warn(
-                      "Failed to set up cache at " + repositoryCachePath + ": " + e.getMessage()));
-        }
+                .getRelative(DEFAULT_CACHE_LOCATION));
+      }
+      // Note that the repo contents cache stuff has to happen _after_ the repo cache stuff, because
+      // the specific settings about the repo contents cache might overwrite the repo cache
+      // settings. In particular, if `--repo_contents_cache` is not set (it's null), we use whatever
+      // default set by `repositoryCache.setPath(...)`.
+      if (repoOptions.repoContentsCache != null) {
+        repositoryCache.getRepoContentsCache().setPath(toPath(repoOptions.repoContentsCache, env));
+      }
+      Path repoContentsCachePath = repositoryCache.getRepoContentsCache().getPath();
+      if (repoContentsCachePath != null
+          && env.getWorkspace() != null
+          && repoContentsCachePath.startsWith(env.getWorkspace())) {
+        // Having the repo contents cache inside the workspace is very dangerous. During the
+        // lifetime of a Bazel invocation, we treat files inside the workspace as immutable. This
+        // can cause mysterious failures if we write files inside the workspace during the
+        // invocation, as is often the case with the repo contents cache.
+        // TODO: wyv@ - This is a crude check that disables some use cases (such as when the output
+        //   base itself is inside the main repo). Investigate a better check.
+        throw new AbruptExitException(
+            detailedExitCode(
+                """
+                The repo contents cache [%s] is inside the workspace [%s]. This can cause spurious \
+                failures. Disable the repo contents cache with `--repo_contents_cache=`, or \
+                specify `--repo_contents_cache=<path outside the workspace>`.
+                """
+                    .formatted(repoContentsCachePath, env.getWorkspace()),
+                Code.BAD_REPO_CONTENTS_CACHE));
       }
 
       try {
@@ -630,6 +636,18 @@ private String getAbsolutePath(String path, CommandEnvironment env) {
     return path;
   }
 
+  /**
+   * An empty path fragment is turned into {@code null}; otherwise, it's treated as relative to the
+   * workspace root.
+   */
+  @Nullable
+  private Path toPath(PathFragment path, CommandEnvironment env) {
+    if (path.isEmpty()) {
+      return null;
+    }
+    return env.getBlazeWorkspace().getWorkspace().getRelative(path);
+  }
+
   @Override
   public ImmutableList<Injected> getPrecomputedValues() {
     Instant now = clock.now();

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

@@ -53,6 +53,7 @@ java_library(
     name = "vendor",
     srcs = ["VendorManager.java"],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/profiler",

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/VendorManager.java

@@ -18,6 +18,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.hash.HashCode;
 import com.google.common.hash.Hasher;
+import com.google.devtools.build.lib.bazel.repository.cache.RepoContentsCache;
 import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.profiler.Profiler;
@@ -75,30 +76,56 @@ public void vendorRepos(Path externalRepoRoot, ImmutableList<RepositoryName> rep
           continue;
         }
 
-        // At this point, the repo should exist under external dir, but check if the vendor src is
-        // already up-to-date.
         Path markerUnderExternal = externalRepoRoot.getChild(repo.getMarkerFileName());
         Path markerUnderVendor = vendorDirectory.getChild(repo.getMarkerFileName());
-        if (isRepoUpToDate(markerUnderVendor, markerUnderExternal)) {
+        // If the marker file doesn't exist under outputBase/external, then the repo is either local
+        // (which cannot be in this case since local repos aren't vendored) or in the repo contents
+        // cache.
+        boolean isCached = !markerUnderExternal.exists();
+        Path actualMarkerFile;
+        if (isCached) {
+          Path cacheRepoDir = repoUnderExternal.resolveSymbolicLinks();
+          actualMarkerFile =
+              cacheRepoDir.replaceName(
+                  cacheRepoDir.getBaseName() + RepoContentsCache.RECORDED_INPUTS_SUFFIX);
+        } else {
+          actualMarkerFile = markerUnderExternal;
+        }
+
+        // At this point, the repo should exist under external dir, but check if the vendor src is
+        // already up-to-date.
+        if (isRepoUpToDate(markerUnderVendor, actualMarkerFile)) {
           continue;
         }
 
-        // Actually vendor the repo:
+        // Actually vendor the repo. If the repo is cached, copy it; otherwise move it.
         // 1. Clean up existing marker file and vendor dir.
         markerUnderVendor.delete();
         repoUnderVendor.deleteTree();
         repoUnderVendor.createDirectory();
-        // 2. Move the marker file to a temporary one under vendor dir.
-        Path tMarker = vendorDirectory.getChild(repo.getMarkerFileName() + ".tmp");
-        FileSystemUtils.moveFile(markerUnderExternal, tMarker);
-        // 3. Move the external repo to vendor dir. It's fine if this step fails or is interrupted,
-        // because the marker file under external is gone anyway.
-        FileSystemUtils.moveTreesBelow(repoUnderExternal, repoUnderVendor);
+        // 2. Copy/move the marker file to a temporary one under vendor dir.
+        Path temporaryMarker = vendorDirectory.getChild(repo.getMarkerFileName() + ".tmp");
+        if (isCached) {
+          FileSystemUtils.copyFile(actualMarkerFile, temporaryMarker);
+        } else {
+          FileSystemUtils.moveFile(actualMarkerFile, temporaryMarker);
+        }
+        // 3. Move/copy the external repo to vendor dir. Note that, in the "move" case, it's fine if
+        // this step fails or is interrupted, because the marker file under external is gone anyway.
+        if (isCached) {
+          FileSystemUtils.copyTreesBelow(repoUnderExternal.resolveSymbolicLinks(), repoUnderVendor);
+        } else {
+          try {
+            repoUnderExternal.renameTo(repoUnderVendor);
+          } catch (IOException e) {
+            FileSystemUtils.moveTreesBelow(repoUnderExternal, repoUnderVendor);
+          }
+        }
         // 4. Re-plant symlinks pointing a path under the external root to a relative path
         // to make sure the vendor src keep working after being moved or output base changed
         replantSymlinks(repoUnderVendor, externalRepoRoot);
-        // 5. Rename the temporary marker file after the move is done.
-        tMarker.renameTo(markerUnderVendor);
+        // 5. Rename the temporary marker file after the move/copy is done.
+        temporaryMarker.renameTo(markerUnderVendor);
         // 6. Leave a symlink in external dir to keep things working.
         repoUnderExternal.deleteTree();
         FileSystemUtils.ensureSymbolicLink(repoUnderExternal, repoUnderVendor);

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

@@ -96,7 +96,7 @@ public Object getResolvedInformation(XattrProvider xattrProvider) {
             });
 
     // Return the needed info.
-    return new FetchResult(ImmutableMap.of());
+    return new FetchResult(ImmutableMap.of(), Reproducibility.YES);
   }
 
   private static String workspaceFileContent(String repositoryName) {

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

@@ -43,8 +43,26 @@ public class RepositoryOptions extends OptionsBase {
           "Specifies the cache location of the downloaded values obtained "
               + "during the fetching of external repositories. An empty string "
               + "as argument requests the cache to be disabled, "
-              + "otherwise the default of '<output_user_root>/cache/repos/v1' is used")
-  public PathFragment experimentalRepositoryCache;
+              + "otherwise the default of '<--output_user_root>/cache/repos/v1' is used")
+  public PathFragment repositoryCache;
+
+  @Option(
+      name = "repo_contents_cache",
+      oldName = "repository_contents_cache",
+      oldNameWarning = false,
+      defaultValue = "null",
+      documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
+      effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
+      converter = OptionsUtils.PathFragmentConverter.class,
+      help =
+          """
+          Specifies the location of the repo contents cache, which contains fetched repo \
+          directories shareable across workspaces. An empty string as argument requests the repo \
+          contents cache to be disabled, otherwise the default of '<--repository_cache>/contents' \
+          is used. Note that this means setting '--repository_cache=' would by default disable the \
+          repo contents cache as well, unless '--repo_contents_cache=<some_path>' is also set.
+          """)
+  public PathFragment repoContentsCache;
 
   @Option(
       name = "registry",

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

@@ -67,7 +67,8 @@ public class RepositoryResolvedEvent implements ResolvedEvent {
   private final boolean informationReturned;
   private final String message;
 
-  public RepositoryResolvedEvent(Rule rule, StructImpl attrs, Path outputDirectory, Object result) {
+  public RepositoryResolvedEvent(
+      Rule rule, StructImpl attrs, Path outputDirectory, Map<?, ?> result) {
     this.outputDirectory = outputDirectory;
 
     String originalClass =
@@ -102,17 +103,17 @@ public RepositoryResolvedEvent(Rule rule, StructImpl attrs, Path outputDirectory
 
     repositoryBuilder.put(ResolvedFileValue.RULE_CLASS, originalClass);
 
-    if (result == Starlark.NONE) {
+    if (result.isEmpty()) {
       // Rule claims to be already reproducible, so wants to be called as is.
       repositoryBuilder.put(ResolvedFileValue.ATTRIBUTES, origAttr);
       this.informationReturned = false;
       this.message = "Repository rule '" + rule.getName() + "' finished.";
-    } else if (result instanceof Map) {
+    } else {
       // Rule claims that the returned (probably changed) arguments are a reproducible
       // version of itself.
       repositoryBuilder.put(ResolvedFileValue.ATTRIBUTES, result);
       Pair<Map<String, Object>, List<String>> diff =
-          compare(origAttr, defaults.buildOrThrow(), (Map<?, ?>) result);
+          compare(origAttr, defaults.buildOrThrow(), result);
       if (diff.getFirst().isEmpty() && diff.getSecond().isEmpty()) {
         this.informationReturned = false;
         this.message = "Repository rule '" + rule.getName() + "' finished.";
@@ -143,13 +144,6 @@ public RepositoryResolvedEvent(Rule rule, StructImpl attrs, Path outputDirectory
                   + Starlark.repr(diff.getSecond());
         }
       }
-    } else {
-      // TODO(aehlig): handle strings specially to allow encodings of the former
-      // values to be accepted as well.
-      resolvedInformationBuilder.put(ResolvedFileValue.REPOSITORIES, result);
-      repositoryBuilder = null; // We already added the REPOSITORIES entry
-      this.informationReturned = true;
-      this.message = "Repository rule '" + rule.getName() + "' returned: " + result;
     }
 
     this.name = rule.getName();

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

@@ -21,6 +21,7 @@ java_library(
         "RepositoryCache.java",
     ],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib/util:file_system_lock",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs/bazel",
         "//third_party:guava",

src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepoContentsCache.java

@@ -14,29 +14,118 @@
 
 package com.google.devtools.build.lib.bazel.repository.cache;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.util.FileSystemLock;
+import com.google.devtools.build.lib.util.FileSystemLock.LockMode;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Comparator;
 import javax.annotation.Nullable;
 
 /** A cache directory that stores the contents of fetched repos across different workspaces. */
 public class RepoContentsCache {
+  public static final String RECORDED_INPUTS_SUFFIX = ".recorded_inputs";
+
   @Nullable private Path path;
 
+  // TODO: wyv@ - implement garbage collection
+
   public void setPath(@Nullable Path path) {
     this.path = path;
   }
 
+  @Nullable
+  public Path getPath() {
+    return path;
+  }
+
   public boolean isEnabled() {
     return path != null;
   }
 
-  public Path recordedInputsFile(String predeclaredInputHash) {
+  /** A candidate repo in the repo contents cache for one predeclared input hash. */
+  public record CandidateRepo(Path recordedInputsFile, Path contentsDir) {
+    private static CandidateRepo fromRecordedInputsFile(Path recordedInputsFile) {
+      String recordedInputsFileBaseName = recordedInputsFile.getBaseName();
+      String contentsDirBaseName =
+          recordedInputsFileBaseName.substring(
+              0, recordedInputsFileBaseName.length() - RECORDED_INPUTS_SUFFIX.length());
+      return new CandidateRepo(
+          recordedInputsFile, recordedInputsFile.replaceName(contentsDirBaseName));
+    }
+  }
+
+  /** Returns the list of candidate repos for the given predeclared input hash. */
+  public ImmutableList<CandidateRepo> getCandidateRepos(String predeclaredInputHash) {
     Preconditions.checkState(path != null);
-    return path.getRelative(predeclaredInputHash + ".recorded_inputs");
+    Path entryDir = path.getRelative(predeclaredInputHash);
+    try {
+      return entryDir.getDirectoryEntries().stream()
+          .filter(path -> path.getBaseName().endsWith(RECORDED_INPUTS_SUFFIX))
+          // We're just sorting for consistency here. (Note that "10.recorded_inputs" would sort
+          // before "1.recorded_inputs".) If necessary, we could use some sort of heuristics here
+          // to speed up the subsequent up-to-date-ness checking.
+          .sorted(Comparator.comparing(Path::getBaseName))
+          .map(CandidateRepo::fromRecordedInputsFile)
+          .collect(toImmutableList());
+    } catch (IOException e) {
+      // This should only happen if `entryDir` doesn't exist yet or is not a directory. Either way,
+      // don't outright fail; just treat it as if the cache is empty.
+      return ImmutableList.of();
+    }
   }
 
-  public Path contentsDir(String predeclaredInputHash) {
+  /** Moves a freshly fetched repo into the contents cache. */
+  public void moveToCache(
+      Path fetchedRepoDir, Path fetchedRepoMarkerFile, String predeclaredInputHash)
+      throws IOException {
     Preconditions.checkState(path != null);
-    return path.getRelative(predeclaredInputHash);
+
+    Path entryDir = path.getRelative(predeclaredInputHash);
+    if (!entryDir.isDirectory()) {
+      entryDir.delete();
+    }
+    String counter = getNextCounterInDir(entryDir);
+    Path cacheRecordedInputsFile = entryDir.getChild(counter + RECORDED_INPUTS_SUFFIX);
+    Path cacheRepoDir = entryDir.getChild(counter);
+
+    cacheRepoDir.deleteTree();
+    cacheRepoDir.createDirectoryAndParents();
+    // Move the fetched marker file to a temp location, so that if following operations fail, both
+    // the fetched repo and the cache locations are considered out-of-date.
+    Path temporaryMarker = entryDir.getChild(counter + ".temp_recorded_inputs");
+    FileSystemUtils.moveFile(fetchedRepoMarkerFile, temporaryMarker);
+    // Now perform the move, and afterwards, restore the marker file.
+    try {
+      fetchedRepoDir.renameTo(cacheRepoDir);
+    } catch (IOException e) {
+      FileSystemUtils.moveTreesBelow(fetchedRepoDir, cacheRepoDir);
+    }
+    temporaryMarker.renameTo(cacheRecordedInputsFile);
+    // Set up a symlink at the original fetched repo dir path.
+    fetchedRepoDir.deleteTree();
+    FileSystemUtils.ensureSymbolicLink(fetchedRepoDir, cacheRepoDir);
+  }
+
+  private static String getNextCounterInDir(Path entryDir) throws IOException {
+    Path counterFile = entryDir.getRelative("counter");
+    try (var lock = FileSystemLock.get(entryDir.getRelative("lock"), LockMode.EXCLUSIVE)) {
+      int c = 0;
+      if (counterFile.exists()) {
+        try {
+          c = Integer.parseInt(FileSystemUtils.readContent(counterFile, StandardCharsets.UTF_8));
+        } catch (NumberFormatException e) {
+          // ignored
+        }
+      }
+      String counter = Integer.toString(c + 1);
+      FileSystemUtils.writeContent(counterFile, StandardCharsets.UTF_8, counter);
+      return counter;
+    }
   }
 }