[7.2.0] Store remote registry file hashes in the lockfile (#22296)

`MODULE.bazel`, `bazel_registry.json` and `source.json` files obtained
from remote registries are stored in the repository cache and their
hashes are collected in the lockfile. This speeds up incremental module
resolutions, such as after adding a new `bazel_dep`.

Yanked versions are not stored in the lockfile. Their handling will be
part of a follow-up PR.

Implements part of
https://docs.google.com/document/d/1TjA7-M5njkI1F38IC0pm305S9EOmxcUwaCIvaSmansg/edit
Work towards #20369

Closes #21901.

PiperOrigin-RevId: 631195852
Change-Id: I35c30af7f9c3626bdbcb04c85b8c2502eeaafd3e

Co-authored-by: Fabian Meumertzheim <[email protected]>

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

@@ -15,12 +15,16 @@
 
 package com.google.devtools.build.lib.bazel;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.CharMatcher;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
@@ -131,8 +135,8 @@ public class BazelRepositoryModule extends BlazeModule {
   public static final String DEFAULT_CACHE_LOCATION = "cache/repos/v1";
 
   // Default list of registries.
-  public static final ImmutableList<String> DEFAULT_REGISTRIES =
-      ImmutableList.of("https://bcr.bazel.build/");
+  public static final ImmutableSet<String> DEFAULT_REGISTRIES =
+      ImmutableSet.of("https://bcr.bazel.build/");
 
   // A map of repository handlers that can be looked up by rule class name.
   private final ImmutableMap<String, RepositoryFunction> repositoryHandlers;
@@ -150,7 +154,7 @@ public class BazelRepositoryModule extends BlazeModule {
   private ImmutableMap<String, ModuleOverride> moduleOverrides = ImmutableMap.of();
   private Optional<RootedPath> resolvedFileReplacingWorkspace = Optional.empty();
   private FileSystem filesystem;
-  private List<String> registries;
+  private ImmutableSet<String> registries;
   private final AtomicBoolean ignoreDevDeps = new AtomicBoolean(false);
   private CheckDirectDepsMode checkDirectDepsMode = CheckDirectDepsMode.WARNING;
   private BazelCompatibilityMode bazelCompatibilityMode = BazelCompatibilityMode.ERROR;
@@ -495,7 +499,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
       }
 
       if (repoOptions.registries != null && !repoOptions.registries.isEmpty()) {
-        registries = repoOptions.registries;
+        registries = normalizeRegistries(repoOptions.registries);
       } else {
         registries = DEFAULT_REGISTRIES;
       }
@@ -525,6 +529,14 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
     }
   }
 
