[7.2.0] Add --lockfile_mode=refresh

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

@@ -41,6 +41,7 @@ java_library(
         "//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/bazel/rules/android",
+        "//src/main/java/com/google/devtools/build/lib/clock",
         "//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/bazel/BazelRepositoryModule.java

@@ -80,6 +80,7 @@
 import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryRule;
 import com.google.devtools.build.lib.bazel.rules.android.AndroidSdkRepositoryFunction;
 import com.google.devtools.build.lib.bazel.rules.android.AndroidSdkRepositoryRule;
+import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.pkgcache.PackageOptions;
@@ -121,6 +122,7 @@
 import com.google.devtools.common.options.OptionsParsingResult;
 import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
+import java.time.Instant;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -159,6 +161,8 @@ public class BazelRepositoryModule extends BlazeModule {
   private CheckDirectDepsMode checkDirectDepsMode = CheckDirectDepsMode.WARNING;
   private BazelCompatibilityMode bazelCompatibilityMode = BazelCompatibilityMode.ERROR;
   private LockfileMode bazelLockfileMode = LockfileMode.UPDATE;
+  private Clock clock;
+  private Instant lastRegistryInvalidation = Instant.EPOCH;
 
   private Optional<Path> vendorDirectory;
   private List<String> allowedYankedVersions = ImmutableList.of();
@@ -526,6 +530,21 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
       starlarkRepositoryFunction.setRepositoryRemoteExecutor(remoteExecutor);
       singleExtensionEvalFunction.setRepositoryRemoteExecutor(remoteExecutor);
       delegatingDownloader.setDelegate(env.getRuntime().getDownloaderSupplier().get());
+
+      clock = env.getClock();
+      try {
+        var lastRegistryInvalidationValue =
+            (PrecomputedValue)
+                env.getSkyframeExecutor()
+                    .getEvaluator()
+                    .getExistingValue(RegistryFunction.LAST_INVALIDATION.getKeyForTesting());
+        if (lastRegistryInvalidationValue != null) {
+          lastRegistryInvalidation = (Instant) lastRegistryInvalidationValue.get();
+        }
+      } catch (InterruptedException e) {
+        // Not thrown in Bazel.
+        throw new IllegalStateException(e);
+      }
     }
   }
 
@@ -556,6 +575,10 @@ private String getAbsolutePath(String path, CommandEnvironment env) {
 
   @Override
   public ImmutableList<Injected> getPrecomputedValues() {
+    Instant now = clock.now();
+    if (now.isAfter(lastRegistryInvalidation.plus(RegistryFunction.INVALIDATION_INTERVAL))) {
+      lastRegistryInvalidation = now;
+    }
     return ImmutableList.of(
         PrecomputedValue.injected(RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, overrides),
         PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, moduleOverrides),
@@ -582,7 +605,8 @@ public ImmutableList<Injected> getPrecomputedValues() {
         PrecomputedValue.injected(
             YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, allowedYankedVersions),
         PrecomputedValue.injected(
-            RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, disableNativeRepoRules));
+            RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, disableNativeRepoRules),
+        PrecomputedValue.injected(RegistryFunction.LAST_INVALIDATION, lastRegistryInvalidation));
   }
 
   @Override

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

