Simplify RepositoryDirectoryValue and rename RepositoryCache

As the title suggests, this PR does two things:

1. It refactors `RepositoryDirectoryValue`.
   - This class used to have a `repositoryExists()` boolean method, and various other methods that throw at runtime depending on whether `repositoryExists()` is true or false. This PR changes it so that `RepositoryDirectoryValue` is a sealed interface with two impls -- `Success` and `Failure`.
   - The "success" case used to hold on to a digest. This digest is effectively a hash of the marker file, and it's not actually used anywhere. However, removing it causes tests to mysteriously start failing. Upon closer inspection, this is actually because it affects change pruning -- the marking file changing is almost 100% the same as the fetched contents changing. I can't think of a counterexample, but this seems slightly wrong, so I changed `RepositoryDirectoryValue` to never use change pruning instead (by implementing `NotComparableSkyValue`).
      - Note that this is potentially a subtle behavior change, but should improve correctness, if anything.
2. It renames `RepositoryCache` to `DownloadCache`. This is admittedly a mostly unrelated change... If any reviewer requests it, I'm happy to spend the time splitting this part into a separate PR.
   - It also adds a new `RepositoryCache` that holds onto the `DownloadCache` and the soon-to-be-introduced `RepoContentsCache`. The latter is just a "readonly" trivial implementation for now.

Work towards #12227.

Closes #25919.

PiperOrigin-RevId: 750740171
Change-Id: Id87180b75b4a856fa0734952105c178b7d41b74e

Author: Wyverald

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

@@ -214,7 +214,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 +245,8 @@ public void workspaceInit(
             isFetch,
             clientEnvironmentSupplier,
             directories,
-            BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER);
+            BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER,
+            repositoryCache.getRepoContentsCache());
     singleExtensionEvalFunction =
         new SingleExtensionEvalFunction(directories, clientEnvironmentSupplier);
 
@@ -311,7 +312,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 +343,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);
@@ -365,7 +369,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
                   .getWorkspace()
                   .getRelative(repoOptions.experimentalRepositoryCache);
         }
-        repositoryCache.setRepositoryCachePath(repositoryCachePath);
+        repositoryCache.setPath(repositoryCachePath);
       } else {
         Path repositoryCachePath =
             env.getDirectories()
@@ -374,7 +378,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
                 .getRelative(DEFAULT_CACHE_LOCATION);
         try {
           repositoryCachePath.createDirectoryAndParents();
-          repositoryCache.setRepositoryCachePath(repositoryCachePath);
+          repositoryCache.setPath(repositoryCachePath);
         } catch (IOException e) {
           env.getReporter()
               .handle(

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

src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java

@@ -54,6 +54,7 @@
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingResult;
 import java.util.List;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
 /** Fetches external repositories. Which is so fetch. */
@@ -216,8 +217,11 @@ private BlazeCommandResult fetchRepos(
 
     String notFoundRepos =
         repositoryNamesAndValues.values().stream()
-            .filter(value -> !value.repositoryExists())
-            .map(value -> value.getErrorMsg())
+            .flatMap(
+                value ->
+                    value instanceof RepositoryDirectoryValue.Failure failure
+                        ? Stream.of(failure.getErrorMsg())
+                        : Stream.of())
             .collect(joining("; "));
     if (!notFoundRepos.isEmpty()) {
       return createFailedBlazeCommandResult(

src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java

@@ -39,6 +39,8 @@
 import com.google.devtools.build.lib.pkgcache.PackageOptions;
 import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
+import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue.Failure;
+import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue.Success;
 import com.google.devtools.build.lib.runtime.BlazeCommand;
 import com.google.devtools.build.lib.runtime.BlazeCommandResult;
 import com.google.devtools.build.lib.runtime.Command;
@@ -257,12 +259,13 @@ private BlazeCommandResult vendorRepos(
     List<String> notFoundRepoErrors = new ArrayList<>();
     for (Entry<RepositoryName, RepositoryDirectoryValue> entry :
         repositoryNamesAndValues.entrySet()) {
-      if (entry.getValue().repositoryExists()) {
-        if (!entry.getValue().excludeFromVendoring()) {
-          reposToVendor.add(entry.getKey());
+      switch (entry.getValue()) {
+        case Success s -> {
+          if (!s.excludeFromVendoring()) {
+            reposToVendor.add(entry.getKey());
+          }
         }
-      } else {
-        notFoundRepoErrors.add(entry.getValue().getErrorMsg());
+        case Failure f -> notFoundRepoErrors.add(f.getErrorMsg());
       }
     }
 
@@ -314,8 +317,7 @@ private ImmutableSet<RepositoryName> collectReposFromTargets(
       SkyKey key = nodes.remove();
       visited.add(key);
       NodeEntry nodeEntry = inMemoryGraph.get(null, Reason.VENDOR_EXTERNAL_REPOS, key);
-      if (nodeEntry.getValue() instanceof RepositoryDirectoryValue repoDirValue
-          && repoDirValue.repositoryExists()
+      if (nodeEntry.getValue() instanceof RepositoryDirectoryValue.Success repoDirValue
           && !repoDirValue.excludeFromVendoring()) {
         repos.add((RepositoryName) key.argument());
       }

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

@@ -14,7 +14,7 @@
 package com.google.devtools.build.lib.bazel.repository;
 
 import com.google.common.eventbus.Subscribe;
-import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCacheHitEvent;
+import com.google.devtools.build.lib.bazel.repository.cache.DownloadCacheHitEvent;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.repository.RepositoryFailedEvent;
@@ -47,7 +47,7 @@ public void afterCommand() {
   }
 
   @Subscribe
-  public synchronized void cacheHit(RepositoryCacheHitEvent event) {
+  public synchronized void cacheHit(DownloadCacheHitEvent event) {
     cacheHitsByContext
         .computeIfAbsent(event.getContext(), k -> new HashSet<>())
         .add(Pair.of(event.getFileHash(), event.getUrl()));

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

@@ -20,7 +20,6 @@
 import com.google.devtools.build.lib.bazel.ResolvedEvent;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
-import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
 import com.google.devtools.build.lib.rules.repository.ResolvedFileValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
@@ -97,8 +96,7 @@ public Object getResolvedInformation(XattrProvider xattrProvider) {
             });
 
     // Return the needed info.
-    return new FetchResult(
-        RepositoryDirectoryValue.builder().setPath(outputDirectory), ImmutableMap.of());
+    return new FetchResult(ImmutableMap.of());
   }
 
   private static String workspaceFileContent(String repositoryName) {

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

@@ -15,7 +15,11 @@ filegroup(
 #   Main Java code for Bazel
 java_library(
     name = "cache",
-    srcs = ["RepositoryCache.java"],
+    srcs = [
+        "DownloadCache.java",
+        "RepoContentsCache.java",
+        "RepositoryCache.java",
+    ],
     deps = [
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs/bazel",
@@ -26,9 +30,6 @@ java_library(
 
 java_library(
     name = "events",
-    srcs = ["RepositoryCacheHitEvent.java"],
-    deps = [
-        "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/main/java/com/google/devtools/build/lib/events",
-    ],
+    srcs = ["DownloadCacheHitEvent.java"],
+    deps = ["//src/main/java/com/google/devtools/build/lib/events"],
 )