[7.1.0] Do not store the repository name in RepoSpec (bazelbuild#21209

) The name can instead be set centrally in `BzlmodRepoRuleFunction`. This removes some unnecessary glue code and paves the way for rewriting canonical repo names based on knowledge of the full dep graph. Work towards bazelbuild#20997 Closes bazelbuild#21026. PiperOrigin-RevId: 604256186 Change-Id: Ia49123f016d1512074b04ec458f017f4bbb88233 Co-authored-by: Fabian Meumertzheim <[email protected]>
comius · Feb 6, 2024 · edd44fd · edd44fd
1 parent 8bb5662
commit edd44fd
Show file tree

Hide file tree

Showing 20 changed files with 104 additions and 310 deletions.
diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -261,11 +261,10 @@ public void workspaceInit(
               "local_config_platform",
               new NonRegistryOverride() {
                 @Override
-                public RepoSpec getRepoSpec(RepositoryName repoName) {
+                public RepoSpec getRepoSpec() {
                   return RepoSpec.builder()
                       .setRuleClassName("local_config_platform")
-                      .setAttributes(
-                          AttributeValues.create(ImmutableMap.of("name", repoName.getName())))
+                      .setAttributes(AttributeValues.create(ImmutableMap.of()))
                       .build();
                 }
 

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ArchiveOverride.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ArchiveOverride.java
@@ -18,7 +18,6 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule.ResolutionReason;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /** Specifies that a module should be retrieved from an archive. */
 @AutoValue
@@ -55,9 +54,8 @@ public static ArchiveOverride create(
 
   /** Returns the {@link RepoSpec} that defines this repository. */
   @Override
-  public RepoSpec getRepoSpec(RepositoryName repoName) {
+  public RepoSpec getRepoSpec() {
     return new ArchiveRepoSpecBuilder()
-        .setRepoName(repoName.getName())
         .setUrls(getUrls())
         .setIntegrity(getIntegrity())
         .setStripPrefix(getStripPrefix())

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ArchiveRepoSpecBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ArchiveRepoSpecBuilder.java
@@ -35,12 +35,6 @@ public ArchiveRepoSpecBuilder() {
     attrBuilder = new ImmutableMap.Builder<>();
   }
 
-  @CanIgnoreReturnValue
-  public ArchiveRepoSpecBuilder setRepoName(String repoName) {
-    attrBuilder.put("name", repoName);
-    return this;
-  }
-
   @CanIgnoreReturnValue
   public ArchiveRepoSpecBuilder setUrls(ImmutableList<String> urls) {
     attrBuilder.put("urls", urls);

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GitOverride.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GitOverride.java
@@ -18,7 +18,6 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule.ResolutionReason;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /** Specifies that a module should be retrieved from a Git repository. */
 @AutoValue
@@ -54,17 +53,15 @@ public static GitOverride create(
 
   /** Returns the {@link RepoSpec} that defines this repository. */
   @Override
-  public RepoSpec getRepoSpec(RepositoryName repoName) {
-    GitRepoSpecBuilder builder = new GitRepoSpecBuilder();
-    builder
-        .setRepoName(repoName.getName())
+  public RepoSpec getRepoSpec() {
+    return new GitRepoSpecBuilder()
         .setRemote(getRemote())
         .setCommit(getCommit())
         .setPatches(getPatches())
         .setPatchCmds(getPatchCmds())
         .setPatchArgs(ImmutableList.of("-p" + getPatchStrip()))
-        .setInitSubmodules(getInitSubmodules());
-    return builder.build();
+        .setInitSubmodules(getInitSubmodules())
+        .build();
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GitRepoSpecBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GitRepoSpecBuilder.java
@@ -33,11 +33,6 @@ public GitRepoSpecBuilder() {
     attrBuilder = new ImmutableMap.Builder<>();
   }
 
-  @CanIgnoreReturnValue
-  public GitRepoSpecBuilder setRepoName(String repoName) {
-    return setAttr("name", repoName);
-  }
-
   @CanIgnoreReturnValue
   public GitRepoSpecBuilder setRemote(String remoteRepoUrl) {
     return setAttr("remote", remoteRepoUrl);
@@ -80,11 +75,6 @@ public GitRepoSpecBuilder setPatches(List<String> patches) {
     return setAttr("patches", patches);
   }
 
-  @CanIgnoreReturnValue
-  public GitRepoSpecBuilder setPatchTool(String patchTool) {
-    return setAttr("patch_tool", patchTool);
-  }
-
   @CanIgnoreReturnValue
   public GitRepoSpecBuilder setPatchArgs(List<String> patchArgs) {
     return setAttr("patch_args", patchArgs);

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java
@@ -22,7 +22,6 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
@@ -187,8 +186,7 @@ private <T> T parseJson(String jsonString, String url, Class<T> klass) throws IO
   }
 
   @Override
-  public RepoSpec getRepoSpec(
-      ModuleKey key, RepositoryName repoName, ExtendedEventHandler eventHandler)
+  public RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler)
       throws IOException, InterruptedException {
     String jsonUrl =
         constructUrl(
@@ -208,22 +206,19 @@ public RepoSpec getRepoSpec(
         {
           ArchiveSourceJson typedSourceJson =
               parseJson(jsonString.get(), jsonUrl, ArchiveSourceJson.class);
-          return createArchiveRepoSpec(
-              typedSourceJson, getBazelRegistryJson(eventHandler), key, repoName);
+          return createArchiveRepoSpec(typedSourceJson, getBazelRegistryJson(eventHandler), key);
         }
       case "local_path":
         {
           LocalPathSourceJson typedSourceJson =
               parseJson(jsonString.get(), jsonUrl, LocalPathSourceJson.class);
-          return createLocalPathRepoSpec(
-              typedSourceJson, getBazelRegistryJson(eventHandler), key, repoName);
+          return createLocalPathRepoSpec(typedSourceJson, getBazelRegistryJson(eventHandler), key);
         }
       case "git_repository":
         {
           GitRepoSourceJson typedSourceJson =
               parseJson(jsonString.get(), jsonUrl, GitRepoSourceJson.class);
-          return createGitRepoSpec(
-              typedSourceJson, getBazelRegistryJson(eventHandler), key, repoName);
+          return createGitRepoSpec(typedSourceJson);
         }
       default:
         throw new IOException(
@@ -249,10 +244,7 @@ private Optional<BazelRegistryJson> getBazelRegistryJson(ExtendedEventHandler ev
   }
 
   private RepoSpec createLocalPathRepoSpec(
-      LocalPathSourceJson sourceJson,
-      Optional<BazelRegistryJson> bazelRegistryJson,
-      ModuleKey key,
-      RepositoryName repoName)
+      LocalPathSourceJson sourceJson, Optional<BazelRegistryJson> bazelRegistryJson, ModuleKey key)
       throws IOException {
     String path = sourceJson.path;
     if (!PathFragment.isAbsolute(path)) {
@@ -278,17 +270,12 @@ private RepoSpec createLocalPathRepoSpec(
     return RepoSpec.builder()
         .setRuleClassName("local_repository")
         .setAttributes(
-            AttributeValues.create(
-                ImmutableMap.of(
-                    "name", repoName.getName(), "path", PathFragment.create(path).toString())))
+            AttributeValues.create(ImmutableMap.of("path", PathFragment.create(path).toString())))
         .build();
   }
 
   private RepoSpec createArchiveRepoSpec(
-      ArchiveSourceJson sourceJson,
-      Optional<BazelRegistryJson> bazelRegistryJson,
-      ModuleKey key,
-      RepositoryName repoName)
+      ArchiveSourceJson sourceJson, Optional<BazelRegistryJson> bazelRegistryJson, ModuleKey key)
       throws IOException {
     URL sourceUrl = sourceJson.url;
     if (sourceUrl == null) {
@@ -332,7 +319,6 @@ private RepoSpec createArchiveRepoSpec(
     }
 
     return new ArchiveRepoSpecBuilder()
-        .setRepoName(repoName.getName())
         .setUrls(urls.build())
         .setIntegrity(sourceJson.integrity)
         .setStripPrefix(Strings.nullToEmpty(sourceJson.stripPrefix))
@@ -342,14 +328,8 @@ private RepoSpec createArchiveRepoSpec(
         .build();
   }
 
-  private RepoSpec createGitRepoSpec(
-      GitRepoSourceJson sourceJson,
-      Optional<BazelRegistryJson> bazelRegistryJson,
-      ModuleKey key,
-      RepositoryName repoName)
-      throws IOException {
+  private RepoSpec createGitRepoSpec(GitRepoSourceJson sourceJson) {
     return new GitRepoSpecBuilder()
-        .setRepoName(repoName.getName())
         .setRemote(sourceJson.remote)
         .setCommit(sourceJson.commit)
         .setShallowSince(sourceJson.shallowSince)

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LocalPathOverride.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LocalPathOverride.java
@@ -18,7 +18,6 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule.ResolutionReason;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /** Specifies that a module should be retrieved from a local directory. */
 @AutoValue
@@ -32,11 +31,10 @@ public static LocalPathOverride create(String path) {
 
   /** Returns the {@link RepoSpec} that defines this repository. */
   @Override
-  public RepoSpec getRepoSpec(RepositoryName repoName) {
+  public RepoSpec getRepoSpec() {
     return RepoSpec.builder()
         .setRuleClassName("local_repository")
-        .setAttributes(
-            AttributeValues.create(ImmutableMap.of("name", repoName.getName(), "path", getPath())))
+        .setAttributes(AttributeValues.create(ImmutableMap.of("path", getPath())))
         .build();
   }
 

diff --git a/.../com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java b/.../com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java
@@ -111,7 +111,9 @@ public void createRepo(StarlarkThread thread, Dict<String, Object> kwargs, RuleC
               ruleClass,
               Maps.transformEntries(kwargs, (k, v) -> k.equals("name") ? prefixedName : v));
 
-      Map<String, Object> attributes = Maps.transformEntries(kwargs, (k, v) -> rule.getAttr(k));
+      Map<String, Object> attributes =
+          Maps.filterKeys(
+              Maps.transformEntries(kwargs, (k, v) -> rule.getAttr(k)), k -> !k.equals("name"));
       String bzlFile = ruleClass.getRuleDefinitionEnvironmentLabel().getUnambiguousCanonicalForm();
       RepoSpec repoSpec =
           RepoSpec.builder()

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java
@@ -37,6 +37,7 @@ public abstract class ModuleKey {
    * normal format seen in {@link #getCanonicalRepoName()}) due to backwards compatibility reasons.
    * For example, bazel_tools must be known as "@bazel_tools" for WORKSPACE repos to work correctly.
    */
+  // Keep in sync with src/tools/bzlmod/utils.bzl.
   private static final ImmutableMap<String, RepositoryName> WELL_KNOWN_MODULES =
       ImmutableMap.of(
           "bazel_tools",

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/NonRegistryOverride.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/NonRegistryOverride.java
@@ -16,7 +16,6 @@
 package com.google.devtools.build.lib.bazel.bzlmod;
 
 import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule.ResolutionReason;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /**
  * An override specifying that the module should not be retrieved from a registry or participate in
@@ -27,7 +26,7 @@
 public interface NonRegistryOverride extends ModuleOverride {
 
   /** Returns the {@link RepoSpec} that defines this repository. */
-  RepoSpec getRepoSpec(RepositoryName repoName);
+  RepoSpec getRepoSpec();
 
   /**
    * Return the exact {@link

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java
@@ -16,7 +16,6 @@
 package com.google.devtools.build.lib.bazel.bzlmod;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import java.io.IOException;
 import java.util.Optional;
@@ -36,9 +35,9 @@ Optional<ModuleFile> getModuleFile(ModuleKey key, ExtendedEventHandler eventHand
 
   /**
    * Retrieves the {@link RepoSpec} object that indicates how the contents of the module identified
-   * by {@code key} should be materialized as a repo (with name {@code repoName}).
+   * by {@code key} should be materialized as a repo.
    */
-  RepoSpec getRepoSpec(ModuleKey key, RepositoryName repoName, ExtendedEventHandler eventHandler)
+  RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler)
       throws IOException, InterruptedException;
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RepoSpec.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RepoSpec.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.bazel.bzlmod;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.collect.Maps;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
 import javax.annotation.Nullable;
@@ -35,6 +36,11 @@ public abstract class RepoSpec implements SkyValue {
 
   public abstract String ruleClassName();
 
+  /**
+   * All attribute values provided to the repository rule, except for the <code>name</code>
+   * attribute, which is set in {@link
+   * com.google.devtools.build.lib.skyframe.BzlmodRepoRuleFunction}.
+   */
   public abstract AttributeValues attributes();
 
   public static Builder builder() {
@@ -50,7 +56,22 @@ public abstract static class Builder {
 
     public abstract Builder setAttributes(AttributeValues attributes);
 
-    public abstract RepoSpec build();
+    abstract AttributeValues attributes();
+
+    abstract RepoSpec autoBuild();
+
+    public final RepoSpec build() {
+      // Ensure backwards compatibility with old lockfiles that still specify the 'name' attribute.
+      // TODO: Remove this after both the lockfile version has been bumped and Bazel is built with
+      //  with Bazel 7.1.0.
+      AttributeValues attributes = attributes();
+      if (attributes.attributes().containsKey("name")) {
+        setAttributes(
+            AttributeValues.create(
+                Maps.filterKeys(attributes.attributes(), k -> !k.equals("name"))));
+      }
+      return autoBuild();
+    }
   }
 
   public boolean isNativeRepoRule() {

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RepoSpecFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RepoSpecFunction.java
@@ -48,8 +48,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
             .profile(ProfilerTask.BZLMOD, () -> "compute repo spec: " + key.getModuleKey())) {
       return registryFactory
           .getRegistryWithUrl(key.getRegistryUrl())
-          .getRepoSpec(
-              key.getModuleKey(), key.getModuleKey().getCanonicalRepoName(), env.getListener());
+          .getRepoSpec(key.getModuleKey(), env.getListener());
     } catch (IOException e) {
       throw new RepoSpecException(
           ExternalDepsException.withCauseAndMessage(

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
@@ -776,7 +776,9 @@ public RunModuleExtensionResult run(
                 .setRuleClassName(repoRule.getRuleClass().getName())
                 .setAttributes(
                     AttributeValues.create(
-                        Maps.transformEntries(kwargs, (k, v) -> ruleInstance.getAttr(k))))
+                        Maps.filterKeys(
+                            Maps.transformEntries(kwargs, (k, v) -> ruleInstance.getAttr(k)),
+                            k -> !k.equals("name"))))
                 .build();
         generatedRepoSpecs.put(name, repoSpec);
       }