Allow module extension to declare it is reproducible and opt-out from…

… the lockfile (fixes #20272) Adding 'reproducible' to the module extension metadata PiperOrigin-RevId: 606288112 Change-Id: Ieca9c5eeb560e3556ba2b7e3f0a29c362bfa4f4b
bazelbuild · Feb 12, 2024 · c796aba · c796aba
1 parent c3db33e
commit c796aba
Show file tree

Hide file tree

Showing 6 changed files with 207 additions and 50 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java
@@ -121,18 +121,23 @@ public void afterCommand() throws AbruptExitException {
 
     // Add the new resolved extensions
     for (var event : extensionResolutionEventsMap.values()) {
+      LockFileModuleExtension extension = event.getModuleExtension();
+      if (!extension.shouldLockExtesnsion()) {
+        continue;
+      }
+
       var oldExtensionEntries = updatedExtensionMap.get(event.getExtensionId());
       ImmutableMap<ModuleExtensionEvalFactors, LockFileModuleExtension> extensionEntries;
       if (oldExtensionEntries != null) {
         // extension exists, add the new entry to the existing map
         extensionEntries =
             new ImmutableMap.Builder<ModuleExtensionEvalFactors, LockFileModuleExtension>()
                 .putAll(oldExtensionEntries)
-                .put(event.getExtensionFactors(), event.getModuleExtension())
+                .put(event.getExtensionFactors(), extension)
                 .buildKeepingLast();
       } else {
         // new extension
-        extensionEntries = ImmutableMap.of(event.getExtensionFactors(), event.getModuleExtension());
+        extensionEntries = ImmutableMap.of(event.getExtensionFactors(), extension);
       }
       updatedExtensionMap.put(event.getExtensionId(), extensionEntries);
     }
@@ -164,12 +169,13 @@ private boolean shouldKeepExtension(
     // If there is a new event for this extension, compare it with the existing ones
     ModuleExtensionResolutionEvent extEvent = extensionResolutionEventsMap.get(extensionId);
     if (extEvent != null) {
+      boolean doNotLockExtension = !extEvent.getModuleExtension().shouldLockExtesnsion();
       boolean dependencyOnOsChanged =
           lockedExtensionKey.getOs().isEmpty() != extEvent.getExtensionFactors().getOs().isEmpty();
       boolean dependencyOnArchChanged =
           lockedExtensionKey.getArch().isEmpty()
               != extEvent.getExtensionFactors().getArch().isEmpty();
-      if (dependencyOnOsChanged || dependencyOnArchChanged) {
+      if (doNotLockExtension || dependencyOnOsChanged || dependencyOnArchChanged) {
         return false;
       }
     }

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java
@@ -54,6 +54,11 @@ public static Builder builder() {
 
   public abstract Builder toBuilder();
 
+  public boolean shouldLockExtesnsion() {
+    return getModuleExtensionMetadata().isEmpty()
+        || !getModuleExtensionMetadata().get().getReproducible();
+  }
+
   /** Builder type for {@link LockFileModuleExtension}. */
   @AutoValue.Builder
   public abstract static class Builder {

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java
@@ -206,11 +206,24 @@ public boolean rootModuleHasNonDevDependency() {
               @ParamType(type = String.class),
               @ParamType(type = NoneType.class)
             }),
+        @Param(
+            name = "reproducible",
+            doc =
+                "States that this module extension ensures complete reproducibility, thereby it "
+                    + "should not be stored in the lockfile.",
+            positional = false,
+            named = true,
+            defaultValue = "False",
+            allowedTypes = {
+              @ParamType(type = Boolean.class),
+            }),
       })
   public ModuleExtensionMetadata extensionMetadata(
-      Object rootModuleDirectDepsUnchecked, Object rootModuleDirectDevDepsUnchecked)
+      Object rootModuleDirectDepsUnchecked,
+      Object rootModuleDirectDevDepsUnchecked,
+      boolean reproducible)
       throws EvalException {
     return ModuleExtensionMetadata.create(
-        rootModuleDirectDepsUnchecked, rootModuleDirectDevDepsUnchecked);
+        rootModuleDirectDepsUnchecked, rootModuleDirectDevDepsUnchecked, reproducible);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java
@@ -63,38 +63,44 @@ public abstract class ModuleExtensionMetadata implements StarlarkValue {
 
   abstract UseAllRepos getUseAllRepos();
 
+  abstract boolean getReproducible();
+
   private static ModuleExtensionMetadata create(
       @Nullable Set<String> explicitRootModuleDirectDeps,
       @Nullable Set<String> explicitRootModuleDirectDevDeps,
-      UseAllRepos useAllRepos) {
+      UseAllRepos useAllRepos,
+      boolean reproducible) {
     return new AutoValue_ModuleExtensionMetadata(
         explicitRootModuleDirectDeps != null
             ? ImmutableSet.copyOf(explicitRootModuleDirectDeps)
             : null,
         explicitRootModuleDirectDevDeps != null
             ? ImmutableSet.copyOf(explicitRootModuleDirectDevDeps)
             : null,
-        useAllRepos);
+        useAllRepos,
+        reproducible);
   }
 
   static ModuleExtensionMetadata create(
-      Object rootModuleDirectDepsUnchecked, Object rootModuleDirectDevDepsUnchecked)
+      Object rootModuleDirectDepsUnchecked,
+      Object rootModuleDirectDevDepsUnchecked,
+      boolean reproducible)
       throws EvalException {
     if (rootModuleDirectDepsUnchecked == Starlark.NONE
         && rootModuleDirectDevDepsUnchecked == Starlark.NONE) {
-      return create(null, null, UseAllRepos.NO);
+      return create(null, null, UseAllRepos.NO, reproducible);
     }
 
     // When root_module_direct_deps = "all", accept both root_module_direct_dev_deps = None and
     // root_module_direct_dev_deps = [], but not root_module_direct_dev_deps = ["some_repo"].
     if (rootModuleDirectDepsUnchecked.equals("all")
         && rootModuleDirectDevDepsUnchecked.equals(StarlarkList.immutableOf())) {
-      return create(null, null, UseAllRepos.REGULAR);
+      return create(null, null, UseAllRepos.REGULAR, reproducible);
     }
 
     if (rootModuleDirectDevDepsUnchecked.equals("all")
         && rootModuleDirectDepsUnchecked.equals(StarlarkList.immutableOf())) {
-      return create(null, null, UseAllRepos.DEV);
+      return create(null, null, UseAllRepos.DEV, reproducible);
     }
 
     if (rootModuleDirectDepsUnchecked.equals("all")
@@ -152,7 +158,11 @@ static ModuleExtensionMetadata create(
       }
     }
 
-    return create(explicitRootModuleDirectDeps, explicitRootModuleDirectDevDeps, UseAllRepos.NO);
+    return create(
+        explicitRootModuleDirectDeps,
+        explicitRootModuleDirectDevDeps,
+        UseAllRepos.NO,
+        reproducible);
   }
 
   public void evaluate(

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java
@@ -151,20 +151,27 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     }
 
     // Check the lockfile first for that module extension
+    LockFileModuleExtension lockedExtension = null;
     LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env);
     if (!lockfileMode.equals(LockfileMode.OFF)) {
       BazelLockFileValue lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY);
       if (lockfile == null) {
         return null;
       }