@@ -71,7 +71,7 @@ public void afterCommand() {
       // Check the Skyframe value instead of the option since some commands (e.g. shutdown) don't
       // propagate the options to Skyframe, but we can only operate on Skyframe values that were
       // generated in UPDATE mode.
-      if (lockfileMode != LockfileMode.UPDATE) {
+      if (lockfileMode != LockfileMode.UPDATE && lockfileMode != LockfileMode.REFRESH) {
         return;
       }
       moduleResolutionValue =

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

@@ -58,8 +58,26 @@ public class IndexRegistry implements Registry {
    * registry.
    */
   public enum KnownFileHashesMode {
+    /**
+     * Neither use nor update any file hashes. All registry downloads will go out to the network.
+     */
     IGNORE,
+    /**
+     * Use file hashes from the lockfile if available and add hashes for new files to the lockfile.
+     * Avoid revalidation of mutable registry information (yanked versions in metadata.json and
+     * modules that previously 404'd) by using these hashes and recording absent files in the
+     * lockfile.
+     */
     USE_AND_UPDATE,
+    /**
+     * Use file hashes from the lockfile if available and add hashes for new files to the lockfile.
+     * Always revalidate mutable registry information.
+     */
+    USE_IMMUTABLE_AND_UPDATE,
+    /**
+     * Require file hashes for all registry downloads. In particular, mutable registry files such as
+     * metadata.json can't be downloaded in this mode.
+     */
     ENFORCE
   }
 
@@ -115,7 +133,9 @@ private Optional<byte[]> grabFile(
       String url, ExtendedEventHandler eventHandler, boolean useChecksum)
       throws IOException, InterruptedException {
     var maybeContent = doGrabFile(url, eventHandler, useChecksum);
-    if (knownFileHashesMode == KnownFileHashesMode.USE_AND_UPDATE && useChecksum) {
+    if ((knownFileHashesMode == KnownFileHashesMode.USE_AND_UPDATE
+            || knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE)
+        && useChecksum) {
       eventHandler.post(RegistryFileDownloadEvent.create(url, maybeContent));
     }
     return maybeContent;
@@ -139,8 +159,14 @@ private Optional<byte[]> doGrabFile(
         // This is a new file, download without providing a checksum.
         checksum = Optional.empty();
       } else if (knownChecksum.isEmpty()) {
-        // The file is known to not exist, so don't attempt to download it.
-        return Optional.empty();
+        // The file didn't exist when the lockfile was created, but it may exist now.
+        if (knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE) {
+          // Attempt to download the file again.
+          checksum = Optional.empty();
+        } else {
+          // Guarantee reproducibility by assuming that the file still doesn't exist.
+          return Optional.empty();
+        }
       } else {
         // The file is known, download with a checksum to potentially obtain a repository cache hit
         // and ensure that the remote file hasn't changed.
@@ -441,6 +467,10 @@ public Optional<ImmutableMap<Version, String>> getYankedVersions(
   @Override
   public Optional<YankedVersionsValue> tryGetYankedVersionsFromLockfile(
       ModuleKey selectedModuleKey) {
+    if (knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE) {
+      // Yanked version information is inherently mutable, so always refresh it when requested.
+      return Optional.empty();
+    }
     String yankedInfo = previouslySelectedYankedVersions.get(selectedModuleKey);
     if (yankedInfo != null) {
       // The module version was selected when the lockfile was created, but known to be yanked

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

@@ -17,12 +17,12 @@
 
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
-import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.NotComparableSkyValue;
 import java.io.IOException;
 import java.util.Optional;
 
 /** A database where module metadata is stored. */
-public interface Registry extends SkyValue {
+public interface Registry extends NotComparableSkyValue {
 
   /** The URL that uniquely identifies the registry. */
   String getUrl();

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

@@ -60,9 +60,11 @@ public Registry createRegistry(
     var knownFileHashesMode =
         switch (uri.getScheme()) {
           case "http", "https" ->
-              lockfileMode == LockfileMode.ERROR
-                  ? KnownFileHashesMode.ENFORCE
-                  : KnownFileHashesMode.USE_AND_UPDATE;
+              switch (lockfileMode) {
+                case ERROR -> KnownFileHashesMode.ENFORCE;
+                case REFRESH -> KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE;
+                case OFF, UPDATE -> KnownFileHashesMode.USE_AND_UPDATE;
+              };
           case "file" -> KnownFileHashesMode.IGNORE;
           default ->
               throw new URISyntaxException(uri.toString(), "Unrecognized registry URL protocol");

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

@@ -17,16 +17,32 @@
 
 import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
 import com.google.devtools.build.lib.server.FailureDetails;
+import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.net.URISyntaxException;
+import java.time.Duration;
+import java.time.Instant;
 import javax.annotation.Nullable;
 
 /** A simple SkyFunction that creates a {@link Registry} with a given URL. */
 public class RegistryFunction implements SkyFunction {
+  /**
+   * Set to the current time in {@link com.google.devtools.build.lib.bazel.BazelRepositoryModule}
+   * after {@link #INVALIDATION_INTERVAL} has passed. This is used to refresh the mutable registry
+   * contents cached in memory from time to time.
+   */
+  public static final Precomputed<Instant> LAST_INVALIDATION =
+      new Precomputed<>("last_registry_invalidation");
+
+  /**
+   * The interval after which the mutable registry contents cached in memory should be refreshed.
+   */
+  public static final Duration INVALIDATION_INTERVAL = Duration.ofHours(1);
+
   private final RegistryFactory registryFactory;
   private final Path workspaceRoot;
 
@@ -41,6 +57,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       throws InterruptedException, RegistryException {
     LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env);
 
+    if (lockfileMode == LockfileMode.REFRESH) {
+      RegistryFunction.LAST_INVALIDATION.get(env);
+    }
+
     BazelLockFileValue lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY);
     if (lockfile == null) {
       return null;

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

@@ -215,7 +215,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     // influence the evaluation of the extension and the validation also runs when the extension
     // result is taken from the lockfile, we can already populate the lockfile info. This is
     // necessary to prevent the extension from rerunning when only the imports change.
-    if (lockfileMode.equals(LockfileMode.UPDATE)) {
+    if (lockfileMode == LockfileMode.UPDATE || lockfileMode == LockfileMode.REFRESH) {
       lockFileInfo =
           Optional.of(
               new LockFileModuleExtension.WithFactors(

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

@@ -320,9 +320,11 @@ public Converter() {
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       help =
           "Specifies how and whether or not to use the lockfile. Valid values are `update` to"
-              + " use the lockfile and update it if there are changes, `error` to use the lockfile"
-              + " but throw an error if it's not up-to-date, or `off` to neither read from or write"
-              + " to the lockfile.")
+              + " use the lockfile and update it if there are changes, `refresh` to additionally"
+              + " refresh mutable information (yanked versions and previously missing modules)"
+              + " from remote registries from time to time, `error` to use the lockfile but throw"
+              + " an error if it's not up-to-date, or `off` to neither read from or write to the"
+              + " lockfile.")
   public LockfileMode lockfileMode;
 
   @Option(
@@ -369,10 +371,11 @@ public Converter() {
   /** An enum for specifying how to use the lockfile. */
   public enum LockfileMode {
     OFF, // Don't use the lockfile at all.
-    UPDATE, // Update the lockfile when it mismatches the module.
-    ERROR; // Throw an error when it mismatches the module.
+    UPDATE, // Update the lockfile wh
+    REFRESH,
+    ERROR; // Throw an error when it mismatc
 
-    /** Converts to {@link BazelLockfileMode}. */
+    /** Converts to {@link LockfileMode}. */
     public static class Converter extends EnumConverter<LockfileMode> {
       public Converter() {
         super(LockfileMode.class, "Lockfile mode");

src/test/py/bazel/bzlmod/bazel_lockfile_test.py

@@ -171,6 +171,41 @@ def testChangeModuleInRegistryWithLockfile(self):
     # Even adding a new dependency should not fail due to the registry change
     self.RunBazel(['build', '--nobuild', '//:all'])
 
+  def testChangeModuleInRegistryWithLockfileInRefreshMode(self):
+    # Add module 'sss' to the registry with dep on 'aaa'
+    self.main_registry.createCcModule('sss', '1.3', {'aaa': '1.1'})
+    # Create a project with deps on 'sss'
+    self.ScratchFile(
+        'MODULE.bazel',
+        [
+            'bazel_dep(name = "sss", version = "1.3")',
+        ],
+    )
+    self.ScratchFile('BUILD', ['filegroup(name = "hello")'])
+    self.RunBazel(['build', '--nobuild', '//:all'])
+
+    # Change registry -> update 'sss' module file (corrupt it)
+    module_dir = self.main_registry.root.joinpath('modules', 'sss', '1.3')
+    scratchFile(module_dir.joinpath('MODULE.bazel'), ['whatever!'])
+
+    # Shutdown bazel to empty any cache of the deps tree
+    self.RunBazel(['shutdown'])
+    # Running with the lockfile, should not recognize the registry changes
+    # hence find no errors
+    self.RunBazel(['build', '--nobuild', '--lockfile_mode=refresh', '//:all'])
+
+    self.ScratchFile(
+        'MODULE.bazel',
+        [
+            'bazel_dep(name = "sss", version = "1.3")',
+            'bazel_dep(name = "bbb", version = "1.1")',
+        ],
+    )
+    # Shutdown bazel to empty any cache of the deps tree
+    self.RunBazel(['shutdown'])
+    # Even adding a new dependency should not fail due to the registry change
+    self.RunBazel(['build', '--nobuild', '--lockfile_mode=refresh', '//:all'])
+
   def testAddModuleToRegistryWithLockfile(self):
     # Create a project with deps on the BCR's 'platforms' module
     self.ScratchFile(
@@ -206,6 +241,46 @@ def testAddModuleToRegistryWithLockfile(self):
     # Even adding a new dependency should not fail due to the registry change
     self.RunBazel(['build', '--nobuild', '//:all'])
 
+  def testAddModuleToRegistryWithLockfileInRefreshMode(self):
+    # Create a project with deps on the BCR's 'platforms' module
+    self.ScratchFile(
+        'MODULE.bazel',
+        [
+            'bazel_dep(name = "platforms", version = "0.0.9")',
+        ],
+    )
+    self.ScratchFile('BUILD', ['filegroup(name = "hello")'])
+    self.RunBazel(['build', '--nobuild', '//:all'])
+
+    # Add a broken 'platforms' module to the first registry
+    module_dir = self.main_registry.root.joinpath(
+        'modules', 'platforms', '0.0.9'
+    )
+    scratchFile(module_dir.joinpath('MODULE.bazel'), ['whatever!'])
+
+    # Shutdown bazel to empty any cache of the deps tree
+    self.RunBazel(['shutdown'])
+    # Running with the lockfile in refresh mode should recognize the addition
+    # to the first registry
+    exit_code, _, stderr = self.RunBazel(
+        [
+            'build',
+            '--nobuild',
+            '--lockfile_mode=refresh',
+            '//:all',
+        ],
+        allow_failure=True,
+    )
+    self.AssertExitCode(exit_code, 48, stderr)
+    self.assertIn(
+        (
+            'ERROR: Error computing the main repository mapping: in module '
+            'dependency chain <root> -> platforms@0.0.9: error parsing '
+            'MODULE.bazel file for platforms@0.0.9'
+        ),
+        stderr,
+    )
+
   def testChangeFlagWithLockfile(self):
     # Create a project with an outdated direct dep on aaa
     self.ScratchFile(