Omit unique module versions from canonical repo names

The canonical repository name of a module `rules_foo` is now `rules_foo~` instead of e.g. `rules_foo~1.2.3` if there is only a single version of the module in the entire dep graph. This also applies to overrides, which previously used the canonical repository name `rules_foo~override`.

This improves cacheability of actions across module version changes and also prevents the output base from filling up with outdated module and extension repositories. See the long comment in `ModuleKey#getCanonicalRepoName` for a detailed explanation of why this particular scheme was chosen.

The change includes a bump of the lockfile version as the canonical repo names of label attributes in extension usages are affected.

RELNOTES: The scheme for generating canonical repository names has changed to improve cacheability of actions across dependency version updates. Note that canonical names are not considered to be public API and can change at any time. See https://bazel.build/external/module#repository_names_and_strict_deps for advice on how to avoid hardcoding canonical repository names.

Fixes #20997

Closes #21035.

PiperOrigin-RevId: 605730371
Change-Id: Ica1be1ba5493d3636248a79a6549a0927021bef9

BUILD

@@ -168,7 +168,9 @@ pkg_tar(
 write_file(
     name = "gen_maven_repo_name",
     out = "MAVEN_CANONICAL_REPO_NAME",
-    content = [get_canonical_repo_name("@maven")],
+    # TODO: Use this instead after building with Bazel 7.1.0 or later.
+    #    content = [get_canonical_repo_name("@maven")],
+    content = ["rules_jvm_external~~maven~maven"],
 )
 
 # The @maven repository is created by maven_install from rules_jvm_external.

scripts/bootstrap/compile.sh

@@ -254,6 +254,10 @@ EOF
   link_file "${PWD}/src/MODULE.tools" "${BAZEL_TOOLS_REPO}/MODULE.bazel"
   new_hash=$(shasum -a 256 "${BAZEL_TOOLS_REPO}/MODULE.bazel" | awk '{print $1}')
   sed -i.bak "/\"bazel_tools\":/s/\"[a-f0-9]*\"/\"$new_hash\"/" MODULE.bazel.lock
+  # TODO: Temporary hack for lockfile version mismatch, remove these lines after updating to 7.1.0
+  sed -i.bak 's/"lockFileVersion": 3/"lockFileVersion": 4/' MODULE.bazel.lock
+  # Replace canonical repository names and parts thereof of the form rules_foo~1.2.3 with rules_foo~
+  sed -i.bak -E 's/([a-z]([a-z0-9._-]*[a-z0-9]){0,1})~[a-zA-Z0-9.]{1,}(-[0-9.-]{1,}){0,1}(\+[0-9.-]{1,}){0,1}/\1/g' MODULE.bazel.lock
   rm MODULE.bazel.lock.bak
 
   mkdir -p "${BAZEL_TOOLS_REPO}/src/conditions"

site/en/external/mod-command.md

@@ -395,7 +395,7 @@ use_repo(toolchains, my_jdk="remotejdk17_linux")
     ## rules_cc@0.0.1:
     # <builtin>
     http_archive(
-      name = "rules_cc~0.0.1",
+      name = "rules_cc~",
       urls = ["https://bcr.bazel.build/test-mirror/github.com/bazelbuild/rules_cc/releases/download/0.0.1/rules_cc-0.0.1.tar.gz", "https://github.com/bazelbuild/rules_cc/releases/download/0.0.1/rules_cc-0.0.1.tar.gz"],
       integrity = "sha256-Tcy/0iwN7xZMj0dFi9UODHFI89kgAs20WcKpamhJgkE=",
       strip_prefix = "",
@@ -408,7 +408,7 @@ use_repo(toolchains, my_jdk="remotejdk17_linux")
     ## stardoc:
     # <builtin>
     http_archive(
-      name = "stardoc~0.5.0",
+      name = "stardoc~",
       urls = ["https://bcr.bazel.build/test-mirror/github.com/bazelbuild/stardoc/releases/download/0.5.0/stardoc-0.5.0.tar.gz", "https://github.com/bazelbuild/stardoc/releases/download/0.5.0/stardoc-0.5.0.tar.gz"],
       integrity = "sha256-yXlNzIAmow/2fPfPkeviRcopSyCwcYRdEsGSr+JDrXI=",
       strip_prefix = "",
@@ -538,7 +538,7 @@ use_repo(toolchains, my_jdk="remotejdk17_linux")
     ## @remote_java_tools:
     # <builtin>
     http_archive(
-      name = "rules_java~5.0.0~toolchains~remote_java_tools",
+      name = "rules_java~~toolchains~remote_java_tools",
       urls = ["https://mirror.bazel.build/bazel_java_tools/releases/java/v11.5/java_tools-v11.5.zip", "https://github.com/bazelbuild/java_tools/releases/download/java_v11.5/java_tools-v11.5.zip"],
       sha256 = "b763ee80e5754e593fd6d5be6d7343f905bc8b73d661d36d842b024ca11b6793",
     )

site/en/external/module.md

@@ -190,19 +190,34 @@ flexibility.
 
 ## Repository names and strict deps
 
-The [canonical name](/external/overview#canonical-repo-name) of a repo backing a
-module is `{{ "<var>" }}module_name{{ "</var>" }}~{{ "<var>" }}version{{
-"</var>" }}` (for example, `bazel_skylib~1.0.3`). For modules with a
-non-registry override, replace the `{{ "<var>" }}version{{ "</var>" }}` part
-with the string `override`. Note that the canonical name format is not an API
-you should depend on and is subject to change at any time.
-
 The [apparent name](/external/overview#apparent-repo-name) of a repo backing a
 module to its direct dependents defaults to its module name, unless the
 `repo_name` attribute of the [`bazel_dep`](/rules/lib/globals/module#bazel_dep)
 directive says otherwise. Note that this means a module can only find its direct
 dependencies. This helps prevent accidental breakages due to changes in
 transitive dependencies.
 
+The [canonical name](/external/overview#canonical-repo-name) of a repo backing a
+module is either `{{ "<var>" }}module_name{{ "</var>" }}~{{ "<var>" }}version{{
+"</var>" }}` (for example, `bazel_skylib~1.0.3`) or `{{ "<var>" }}module_name{{
+"</var>" }}~` (for example, `bazel_features~`), depending on whether there are
+multiple versions of the module in the entire dependency graph (see
+[`multiple_version_override`](/rules/lib/globals/module#multiple_version_override)).
+Note that **the canonical name format** is not an API you should depend on and
+**is subject to change at any time**. Instead of hard-coding the canonical name,
+use a supported way to get it directly from Bazel:
+* In BUILD and `.bzl` files, use
+  [`Label.repo_name`](/rules/lib/builtins/Label#repo_name) on a `Label` instance
+  constructed from a label string given by the apparent name of the repo, e.g.,
+  `Label("@bazel_skylib").repo_name`.
+* When looking up runfiles, use
+  [`$(rlocationpath ...)`](https://bazel.build/reference/be/make-variables#predefined_label_variables)
+  or one of the runfiles libraries in
+  `@bazel_tools//tools/{bash,cpp,java}/runfiles` or, for a ruleset `rules_foo`,
+  in `@rules_foo//foo/runfiles`.
+* When interacting with Bazel from an external tool such as an IDE or language
+  server, use the `bazel mod dump_repo_mapping` command to get the mapping from
+  apparent names to canonical names for a given set of repositories.
+
 [Module extensions](/external/extension) can also introduce additional repos
 into the visible scope of a module.

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

@@ -16,7 +16,6 @@
 package com.google.devtools.build.lib.bazel.bzlmod;
 
 import com.google.auto.value.AutoValue;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /**
  * An abridged version of a {@link Module}, with a reduced set of information available, used for
@@ -30,10 +29,6 @@ public abstract class AbridgedModule {
 
   public abstract ModuleKey getKey();
 
-  public final RepositoryName getCanonicalRepoName() {
-    return getKey().getCanonicalRepoName();
-  }
-
   public static AbridgedModule from(Module module) {
     return new AutoValue_AbridgedModule(module.getName(), module.getVersion(), module.getKey());
   }

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

@@ -325,6 +325,7 @@ java_library(
     deps = [
         ":common",
         ":module_extension",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",

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

@@ -16,8 +16,12 @@
 package com.google.devtools.build.lib.bazel.bzlmod;
 
 import static com.google.common.base.Strings.nullToEmpty;
+import static com.google.common.collect.ImmutableBiMap.toImmutableBiMap;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static java.util.stream.Collectors.counting;
+import static java.util.stream.Collectors.groupingBy;
+import static java.util.stream.Collectors.toSet;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
@@ -31,6 +35,7 @@
 import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.packages.LabelConverter;
 import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
@@ -44,6 +49,7 @@
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
@@ -113,13 +119,11 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       depGraph = selectionResult.getResolvedDepGraph();
     }
 
-    ImmutableMap<RepositoryName, ModuleKey> canonicalRepoNameLookup =
-        depGraph.keySet().stream()
-            .collect(toImmutableMap(ModuleKey::getCanonicalRepoName, key -> key));
-
+    ImmutableBiMap<RepositoryName, ModuleKey> canonicalRepoNameLookup =
+        computeCanonicalRepoNameLookup(depGraph);
     ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> extensionUsagesById;
     try {
-      extensionUsagesById = getExtensionUsagesById(depGraph);
+      extensionUsagesById = getExtensionUsagesById(depGraph, canonicalRepoNameLookup.inverse());
     } catch (ExternalDepsException e) {
       throw new BazelDepGraphFunctionException(e, Transience.PERSISTENT);
     }
@@ -214,13 +218,23 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
   public static ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
       getExtensionUsagesById(ImmutableMap<ModuleKey, Module> depGraph)
           throws ExternalDepsException {
+    return getExtensionUsagesById(depGraph, computeCanonicalRepoNameLookup(depGraph).inverse());
+  }
+
+  private static ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
+      getExtensionUsagesById(
+          ImmutableMap<ModuleKey, Module> depGraph,
+          ImmutableMap<ModuleKey, RepositoryName> moduleKeyToRepositoryNames)
+          throws ExternalDepsException {
     ImmutableTable.Builder<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
         extensionUsagesTableBuilder = ImmutableTable.builder();
     for (Module module : depGraph.values()) {
+      RepositoryMapping repoMapping =
+          module.getRepoMappingWithBazelDepsOnly(moduleKeyToRepositoryNames);
       LabelConverter labelConverter =
           new LabelConverter(
-              PackageIdentifier.create(module.getCanonicalRepoName(), PathFragment.EMPTY_FRAGMENT),
-              module.getRepoMappingWithBazelDepsOnly());
+              PackageIdentifier.create(repoMapping.ownerRepo(), PathFragment.EMPTY_FRAGMENT),
+              module.getRepoMappingWithBazelDepsOnly(moduleKeyToRepositoryNames));
       for (ModuleExtensionUsage usage : module.getExtensionUsages()) {
         ModuleExtensionId moduleExtensionId;
         try {
@@ -249,6 +263,37 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
     return extensionUsagesTableBuilder.buildOrThrow();
   }
 
+  private static ImmutableBiMap<RepositoryName, ModuleKey> computeCanonicalRepoNameLookup(
+      ImmutableMap<ModuleKey, Module> depGraph) {
+    // Find modules with multiple versions in the dep graph. Currently, the only source of such
+    // modules is multiple_version_override.
+    Set<String> multipleVersionsModules =
+        depGraph.keySet().stream()
+            .collect(groupingBy(ModuleKey::getName, counting()))
+            .entrySet()
+            .stream()
+            .filter(entry -> entry.getValue() > 1)
+            .map(Entry::getKey)
+            .collect(toSet());
+
+    // If there is a unique version of this module in the entire dep graph, we elide the version
+    // from the canonical repository name. This has a number of benefits:
+    // * It prevents the output base from being polluted with repository directories corresponding
+    //   to outdated versions of modules, which can be large and would otherwise only be cleaned
+    //   up by the discouraged bazel clean --expunge.
+    // * It improves cache hit rates by ensuring that a module update doesn't e.g. cause the paths
+    //   of all toolchains provided by its extensions to change, which would result in widespread
+    //   cache misses on every update.
+    return depGraph.keySet().stream()
+        .collect(
+            toImmutableBiMap(
+                key ->
+                    multipleVersionsModules.contains(key.getName())
+                        ? key.getCanonicalRepoNameWithVersion()
+                        : key.getCanonicalRepoNameWithoutVersion(),
+                key -> key));
+  }
+
   private ImmutableBiMap<String, ModuleExtensionId> calculateUniqueNameForUsedExtensionId(
       ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> extensionUsagesById) {
     // Calculate a unique name for each used extension id with the following property that is

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

@@ -18,6 +18,7 @@
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableBiMap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableTable;
@@ -45,7 +46,7 @@ public static BazelDepGraphValue create(
       ImmutableMap<ModuleExtensionId, String> extensionUniqueNames) {
     return new AutoValue_BazelDepGraphValue(
         depGraph,
-        canonicalRepoNameLookup,
+        ImmutableBiMap.copyOf(canonicalRepoNameLookup),
         abridgedModules,
         extensionUsagesTable,
         extensionUniqueNames);
@@ -67,7 +68,12 @@ public static BazelDepGraphValue createEmptyDepGraph() {
 
     ImmutableMap<RepositoryName, ModuleKey> canonicalRepoNameLookup =
         emptyDepGraph.keySet().stream()
-            .collect(toImmutableMap(ModuleKey::getCanonicalRepoName, key -> key));
+            .collect(
+                toImmutableMap(
+                    // All modules in the empty dep graph (just the root module) have an empty
+                    // version, so the choice of including it in the canonical repo name does not
+                    // matter.
+                    ModuleKey::getCanonicalRepoNameWithoutVersion, key -> key));
 
     return BazelDepGraphValue.create(
         emptyDepGraph,
@@ -83,8 +89,8 @@ public static BazelDepGraphValue createEmptyDepGraph() {
    */
   public abstract ImmutableMap<ModuleKey, Module> getDepGraph();
 
-  /** A mapping from a canonical repo name to the key of the module backing it. */
-  public abstract ImmutableMap<RepositoryName, ModuleKey> getCanonicalRepoNameLookup();
+  /** A mapping from a canonical repo name to the key of the module backing it and back. */
+  public abstract ImmutableBiMap<RepositoryName, ModuleKey> getCanonicalRepoNameLookup();
 
   /** All modules in the same order as {@link #getDepGraph}, but with limited information. */
   public abstract ImmutableList<AbridgedModule> getAbridgedModules();
@@ -124,7 +130,7 @@ public final RepositoryMapping getFullRepoMapping(ModuleKey key) {
     }
     return getDepGraph()
         .get(key)
-        .getRepoMappingWithBazelDepsOnly()
+        .getRepoMappingWithBazelDepsOnly(getCanonicalRepoNameLookup().inverse())
         .withAdditionalMappings(mapping.buildOrThrow());
   }
 }

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

@@ -37,7 +37,7 @@
 @GenerateTypeAdapter
 public abstract class BazelLockFileValue implements SkyValue, Postable {
 
-  public static final int LOCK_FILE_VERSION = 3;
+  public static final int LOCK_FILE_VERSION = 4;
 
   @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE;
 

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

@@ -82,7 +82,11 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
                         AugmentedModule::getName,
                         Collectors.mapping(AugmentedModule::getKey, toImmutableSet()))));
 
-    return BazelModuleInspectorValue.create(depGraph, modulesIndex, extensionToRepoInternalNames);
+    return BazelModuleInspectorValue.create(
+        depGraph,
+        modulesIndex,
+        extensionToRepoInternalNames,
+        depGraphValue.getCanonicalRepoNameLookup().inverse());
   }
 
   public static ImmutableMap<ModuleKey, AugmentedModule> computeAugmentedGraph(

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

@@ -21,6 +21,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.ImmutableSortedMap;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -42,9 +43,10 @@ public abstract class BazelModuleInspectorValue implements SkyValue {
   public static BazelModuleInspectorValue create(
       ImmutableMap<ModuleKey, AugmentedModule> depGraph,
       ImmutableMap<String, ImmutableSet<ModuleKey>> modulesIndex,
-      ImmutableSetMultimap<ModuleExtensionId, String> extensionToRepoInternalNames) {
+      ImmutableSetMultimap<ModuleExtensionId, String> extensionToRepoInternalNames,
+      ImmutableMap<ModuleKey, RepositoryName> moduleKeyToCanonicalNames) {
     return new AutoValue_BazelModuleInspectorValue(
-        depGraph, modulesIndex, extensionToRepoInternalNames);
+        depGraph, modulesIndex, extensionToRepoInternalNames, moduleKeyToCanonicalNames);
   }
 
   /**
@@ -69,6 +71,9 @@ public static BazelModuleInspectorValue create(
    */
   public abstract ImmutableSetMultimap<ModuleExtensionId, String> getExtensionToRepoInternalNames();
 
+  /** A mapping from a module key to the canonical repository name of the module repository. */
+  public abstract ImmutableMap<ModuleKey, RepositoryName> getModuleKeyToCanonicalNames();
+
   /**
    * A wrapper for {@link Module}, augmented with references to dependants (and also those who are
    * not used in the final dep graph).

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

@@ -48,23 +48,25 @@ public abstract class Module extends ModuleBase {
    * Returns a {@link RepositoryMapping} with only Bazel module repos and no repos from module
    * extensions. For the full mapping, see {@link BazelDepGraphValue#getFullRepoMapping}.
    */
-  public final RepositoryMapping getRepoMappingWithBazelDepsOnly() {
+  public final RepositoryMapping getRepoMappingWithBazelDepsOnly(
+      ImmutableMap<ModuleKey, RepositoryName> moduleKeyToRepositoryNames) {
     ImmutableMap.Builder<String, RepositoryName> mapping = ImmutableMap.builder();
     // If this is the root module, then the main repository should be visible as `@`.
     if (getKey().equals(ModuleKey.ROOT)) {
       mapping.put("", RepositoryName.MAIN);
     }
     // Every module should be able to reference itself as @<module repo name>.
     // If this is the root module, this perfectly falls into @<module repo name> => @
+    RepositoryName owner = moduleKeyToRepositoryNames.get(getKey());
     if (!getRepoName().isEmpty()) {
-      mapping.put(getRepoName(), getCanonicalRepoName());
+      mapping.put(getRepoName(), owner);
     }
     for (Map.Entry<String, ModuleKey> dep : getDeps().entrySet()) {
       // Special note: if `dep` is actually the root module, its ModuleKey would be ROOT whose
       // canonicalRepoName is the empty string. This perfectly maps to the main repo ("@").
-      mapping.put(dep.getKey(), dep.getValue().getCanonicalRepoName());
+      mapping.put(dep.getKey(), moduleKeyToRepositoryNames.get(dep.getValue()));
     }
-    return RepositoryMapping.create(mapping.buildOrThrow(), getCanonicalRepoName());
+    return RepositoryMapping.create(mapping.buildOrThrow(), owner);
   }
 
   /**