Store remote registry file hashes in the lockfile

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.common.util.concurrent.ThreadFactoryBuilder;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
@@ -134,8 +138,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;
@@ -153,7 +157,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;
@@ -530,7 +534,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;
       }
@@ -560,6 +564,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

@@ -77,9 +77,11 @@ 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",
@@ -98,11 +100,13 @@ java_library(
     srcs = ["BazelLockFileModule.java"],
     deps = [
         ":exception",
+        ":registry",
         ":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",
@@ -144,6 +148,7 @@ java_library(
         "RegistryKey.java",
         "RegistryOverride.java",
         "RepoSpecKey.java",
+        "RepoSpecValue.java",
         "SingleExtensionUsagesValue.java",
         "SingleExtensionValue.java",
         "SingleVersionOverride.java",
@@ -160,6 +165,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 +232,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
         "//src/main/java/com/google/devtools/build/lib/bazel:bazel_version",
         "//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

@@ -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;
@@ -186,7 +187,7 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
       return null;
     }
 
-    ImmutableList<String> registries = ImmutableList.copyOf(ModuleFileFunction.REGISTRIES.get(env));
+    ImmutableSet<String> registries = ImmutableSet.copyOf(ModuleFileFunction.REGISTRIES.get(env));
     ImmutableMap<String, String> moduleOverrides =
         ModuleFileFunction.MODULE_OVERRIDES.get(env).entrySet().stream()
             .collect(

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;
   }
 
   /**

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
@@ -37,14 +39,15 @@
 @GenerateTypeAdapter
 public abstract class BazelLockFileValue implements SkyValue, Postable {
 
-  public static final int LOCK_FILE_VERSION = 7;
+  public static final int LOCK_FILE_VERSION = 8;
 
   @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE;
 
   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,