bzlmod: Remove override_dep in favor of having module_name on each ov…

…erride type

(#13316)

We're "denormalizing" the override API: whereas you'd write "override_dep(name='X', Y_override(...))" before, you'd now write "Y_override(module_name='X', ...)".

PiperOrigin-RevId: 380173439

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

@@ -40,6 +40,7 @@ java_library(
         "ModuleFileFunction.java",
         "ModuleFileGlobals.java",
         "ModuleFileValue.java",
+        "ModuleOverride.java",
         "NonRegistryOverride.java",
         "RegistryOverride.java",
         "SingleVersionOverride.java",

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

@@ -22,7 +22,6 @@
 import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
-import com.google.devtools.build.lib.starlarkbuildapi.repository.StarlarkOverrideApi;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -121,8 +120,8 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir
     Module module = moduleFileGlobals.buildModule(null);
 
     // Check that overrides don't contain the root module itself.
-    ImmutableMap<String, StarlarkOverrideApi> overrides = moduleFileGlobals.buildOverrides();
-    StarlarkOverrideApi rootOverride = overrides.get(module.getName());
+    ImmutableMap<String, ModuleOverride> overrides = moduleFileGlobals.buildOverrides();
+    ModuleOverride rootOverride = overrides.get(module.getName());
     if (rootOverride != null) {
       throw errorf("invalid override for the root module found: %s", rootOverride);
     }
@@ -162,7 +161,7 @@ private static class GetModuleFileResult {
 
   @Nullable
   private GetModuleFileResult getModuleFile(
-      ModuleKey key, @Nullable StarlarkOverrideApi override, Environment env)
+      ModuleKey key, @Nullable ModuleOverride override, Environment env)
       throws ModuleFileFunctionException, InterruptedException {
     // If there is a non-registry override for this module, we need to fetch the corresponding repo
     // first and read the module file from there.

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

@@ -19,7 +19,6 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction.ModuleFileFunctionException;
 import com.google.devtools.build.lib.starlarkbuildapi.repository.ModuleFileGlobalsApi;
-import com.google.devtools.build.lib.starlarkbuildapi.repository.StarlarkOverrideApi;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -33,7 +32,7 @@ public class ModuleFileGlobals implements ModuleFileGlobalsApi<ModuleFileFunctio
   private boolean moduleCalled = false;
   private final Module.Builder module = Module.builder();
   private final Map<String, ModuleKey> deps = new LinkedHashMap<>();
-  private final Map<String, StarlarkOverrideApi> overrides = new HashMap<>();
+  private final Map<String, ModuleOverride> overrides = new HashMap<>();
 
   public ModuleFileGlobals() {}
 
@@ -57,11 +56,10 @@ public void bazelDep(String name, String version, String repoName) throws EvalEx
     }
   }
 
-  @Override
-  public void overrideDep(String name, StarlarkOverrideApi override) throws EvalException {
-    StarlarkOverrideApi existingOverride = overrides.putIfAbsent(name, override);
+  private void addOverride(String moduleName, ModuleOverride override) throws EvalException {
+    ModuleOverride existingOverride = overrides.putIfAbsent(moduleName, override);
     if (existingOverride != null) {
-      throw Starlark.errorf("multiple overrides for dep %s found", name);
+      throw Starlark.errorf("multiple overrides for dep %s found", moduleName);
     }
   }
 
@@ -82,18 +80,25 @@ private static ImmutableList<String> checkAllStrings(Iterable<?> iterable, Strin
   }
 
   @Override
-  public StarlarkOverrideApi singleVersionOverride(
-      String version, String registry, Iterable<?> patches, StarlarkInt patchStrip)
+  public void singleVersionOverride(
+      String moduleName,
+      String version,
+      String registry,
+      Iterable<?> patches,
+      StarlarkInt patchStrip)
       throws EvalException {
-    return SingleVersionOverride.create(
-        version,
-        registry,
-        checkAllStrings(patches, "patches"),
-        patchStrip.toInt("single_version_override.patch_strip"));
+    addOverride(
+        moduleName,
+        SingleVersionOverride.create(
+            version,
+            registry,
+            checkAllStrings(patches, "patches"),
+            patchStrip.toInt("single_version_override.patch_strip")));
   }
 
   @Override
-  public StarlarkOverrideApi archiveOverride(
+  public void archiveOverride(
+      String moduleName,
       Object urls,
       String integrity,
       String stripPrefix,
@@ -104,35 +109,39 @@ public StarlarkOverrideApi archiveOverride(
         urls instanceof String
             ? ImmutableList.of((String) urls)
             : checkAllStrings((Iterable<?>) urls, "urls");
-    return ArchiveOverride.create(
-        urlList,
-        checkAllStrings(patches, "patches"),
-        integrity,
-        stripPrefix,
-        patchStrip.toInt("archive_override.patch_strip"));
+    addOverride(
+        moduleName,
+        ArchiveOverride.create(
+            urlList,
+            checkAllStrings(patches, "patches"),
+            integrity,
+            stripPrefix,
+            patchStrip.toInt("archive_override.patch_strip")));
   }
 
   @Override
-  public StarlarkOverrideApi gitOverride(
-      String remote, String commit, Iterable<?> patches, StarlarkInt patchStrip)
+  public void gitOverride(
+      String moduleName, String remote, String commit, Iterable<?> patches, StarlarkInt patchStrip)
       throws EvalException {
-    return GitOverride.create(
-        remote,
-        commit,
-        checkAllStrings(patches, "patches"),
-        patchStrip.toInt("git_override.patch_strip"));
+    addOverride(
+        moduleName,
+        GitOverride.create(
+            remote,
+            commit,
+            checkAllStrings(patches, "patches"),
+            patchStrip.toInt("git_override.patch_strip")));
   }
 
   @Override
-  public StarlarkOverrideApi localPathOverride(String path) {
-    return LocalPathOverride.create(path);
+  public void localPathOverride(String moduleName, String path) throws EvalException {
+    addOverride(moduleName, LocalPathOverride.create(path));
   }
 
   public Module buildModule(Registry registry) {
     return module.setDeps(ImmutableMap.copyOf(deps)).setRegistry(registry).build();
   }
 
-  public ImmutableMap<String, StarlarkOverrideApi> buildOverrides() {
+  public ImmutableMap<String, ModuleOverride> buildOverrides() {
     return ImmutableMap.copyOf(overrides);
   }
 }

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

@@ -22,7 +22,6 @@
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.lib.starlarkbuildapi.repository.StarlarkOverrideApi;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -45,14 +44,14 @@ public abstract class ModuleFileValue implements SkyValue {
    * The overrides specified by the evaluated module file. The key is the module name and the value
    * is the override itself.
    */
-  public abstract ImmutableMap<String, StarlarkOverrideApi> getOverrides();
+  public abstract ImmutableMap<String, ModuleOverride> getOverrides();
 
   public static ModuleFileValue create(
-      Module module, ImmutableMap<String, StarlarkOverrideApi> overrides) {
+      Module module, ImmutableMap<String, ModuleOverride> overrides) {
     return new AutoValue_ModuleFileValue(module, overrides);
   }
 
-  public static Key key(ModuleKey moduleKey, @Nullable StarlarkOverrideApi override) {
+  public static Key key(ModuleKey moduleKey, @Nullable ModuleOverride override) {
     return Key.create(moduleKey, override);
   }
 
@@ -75,11 +74,11 @@ abstract static class Key implements SkyKey {
     abstract ModuleKey getModuleKey();
 
     @Nullable
-    abstract StarlarkOverrideApi getOverride();
+    abstract ModuleOverride getOverride();
 
     @AutoCodec.VisibleForSerialization
     @AutoCodec.Instantiator
-    static Key create(ModuleKey moduleKey, @Nullable StarlarkOverrideApi override) {
+    static Key create(ModuleKey moduleKey, @Nullable ModuleOverride override) {
       return interner.intern(new AutoValue_ModuleFileValue_Key(moduleKey, override));
     }
 

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/StarlarkOverrideApi.java → src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleOverride.java

@@ -13,17 +13,14 @@
 // limitations under the License.
 //
 
-package com.google.devtools.build.lib.starlarkbuildapi.repository;
+package com.google.devtools.build.lib.bazel.bzlmod;
 
-import com.google.devtools.build.docgen.annot.DocCategory;
-import net.starlark.java.annot.StarlarkBuiltin;
-import net.starlark.java.eval.StarlarkValue;
-
-/** Base interface for override objects. */
-@StarlarkBuiltin(
-    name = "override",
-    category = DocCategory.BUILTIN,
-    doc = "Overrides some information about a Bazel module dependency.")
-public interface StarlarkOverrideApi extends StarlarkValue {
+/**
+ * Represents an "override" applied to a particular module in the dependency graph. Must be of one
+ * of two categories: {@link RegistryOverride} and {@link NonRegistryOverride}. See there for
+ * further details.
+ */
+// This interface is not named "Override" because of the Java @Override annotation.
+public interface ModuleOverride {
   // This space intentionally left blank
 }

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

@@ -15,15 +15,13 @@
 
 package com.google.devtools.build.lib.bazel.bzlmod;
 
-import com.google.devtools.build.lib.starlarkbuildapi.repository.StarlarkOverrideApi;
-
 /**
  * An override specifying that the module should not be retrieved from a registry or participate in
  * version resolution, and should be retrieved from another given source instead. To evaluate the
  * module file of such modules, we need to first fetch the entire module contents and find the
  * module file in the root of the module.
  */
-public interface NonRegistryOverride extends StarlarkOverrideApi {
+public interface NonRegistryOverride extends ModuleOverride {
 
   // TODO(wyv): getRepoSpec
 }

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

@@ -15,13 +15,11 @@
 
 package com.google.devtools.build.lib.bazel.bzlmod;
 
-import com.google.devtools.build.lib.starlarkbuildapi.repository.StarlarkOverrideApi;
-
 /**
  * An override specifying that the module should still come from a registry, albeit with some other
  * properties overridden (such as which registry it comes from, whether patches are applied, etc.)
  */
-public interface RegistryOverride extends StarlarkOverrideApi {
+public interface RegistryOverride extends ModuleOverride {
 
   /**
    * The registry that should be used instead of the default list. Can be empty if there is no

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/ModuleFileGlobalsApi.java

@@ -91,33 +91,18 @@ void bazelDep(String name, String version, String repoName)
       throws EvalException, InterruptedException;
 
   @StarlarkMethod(
-      name = "override_dep",
+      name = "single_version_override",
       doc =
-          " Specifies that a direct or indirect dependency should be overridden to act in some way"
-              + " other than the default. This directive can only be used by the root module; in"
-              + " other words, if a module specifies any overrides, it cannot be used as a"
-              + " dependency by others.",
+          "Specifies that a dependency should still come from a registry, but its version should"
+              + " be pinned, or its registry overridden, or a list of patches applied. This"
+              + " directive can only be used by the root module; in other words, if a module"
+              + " specifies any overrides, it cannot be used as a dependency by others.",
       parameters = {
         @Param(
-            name = "name",
-            doc = "The name of the Bazel module dependency to apply an override to.",
+            name = "module_name",
+            doc = "The name of the Bazel module dependency to apply this override to.",
             named = true,
             positional = false),
-        @Param(
-            name = "override",
-            doc = "The actual override to apply.",
-            named = true,
-            positional = false),
-      })
-  void overrideDep(String name, StarlarkOverrideApi override)
-      throws EvalException, InterruptedException;
-
-  @StarlarkMethod(
-      name = "single_version_override",
-      doc =
-          "Specifies that the dependency should still come from a registry, but its version should"
-              + " be pinned, or its registry overridden, or a list of patches applied.",
-      parameters = {
         @Param(
             name = "version",
             doc =
@@ -153,16 +138,27 @@ void overrideDep(String name, StarlarkOverrideApi override)
             positional = false,
             defaultValue = "0"),
       })
-  StarlarkOverrideApi singleVersionOverride(
-      String version, String registry, Iterable<?> patches, StarlarkInt patchStrip)
+  void singleVersionOverride(
+      String moduleName,
+      String version,
+      String registry,
+      Iterable<?> patches,
+      StarlarkInt patchStrip)
       throws EvalException;
 
   @StarlarkMethod(
       name = "archive_override",
       doc =
           "Specifies that this dependency should come from an archive file (zip, gzip, etc) at a"
-              + " certain location, instead of from a registry.",
+              + " certain location, instead of from a registry. This directive can only be used by"
+              + " the root module; in other words, if a module specifies any overrides, it cannot"
+              + " be used as a dependency by others.",
       parameters = {
+        @Param(
+            name = "module_name",
+            doc = "The name of the Bazel module dependency to apply this override to.",
+            named = true,
+            positional = false),
         @Param(
             name = "urls",
             allowedTypes = {
@@ -201,7 +197,8 @@ StarlarkOverrideApi singleVersionOverride(
             positional = false,
             defaultValue = "0"),
       })
-  StarlarkOverrideApi archiveOverride(
+  void archiveOverride(
+      String moduleName,
       Object urls,
       String integrity,
       String stripPrefix,
@@ -211,8 +208,16 @@ StarlarkOverrideApi archiveOverride(
 
   @StarlarkMethod(
       name = "git_override",
-      doc = "Specifies that this dependency should come from a certain commit of a Git repository.",
+      doc =
+          "Specifies that a dependency should come from a certain commit of a Git repository. This"
+              + " directive can only be used by the root module; in other words, if a module"
+              + " specifies any overrides, it cannot be used as a dependency by others.",
       parameters = {
+        @Param(
+            name = "module_name",
+            doc = "The name of the Bazel module dependency to apply this override to.",
+            named = true,
+            positional = false),
         @Param(
             name = "remote",
             doc = "The URL of the remote Git repository.",
@@ -241,21 +246,29 @@ StarlarkOverrideApi archiveOverride(
             positional = false,
             defaultValue = "0"),
       })
-  StarlarkOverrideApi gitOverride(
-      String remote, String commit, Iterable<?> patches, StarlarkInt patchStrip)
+  void gitOverride(
+      String moduleName, String remote, String commit, Iterable<?> patches, StarlarkInt patchStrip)
       throws EvalException, ModuleFileFunctionExceptionT;
 
   @StarlarkMethod(
       name = "local_path_override",
-      doc = "Specifies that this dependency should come from a certain directory on local disk.",
+      doc =
+          "Specifies that a dependency should come from a certain directory on local disk. This"
+              + " directive can only be used by the root module; in other words, if a module"
+              + " specifies any overrides, it cannot be used as a dependency by others.",
       parameters = {
+        @Param(
+            name = "module_name",
+            doc = "The name of the Bazel module dependency to apply this override to.",
+            named = true,
+            positional = false),
         @Param(
             name = "path",
             doc = "The path to the directory where this module is.",
             named = true,
             positional = false),
       })
-  StarlarkOverrideApi localPathOverride(String path);
+  void localPathOverride(String moduleName, String path) throws EvalException;
 
   // TODO(wyv): multiple_version_override
 }