[8.3.0] Repo contents cache

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/BUILD

@@ -47,6 +47,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/pkgcache",
+        "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/new_local_repository_function",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/new_local_repository_rule",

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

@@ -81,6 +81,9 @@
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.pkgcache.PackageOptions;
+import com.google.devtools.build.lib.profiler.Profiler;
+import com.google.devtools.build.lib.profiler.ProfilerTask;
+import com.google.devtools.build.lib.profiler.SilentCloseable;
 import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
 import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
 import com.google.devtools.build.lib.rules.repository.NewLocalRepositoryFunction;
@@ -214,7 +217,7 @@ private static class RepositoryCacheInfoItem extends InfoItem {
     public byte[] get(
         Supplier<BuildConfigurationValue> configurationSupplier, CommandEnvironment env)
         throws AbruptExitException, InterruptedException {
-      return print(repositoryCache.getRootPath());
+      return print(repositoryCache.getPath());
     }
   }
 
@@ -245,7 +248,8 @@ public void workspaceInit(
             isFetch,
             clientEnvironmentSupplier,
             directories,
-            BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER);
+            BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER,
+            repositoryCache.getRepoContentsCache());
     singleExtensionEvalFunction =
         new SingleExtensionEvalFunction(directories, clientEnvironmentSupplier);
 
@@ -311,7 +315,10 @@ public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) {
   @Override
   public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
     DownloadManager downloadManager =
-        new DownloadManager(repositoryCache, env.getDownloaderDelegate(), env.getHttpDownloader());
+        new DownloadManager(
+            repositoryCache.getDownloadCache(),
+            env.getDownloaderDelegate(),
+            env.getHttpDownloader());
     this.starlarkRepositoryFunction.setDownloadManager(downloadManager);
     this.moduleFileFunction.setDownloadManager(downloadManager);
     this.repoSpecFunction.setDownloadManager(downloadManager);
@@ -339,7 +346,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
       }
       disableNativeRepoRules = repoOptions.disableNativeRepoRules;
 
-      repositoryCache.setHardlink(repoOptions.useHardlinks);
+      repositoryCache.getDownloadCache().setHardlink(repoOptions.useHardlinks);
       if (repoOptions.experimentalScaleTimeouts > 0.0) {
         starlarkRepositoryFunction.setTimeoutScaling(repoOptions.experimentalScaleTimeouts);
         singleExtensionEvalFunction.setTimeoutScaling(repoOptions.experimentalScaleTimeouts);
@@ -352,34 +359,61 @@ 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.setRepositoryCachePath(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.setRepositoryCachePath(repositoryCachePath);
+                .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.
+        repositoryCache.getRepoContentsCache().setPath(null);
+        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));
+      }
+      if (repositoryCache.getRepoContentsCache().isEnabled()) {
+        try (SilentCloseable c =
+            Profiler.instance()
+                .profile(ProfilerTask.REPO_CACHE_GC_WAIT, "waiting to acquire repo cache lock")) {
+          repositoryCache.getRepoContentsCache().acquireSharedLock();
         } catch (IOException e) {
-          env.getReporter()
-              .handle(
-                  Event.warn(
-                      "Failed to set up cache at " + repositoryCachePath + ": " + e.getMessage()));
+          throw new AbruptExitException(
+              detailedExitCode(
+                  "could not acquire lock on repo contents cache", Code.BAD_REPO_CONTENTS_CACHE),
+              e);
+        }
+        if (!repoOptions.repoContentsCacheGcMaxAge.isZero()) {
+          env.addIdleTask(
+              repositoryCache
+                  .getRepoContentsCache()
+                  .createGcIdleTask(
+                      repoOptions.repoContentsCacheGcMaxAge,
+                      repoOptions.repoContentsCacheGcIdleDelay));
         }
       }
 
@@ -626,6 +660,32 @@ 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() || env.getBlazeWorkspace().getWorkspace() == null) {
+      return null;
+    }
+    return env.getBlazeWorkspace().getWorkspace().getRelative(path);
+  }
+
+  @Override
+  public void afterCommand() throws AbruptExitException {
+    if (repositoryCache.getRepoContentsCache().isEnabled()) {
+      try {
+        repositoryCache.getRepoContentsCache().releaseSharedLock();
+      } catch (IOException e) {
+        throw new AbruptExitException(
+            detailedExitCode(
+                "could not release lock on repo contents cache", Code.BAD_REPO_CONTENTS_CACHE),
+            e);
+      }
+    }
+  }
+
   @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/BazelFetchAllFunction.java

@@ -98,7 +98,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
       if (repoDirValue == null) {
         return null;
       }
-      if (!repoDirValue.excludeFromVendoring()) {
+      if (repoDirValue instanceof RepositoryDirectoryValue.Success s && !s.excludeFromVendoring()) {
         shouldVendor.add((RepositoryName) repoDelegatorKey.argument());
       }
     }

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

@@ -25,7 +25,7 @@
 import com.google.common.collect.ImmutableTable;
 import com.google.common.collect.Table;
 import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
-import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
+import com.google.devtools.build.lib.bazel.repository.cache.DownloadCache;
 import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
