[6.4.0] Do not rerun module extensions when only imports or locations…

… change (bazelbuild#19284)

When using the lockfile, extension evaluation results are now persisted
even if the imports (`use_repo`s) fail validation against the actually
generated repos and reused if only imports or Starlark locations change.

This requires storing `ModuleExtensionMetadata` in the lockfile to
ensure that the correct fixup warnings are shown if imports are updated
but the extension isn't rerun.

Closes bazelbuild#19253.

Commit
bazelbuild@2c2b07b

PiperOrigin-RevId: 557878432
Change-Id: I25909294b118fb445f9c48f61e18463762f78208

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>

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

@@ -138,6 +138,7 @@ java_library(
         ":common",
         ":inspection",
         ":module_extension",
+        ":module_extension_metadata",
         ":registry",
         ":repo_rule_creator",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
@@ -170,7 +171,6 @@ java_library(
         "Discovery.java",
         "GsonTypeAdapterUtil.java",
         "ModuleExtensionContext.java",
-        "ModuleExtensionMetadata.java",
         "ModuleFileFunction.java",
         "ModuleFileGlobals.java",
         "Selection.java",
@@ -184,6 +184,7 @@ java_library(
         ":common",
         ":exception",
         ":module_extension",
+        ":module_extension_metadata",
         ":registry",
         ":resolution",
         "//src/main/java/com/google/devtools/build/docgen/annot",
@@ -284,3 +285,24 @@ java_library(
         "//third_party:jsr305",
     ],
 )
+
+java_library(
+    name = "module_extension_metadata",
+    srcs = [
+        "ModuleExtensionMetadata.java",
+    ],
+    deps = [
+        ":common",
+        ":module_extension",
+        "//src/main/java/com/google/devtools/build/docgen/annot",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/main/java/com/google/devtools/build/lib/events",
+        "//src/main/java/net/starlark/java/annot",
+        "//src/main/java/net/starlark/java/eval",
+        "//src/main/java/net/starlark/java/syntax",
+        "//third_party:auto_value",
+        "//third_party:gson",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)

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

@@ -130,7 +130,7 @@ public ImmutableList<String> getModuleExtensionDiff(
       extDiff.add("One or more files the extension '" + extensionId + "' is using have changed");
     }
     if (!extensionUsages.equals(lockedExtensionUsages)) {
-      extDiff.add("The usages of the extension '" + extensionId + "' has changed");
+      extDiff.add("The usages of the extension '" + extensionId + "' have changed");
     }
     if (!envVariables.equals(lockedExtension.getEnvVariables())) {
       extDiff.add(

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

@@ -19,6 +19,7 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
 import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
+import java.util.Optional;
 
 /**
  * This object serves as a container for the transitive digest (obtained from transitive .bzl files)
@@ -33,7 +34,8 @@ public static Builder builder() {
     return new AutoValue_LockFileModuleExtension.Builder()
         // TODO(salmasamy) can be removed when updating lockfile version
         .setEnvVariables(ImmutableMap.of())
-        .setAccumulatedFileDigests(ImmutableMap.of());
+        .setAccumulatedFileDigests(ImmutableMap.of())
+        .setModuleExtensionMetadata(Optional.empty());
   }
 
   @SuppressWarnings("mutable")
@@ -45,6 +47,8 @@ public static Builder builder() {
 
   public abstract ImmutableMap<String, RepoSpec> getGeneratedRepoSpecs();
 
+  public abstract Optional<ModuleExtensionMetadata> getModuleExtensionMetadata();
+
   public abstract Builder toBuilder();
 
   /** Builder type for {@link LockFileModuleExtension}. */
@@ -59,6 +63,8 @@ public abstract static class Builder {
 
     public abstract Builder setGeneratedRepoSpecs(ImmutableMap<String, RepoSpec> value);
 
+    public abstract Builder setModuleExtensionMetadata(Optional<ModuleExtensionMetadata> value);
+
     public abstract LockFileModuleExtension build();
   }
 }

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

@@ -18,6 +18,7 @@
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static java.util.stream.Collectors.joining;
 
+import com.google.auto.value.AutoValue;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
@@ -27,6 +28,7 @@
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedHashSet;
@@ -49,24 +51,29 @@
     doc =
         "Return values of this type from a module extension's implementation function to "
             + "provide metadata about the repositories generated by the extension to Bazel.")
-public class ModuleExtensionMetadata implements StarlarkValue {
-  @Nullable private final ImmutableSet<String> explicitRootModuleDirectDeps;
-  @Nullable private final ImmutableSet<String> explicitRootModuleDirectDevDeps;
-  private final UseAllRepos useAllRepos;
+@AutoValue
+@GenerateTypeAdapter
+public abstract class ModuleExtensionMetadata implements StarlarkValue {
+  @Nullable
+  abstract ImmutableSet<String> getExplicitRootModuleDirectDeps();
 
-  private ModuleExtensionMetadata(
+  @Nullable
+  abstract ImmutableSet<String> getExplicitRootModuleDirectDevDeps();
+
+  abstract UseAllRepos getUseAllRepos();
+
+  private static ModuleExtensionMetadata create(
       @Nullable Set<String> explicitRootModuleDirectDeps,
       @Nullable Set<String> explicitRootModuleDirectDevDeps,
       UseAllRepos useAllRepos) {
-    this.explicitRootModuleDirectDeps =
+    return new AutoValue_ModuleExtensionMetadata(
         explicitRootModuleDirectDeps != null
             ? ImmutableSet.copyOf(explicitRootModuleDirectDeps)
-            : null;
-    this.explicitRootModuleDirectDevDeps =
+            : null,
         explicitRootModuleDirectDevDeps != null
             ? ImmutableSet.copyOf(explicitRootModuleDirectDevDeps)
-            : null;
-    this.useAllRepos = useAllRepos;
+            : null,
+        useAllRepos);
   }
 
   static ModuleExtensionMetadata create(
@@ -76,19 +83,19 @@ static ModuleExtensionMetadata create(
       throws EvalException {
     if (rootModuleDirectDepsUnchecked == Starlark.NONE
         && rootModuleDirectDevDepsUnchecked == Starlark.NONE) {
-      return new ModuleExtensionMetadata(null, null, UseAllRepos.NO);
+      return create(null, null, UseAllRepos.NO);
     }
 
     // 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 new ModuleExtensionMetadata(null, null, UseAllRepos.REGULAR);
+      return create(null, null, UseAllRepos.REGULAR);
     }
 
     if (rootModuleDirectDevDepsUnchecked.equals("all")
         && rootModuleDirectDepsUnchecked.equals(StarlarkList.immutableOf())) {
-      return new ModuleExtensionMetadata(null, null, UseAllRepos.DEV);
+      return create(null, null, UseAllRepos.DEV);
     }
 
     if (rootModuleDirectDepsUnchecked.equals("all")
@@ -146,8 +153,7 @@ static ModuleExtensionMetadata create(
       }
     }
 
-    return new ModuleExtensionMetadata(
-        explicitRootModuleDirectDeps, explicitRootModuleDirectDevDeps, UseAllRepos.NO);
+    return create(explicitRootModuleDirectDeps, explicitRootModuleDirectDevDeps, UseAllRepos.NO);
   }
 
   public void evaluate(
@@ -358,18 +364,18 @@ private static Optional<String> makeUseRepoCommand(
 
   private Optional<ImmutableSet<String>> getRootModuleDirectDeps(Set<String> allRepos)
       throws EvalException {
-    switch (useAllRepos) {
+    switch (getUseAllRepos()) {
       case NO:
-        if (explicitRootModuleDirectDeps != null) {
-          Set<String> invalidRepos = Sets.difference(explicitRootModuleDirectDeps, allRepos);
+        if (getExplicitRootModuleDirectDeps() != null) {
+          Set<String> invalidRepos = Sets.difference(getExplicitRootModuleDirectDeps(), allRepos);
           if (!invalidRepos.isEmpty()) {
             throw Starlark.errorf(
                 "root_module_direct_deps contained the following repositories "
                     + "not generated by the extension: %s",
                 String.join(", ", invalidRepos));
           }
         }
-        return Optional.ofNullable(explicitRootModuleDirectDeps);
+        return Optional.ofNullable(getExplicitRootModuleDirectDeps());
       case REGULAR:
         return Optional.of(ImmutableSet.copyOf(allRepos));
       case DEV:
@@ -380,18 +386,19 @@ private Optional<ImmutableSet<String>> getRootModuleDirectDeps(Set<String> allRe
 
   private Optional<ImmutableSet<String>> getRootModuleDirectDevDeps(Set<String> allRepos)
       throws EvalException {
-    switch (useAllRepos) {
+    switch (getUseAllRepos()) {
       case NO:
-        if (explicitRootModuleDirectDevDeps != null) {
-          Set<String> invalidRepos = Sets.difference(explicitRootModuleDirectDevDeps, allRepos);
+        if (getExplicitRootModuleDirectDevDeps() != null) {
+          Set<String> invalidRepos =
+              Sets.difference(getExplicitRootModuleDirectDevDeps(), allRepos);
           if (!invalidRepos.isEmpty()) {
             throw Starlark.errorf(
                 "root_module_direct_dev_deps contained the following "
                     + "repositories not generated by the extension: %s",
                 String.join(", ", invalidRepos));
           }
         }
-        return Optional.ofNullable(explicitRootModuleDirectDevDeps);
+        return Optional.ofNullable(getExplicitRootModuleDirectDevDeps());
       case REGULAR:
         return Optional.of(ImmutableSet.of());
       case DEV:
@@ -400,7 +407,7 @@ private Optional<ImmutableSet<String>> getRootModuleDirectDevDeps(Set<String> al
     throw new IllegalStateException("not reached");
   }
 
-  private enum UseAllRepos {
+  enum UseAllRepos {
     NO,
     REGULAR,
     DEV,

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

@@ -80,6 +80,8 @@ public abstract class ModuleExtensionUsage {
    */
   public abstract boolean getHasNonDevUseExtension();
 
+  public abstract Builder toBuilder();
+
   public static Builder builder() {
     return new AutoValue_ModuleExtensionUsage.Builder();
   }