+  private static ImmutableSet<String> normalizeRegistries(List<String> registries) {
+    // Ensure that registries aren't duplicated even after `/modules/...` paths are appended to
+    // them.
+    return registries.stream()
+        .map(url -> CharMatcher.is('/').trimTrailingFrom(url))
+        .collect(toImmutableSet());
+  }
+
   /**
    * If the given path is absolute path, leave it as it is. If the given path is a relative path, it
    * is relative to the current working directory. If the given path starts with '%workspace%, it is

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

@@ -26,8 +26,6 @@ java_library(
     ],
     deps = [
         "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
-        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/main/java/net/starlark/java/eval",
         "//third_party:auto_value",
         "//third_party:gson",
@@ -77,11 +75,12 @@ java_library(
         "Registry.java",
         "RegistryFactory.java",
         "RegistryFactoryImpl.java",
+        "RegistryFileDownloadEvent.java",
     ],
     deps = [
         ":common",
+        "//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/events",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/util:os",
@@ -97,17 +96,15 @@ java_library(
     name = "bazel_lockfile_module",
     srcs = ["BazelLockFileModule.java"],
     deps = [
-        ":exception",
         ":resolution",
         ":resolution_impl",
         "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
+        "//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/skyframe:skyframe_cluster",
-        "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
         "//src/main/java/com/google/devtools/build/lib/vfs",
-        "//src/main/java/com/google/devtools/build/skyframe",
         "//third_party:flogger",
         "//third_party:guava",
         "//third_party:jsr305",
@@ -144,6 +141,7 @@ java_library(
         "RegistryKey.java",
         "RegistryOverride.java",
         "RepoSpecKey.java",
+        "RepoSpecValue.java",
         "SingleExtensionUsagesValue.java",
         "SingleExtensionValue.java",
         "SingleVersionOverride.java",
@@ -160,6 +158,7 @@ java_library(
         ":root_module_file_fixup",
         "//src/main/java/com/google/devtools/build/docgen/annot",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
+        "//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/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
@@ -226,6 +225,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/bazel:bazel_version",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
+        "//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/bazel/repository/starlark",
         "//src/main/java/com/google/devtools/build/lib/cmdline",

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

@@ -19,9 +19,9 @@
 import static com.google.common.collect.ImmutableBiMap.toImmutableBiMap;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static java.util.stream.Collectors.counting;
 import static java.util.stream.Collectors.groupingBy;
-import static java.util.stream.Collectors.toSet;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
@@ -30,6 +30,7 @@
 import com.google.common.collect.ImmutableBiMap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableTable;
 import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
 import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
@@ -49,7 +50,6 @@
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
@@ -186,7 +186,6 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
       return null;
     }
 
-    ImmutableList<String> registries = ImmutableList.copyOf(ModuleFileFunction.REGISTRIES.get(env));
     ImmutableMap<String, String> moduleOverrides =
         ModuleFileFunction.MODULE_OVERRIDES.get(env).entrySet().stream()
             .collect(
@@ -202,7 +201,7 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
     String envYanked = allowedYankedVersionsFromEnv.getValue();
 
     return BzlmodFlagsAndEnvVars.create(
-        registries,
+        ModuleFileFunction.REGISTRIES.get(env),
         moduleOverrides,
         yankedVersions,
         nullToEmpty(envYanked),
@@ -257,14 +256,14 @@ private static ImmutableBiMap<RepositoryName, ModuleKey> computeCanonicalRepoNam
       ImmutableMap<ModuleKey, Module> depGraph) {
     // Find modules with multiple versions in the dep graph. Currently, the only source of such
     // modules is multiple_version_override.
-    Set<String> multipleVersionsModules =
+    ImmutableSet<String> multipleVersionsModules =
         depGraph.keySet().stream()
             .collect(groupingBy(ModuleKey::getName, counting()))
             .entrySet()
             .stream()
             .filter(entry -> entry.getValue() > 1)
             .map(Entry::getKey)
-            .collect(toSet());
+            .collect(toImmutableSet());
 
     // If there is a unique version of this module in the entire dep graph, we elide the version
     // from the canonical repository name. This has a number of benefits:

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

@@ -19,6 +19,7 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.FileValue;
 import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
@@ -55,7 +56,7 @@ public class BazelLockFileFunction implements SkyFunction {
 
   private static final BzlmodFlagsAndEnvVars EMPTY_FLAGS =
       BzlmodFlagsAndEnvVars.create(
-          ImmutableList.of(), ImmutableMap.of(), ImmutableList.of(), "", false, "", "");
+          ImmutableSet.of(), ImmutableMap.of(), ImmutableList.of(), "", false, "", "");
 
   private static final BazelLockFileValue EMPTY_LOCKFILE =
       BazelLockFileValue.builder()
@@ -65,6 +66,7 @@ public class BazelLockFileFunction implements SkyFunction {
           .setLocalOverrideHashes(ImmutableMap.of())
           .setModuleDepGraph(ImmutableMap.of())
           .setModuleExtensions(ImmutableMap.of())
+          .setRegistryFileHashes(ImmutableMap.of())
           .build();
 
   public BazelLockFileFunction(Path rootDirectory) {

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

@@ -22,18 +22,19 @@
 import com.google.common.flogger.GoogleLogger;
 import com.google.devtools.build.lib.bazel.repository.RepositoryOptions;
 import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
+import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
-import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.Nullable;
 
@@ -45,6 +46,7 @@ public class BazelLockFileModule extends BlazeModule {
 
   private SkyframeExecutor executor;
   private Path workspaceRoot;
+  private boolean enabled;
   @Nullable private BazelModuleResolutionEvent moduleResolutionEvent;
 
   private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
@@ -53,20 +55,44 @@ public class BazelLockFileModule extends BlazeModule {
   public void beforeCommand(CommandEnvironment env) {
     executor = env.getSkyframeExecutor();
     workspaceRoot = env.getWorkspace();
-    RepositoryOptions options = env.getOptions().getOptions(RepositoryOptions.class);
-    if (options.lockfileMode.equals(LockfileMode.UPDATE)) {
-      env.getEventBus().register(this);
-    }
+
+    enabled =
+        env.getOptions().getOptions(RepositoryOptions.class).lockfileMode == LockfileMode.UPDATE;
+    moduleResolutionEvent = null;
+    env.getEventBus().register(this);
   }
 
   @Override
-  public void afterCommand() throws AbruptExitException {
-    if (moduleResolutionEvent == null) {
+  public void afterCommand() {
+    if (!enabled || moduleResolutionEvent == null) {
       // Command does not use Bazel modules or the lockfile mode is not update.
       // Since Skyframe caches events, they are replayed even when nothing has changed.
       return;
     }
 
+    BazelDepGraphValue depGraphValue;
+    BazelModuleResolutionValue moduleResolutionValue;
+    try {
+      depGraphValue =
+          (BazelDepGraphValue) executor.getEvaluator().getExistingValue(BazelDepGraphValue.KEY);
+      moduleResolutionValue =
+          (BazelModuleResolutionValue)
+              executor.getEvaluator().getExistingValue(BazelModuleResolutionValue.KEY);
+    } catch (InterruptedException e) {
+      // Not thrown in Bazel.
+      throw new IllegalStateException(e);
+    }
+
+    BazelLockFileValue oldLockfile = moduleResolutionEvent.getOnDiskLockfileValue();
+    ImmutableMap<String, Optional<Checksum>> fileHashes;
+    if (moduleResolutionValue == null) {
+      // BazelDepGraphFunction took the dep graph from the lockfile and didn't cause evaluation of
+      // BazelModuleResolutionFunction. The file hashes in the lockfile are still up-to-date.
+      fileHashes = oldLockfile.getRegistryFileHashes();
+    } else {
+      fileHashes = ImmutableSortedMap.copyOf(moduleResolutionValue.getRegistryFileHashes());
+    }
+
     // All nodes corresponding to module extensions that have been evaluated in the current build
     // are done at this point. Look up entries by eval keys to record results even if validation
     // later fails due to invalid imports.
@@ -88,24 +114,16 @@ public void afterCommand() throws AbruptExitException {
                 newExtensionInfos.put(key.argument(), value.getLockFileInfo().get());
               }
             });
+    var combinedExtensionInfos =
+        combineModuleExtensions(
+            oldLockfile.getModuleExtensions(), newExtensionInfos, depGraphValue);
 
-    BazelDepGraphValue depGraphValue;
-    try {
-      depGraphValue =
-          (BazelDepGraphValue) executor.getEvaluator().getExistingValue(BazelDepGraphValue.KEY);
-    } catch (InterruptedException e) {
-      // Not thrown in Bazel.
-      throw new IllegalStateException(e);
-    }
-
-    BazelLockFileValue oldLockfile = moduleResolutionEvent.getOnDiskLockfileValue();
     // Create an updated version of the lockfile, keeping only the extension results from the old
     // lockfile that are still up-to-date and adding the newly resolved extension results.
     BazelLockFileValue newLockfile =
         moduleResolutionEvent.getResolutionOnlyLockfileValue().toBuilder()
-            .setModuleExtensions(
-                combineModuleExtensions(
-                    oldLockfile.getModuleExtensions(), newExtensionInfos, depGraphValue))
+            .setRegistryFileHashes(fileHashes)
+            .setModuleExtensions(combinedExtensionInfos)
             .build();
 
     // Write the new value to the file, but only if needed. This is not just a performance
@@ -115,7 +133,6 @@ public void afterCommand() throws AbruptExitException {
     if (!newLockfile.equals(oldLockfile)) {
       updateLockfile(workspaceRoot, newLockfile);
     }
-    this.moduleResolutionEvent = null;
   }
 
   /**
@@ -140,8 +157,6 @@ public void afterCommand() throws AbruptExitException {
       var factorToLockedExtension = entry.getValue();
       ModuleExtensionEvalFactors firstEntryFactors =
           factorToLockedExtension.keySet().iterator().next();
-      LockFileModuleExtension firstEntryExtension =
-          factorToLockedExtension.values().iterator().next();
       // All entries for a single extension share the same usages digest, so it suffices to check
       // the first entry.
       if (shouldKeepExtension(

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

@@ -20,13 +20,15 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
 import java.util.Map;
+import java.util.Optional;
 
 /**
  * The result of reading the lockfile. Contains the lockfile version, module hash, definitions of
@@ -44,7 +46,8 @@ public abstract class BazelLockFileValue implements SkyValue, Postable {
   static Builder builder() {
     return new AutoValue_BazelLockFileValue.Builder()
         .setLockFileVersion(LOCK_FILE_VERSION)
-        .setModuleExtensions(ImmutableMap.of());
+        .setModuleExtensions(ImmutableMap.of())
+        .setRegistryFileHashes(ImmutableMap.of());
   }
 
   /** Current version of the lock file */
@@ -62,6 +65,9 @@ static Builder builder() {
   /** The post-selection dep graph retrieved from the lock file. */
   public abstract ImmutableMap<ModuleKey, Module> getModuleDepGraph();
 
+  /** Hashes of files retrieved from registries. */
+  public abstract ImmutableMap<String, Optional<Checksum>> getRegistryFileHashes();
+
   /** Mapping the extension id to the module extension data */
   public abstract ImmutableMap<
           ModuleExtensionId, ImmutableMap<ModuleExtensionEvalFactors, LockFileModuleExtension>>
@@ -82,6 +88,8 @@ public abstract static class Builder {
 
     public abstract Builder setModuleDepGraph(ImmutableMap<ModuleKey, Module> value);
 
+    public abstract Builder setRegistryFileHashes(ImmutableMap<String, Optional<Checksum>> value);
+
     public abstract Builder setModuleExtensions(
         ImmutableMap<
                 ModuleExtensionId,