@@ -445,7 +445,7 @@ public Optional<Checksum> read(JsonReader jsonReader) throws IOException {
           return Optional.empty();
         }
         try {
-          return Optional.of(Checksum.fromString(RepositoryCache.KeyType.SHA256, checksumString));
+          return Optional.of(Checksum.fromString(DownloadCache.KeyType.SHA256, checksumString));
         } catch (Checksum.InvalidChecksumException e) {
           throw new JsonParseException(String.format("Invalid checksum: %s", checksumString), e);
         }

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

@@ -28,8 +28,12 @@
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.actions.FileValue;
 import com.google.devtools.build.lib.bazel.bzlmod.CompiledModuleFile.IncludeStatement;
+import com.google.devtools.build.lib.bazel.bzlmod.ModuleExtensionUsage.Proxy;
+import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileGlobals.ModuleExtensionProxy;
 import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.NonRootModuleFileValue;
 import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
+import com.google.devtools.build.lib.bazel.bzlmod.ModuleThreadContext.ModuleExtensionUsageBuilder;
+import com.google.devtools.build.lib.bazel.bzlmod.Registry.NotFoundException;
 import com.google.devtools.build.lib.bazel.repository.PatchUtil;
 import com.google.devtools.build.lib.bazel.repository.downloader.Checksum.MissingChecksumException;
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
@@ -47,6 +51,7 @@
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.profiler.SilentCloseable;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
+import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue.Success;
 import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
 import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction;
 import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue;
@@ -62,6 +67,7 @@
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunction.Environment.SkyKeyComputeState;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -132,7 +138,7 @@ public ModuleFileFunction(
     this.builtinModules = builtinModules;
   }
 
-  private static class State implements Environment.SkyKeyComputeState {
+  private static class State implements SkyKeyComputeState {
     // The following fields are used during root module file evaluation. We try to compile the root
     // module file itself first, and then read, parse, and compile any included module files layer
     // by layer, in a BFS fashion (hence the `horizon` field). Finally, everything is collected into
@@ -480,7 +486,7 @@ public static RootModuleFileValue evaluateRootModuleFile(
       throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for the root module");
     }
     for (ModuleExtensionUsage usage : module.getExtensionUsages()) {
-      ModuleExtensionUsage.Proxy firstProxy = usage.getProxies().getFirst();
+      Proxy firstProxy = usage.getProxies().getFirst();
       if (usage.getIsolationKey().isPresent() && firstProxy.getImports().isEmpty()) {
         throw errorf(
             Code.BAD_MODULE,
@@ -581,16 +587,16 @@ private static void injectRepos(
       return;
     }
     // Use the innate extension backing use_repo_rule.
-    ModuleThreadContext.ModuleExtensionUsageBuilder usageBuilder =
-        new ModuleThreadContext.ModuleExtensionUsageBuilder(
+    ModuleExtensionUsageBuilder usageBuilder =
+        new ModuleExtensionUsageBuilder(
             context,
             "//:MODULE.bazel",
             "@bazel_tools//tools/build_defs/repo:local.bzl local_repository",
             /* isolate= */ false);
-    ModuleFileGlobals.ModuleExtensionProxy extensionProxy =
-        new ModuleFileGlobals.ModuleExtensionProxy(
+    ModuleExtensionProxy extensionProxy =
+        new ModuleExtensionProxy(
             usageBuilder,
-            ModuleExtensionUsage.Proxy.builder()
+            Proxy.builder()
                 .setDevDependency(true)
                 .setLocation(Location.BUILTIN)
                 .setContainingModuleFilePath(context.getCurrentModuleFilePath()));
@@ -638,9 +644,12 @@ private GetModuleFileResult getModuleFile(
       if (repoDir == null) {
         return null;
       }
+      // This repo _definitely_ exists, since it has a non-registry override, which directly gets
+      // "translated" into a repo spec. So we can cast `repoDir` to `Success`.
+      Path repoDirPath = ((Success) repoDir).getPath();
       RootedPath moduleFilePath =
           RootedPath.toRootedPath(
-              Root.fromPath(repoDir.getPath()), LabelConstants.MODULE_DOT_BAZEL_FILE_NAME);
+              Root.fromPath(repoDirPath), LabelConstants.MODULE_DOT_BAZEL_FILE_NAME);
       if (env.getValue(FileValue.key(moduleFilePath)) == null) {
         return null;
       }
@@ -706,7 +715,7 @@ private GetModuleFileResult getModuleFile(
         try {
           originalModuleFile =
               registry.getModuleFile(key, downloadEventHandler, this.downloadManager);
-        } catch (Registry.NotFoundException e) {
+        } catch (NotFoundException e) {
           if (notFoundTrace == null) {
             notFoundTrace = new ArrayList<>();
           }

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

@@ -17,7 +17,7 @@
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.hash.Hashing;
-import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
+import com.google.devtools.build.lib.bazel.repository.cache.DownloadCache;
 import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
 import java.util.Collection;
@@ -44,7 +44,7 @@ static ImmutableMap<String, Optional<Checksum>> collectToMap(Collection<Postable
   private static Checksum computeHash(byte[] bytes) {
     try {
       return Checksum.fromString(
-          RepositoryCache.KeyType.SHA256, Hashing.sha256().hashBytes(bytes).toString());
+          DownloadCache.KeyType.SHA256, Hashing.sha256().hashBytes(bytes).toString());
     } catch (Checksum.InvalidChecksumException e) {
       // This can't happen since HashCode.toString() always returns a valid hash.
       throw new IllegalStateException(e);