-      try {
-        SingleExtensionEvalValue singleExtensionEvalValue =
-            tryGettingValueFromLockFile(env, extensionId, extension, usagesValue, lockfile);
-        if (singleExtensionEvalValue != null) {
-          return singleExtensionEvalValue;
+      var lockedExtensionMap = lockfile.getModuleExtensions().get(extensionId);
+      lockedExtension =
+          lockedExtensionMap == null ? null : lockedExtensionMap.get(extension.getEvalFactors());
+      if (lockedExtension != null) {
+        try {
+          SingleExtensionEvalValue singleExtensionEvalValue =
+              tryGettingValueFromLockFile(
+                  env, extensionId, extension, usagesValue, lockfile, lockedExtension);
+          if (singleExtensionEvalValue != null) {
+            return singleExtensionEvalValue;
+          }
+        } catch (NeedsSkyframeRestartException e) {
+          return null;
         }
-      } catch (NeedsSkyframeRestartException e) {
-        return null;
       }
     }
 
@@ -181,6 +188,24 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     Optional<ModuleExtensionMetadata> moduleExtensionMetadata =
         moduleExtensionResult.getModuleExtensionMetadata();
 
+    if (lockfileMode.equals(LockfileMode.ERROR)) {
+      boolean extensionShouldHaveBeenLocked =
+          moduleExtensionMetadata.map(metadata -> !metadata.getReproducible()).orElse(true);
+      // If this extension was not found in the lockfile, and after evaluation we found that it is
+      // not reproducible, then error indicating that it was expected to be in the lockfile.
+      if (lockedExtension == null && extensionShouldHaveBeenLocked) {
+        throw new SingleExtensionEvalFunctionException(
+            ExternalDepsException.withMessage(
+                Code.BAD_MODULE,
+                "The module extension '%s'%s does not exist in the lockfile",
+                extensionId,
+                extension.getEvalFactors().isEmpty()
+                    ? ""
+                    : " for platform " + extension.getEvalFactors()),
+            Transience.PERSISTENT);
+      }
+    }
+
     // At this point the extension has been evaluated successfully, but SingleExtensionEvalFunction
     // may still fail if imported repositories were not generated. However, since imports do not
     // influence the evaluation of the extension and the validation also runs when the extension
@@ -219,30 +244,13 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile(
       ModuleExtensionId extensionId,
       RunnableExtension extension,
       SingleExtensionUsagesValue usagesValue,
-      BazelLockFileValue lockfile)
+      BazelLockFileValue lockfile,
+      LockFileModuleExtension lockedExtension)
       throws SingleExtensionEvalFunctionException,
           InterruptedException,
           NeedsSkyframeRestartException {
     LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env);
 
-    var lockedExtensionMap = lockfile.getModuleExtensions().get(extensionId);
-    LockFileModuleExtension lockedExtension =
-        lockedExtensionMap == null ? null : lockedExtensionMap.get(extension.getEvalFactors());
-    if (lockedExtension == null) {
-      if (lockfileMode.equals(LockfileMode.ERROR)) {
-        throw new SingleExtensionEvalFunctionException(
-            ExternalDepsException.withMessage(
-                Code.BAD_MODULE,
-                "The module extension '%s'%s does not exist in the lockfile",
-                extensionId,
-                extension.getEvalFactors().isEmpty()
-                    ? ""
-                    : " for platform " + extension.getEvalFactors()),
-            Transience.PERSISTENT);
-      }
-      return null;
-    }
-
     ImmutableMap<ModuleKey, ModuleExtensionUsage> lockedExtensionUsages;
     try {
       // TODO(salmasamy) might be nicer to precompute this table when we construct