fix(bzlmod)!: Remove ability to specify toolchain repo name. (#1258)

The main reasons this is removed is because if modules choose different
names for the same toolchain, only one of the two toolchains (which are,
hopefully, identical) will be used. Which toolchain is used depends on
the module graph dependency ordering.

Furthermore, as of #1238, only one repo per version is created; others
are ignored. This means a module doesn't know if the name it chooses
will result in a repo being created with that name.

Instead, the toolchain repos are named by rules_python:
`python_{major}_{minor}`. These repo names are currently part of the
public API, since they end up referenced in MODULE config (to wire the
toolchain interpreter to pip).

BREAKING CHANGES

This removes the `name` arg from `python.toolchain()`. Users will need
to remove such usages from their `MODULE.bazel` and update their
`use_repo()` statements. If keeping the custom repo name is necessary,
then repo mappings can be used. See #1232 for additional migration
steps, commands, and information.

9 files changed, 27 insertions, 74 deletions

diff --git a/.bazelignore b/.bazelignore
index a603a7b..135f709 100644
--- a/.bazelignore
+++ b/.bazelignore
@@ -6,3 +6,6 @@ bazel-rules_python
 bazel-bin
 bazel-out
 bazel-testlogs
+examples/bzlmod/bazel-bzlmod
+examples/bzlmod_build_file_generation/bazel-bzlmod_build_file_generation
+examples/py_proto_library/bazel-py_proto_library
diff --git a/examples/bzlmod/BUILD.bazel b/examples/bzlmod/BUILD.bazel
index 0a068ce..3bff208 100644
--- a/examples/bzlmod/BUILD.bazel
+++ b/examples/bzlmod/BUILD.bazel
@@ -7,7 +7,7 @@
 # names. Those names are defined in the MODULES.bazel file.
 load("@bazel_skylib//rules:build_test.bzl", "build_test")
 load("@pip//:requirements.bzl", "all_requirements", "all_whl_requirements", "requirement")
-load("@python_39//:defs.bzl", py_test_with_transition = "py_test")
+load("@python_3_9//:defs.bzl", py_test_with_transition = "py_test")
 load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test")
 load("@rules_python//python:pip.bzl", "compile_pip_requirements")
 
diff --git a/examples/bzlmod/MODULE.bazel b/examples/bzlmod/MODULE.bazel
index 24bb458..40dfb6a 100644
--- a/examples/bzlmod/MODULE.bazel
+++ b/examples/bzlmod/MODULE.bazel
@@ -11,13 +11,13 @@ local_path_override(
     path = "../..",
 )
 
-# This name is passed into python.toolchain and it's use_repo statement.
-# We also use the same value in the python.host_python_interpreter call.
-PYTHON_NAME_39 = "python_39"
+# This name is generated by python.toolchain(), and is later passed
+# to use_repo() and interpreter.install().
+PYTHON_NAME_39 = "python_3_9"
 
 INTERPRETER_NAME_39 = "interpreter_39"
 
-PYTHON_NAME_310 = "python_310"
+PYTHON_NAME_310 = "python_3_10"
 
 INTERPRETER_NAME_310 = "interpreter_310"
 
@@ -25,10 +25,6 @@ INTERPRETER_NAME_310 = "interpreter_310"
 # You can set different Python versions in this block.
 python = use_extension("@rules_python//python/extensions:python.bzl", "python")
 python.toolchain(
-    # This name is used in the various use_repo statements
-    # below, and in the local extension that is in this
-    # example.
-    name = PYTHON_NAME_39,
     configure_coverage_tool = True,
     # Only set when you have mulitple toolchain versions.
     is_default = True,
@@ -41,7 +37,6 @@ python.toolchain(
 # Note: we do not supporting using multiple pip extensions, this is
 # work in progress.
 python.toolchain(
-    name = PYTHON_NAME_310,
     configure_coverage_tool = True,
     python_version = "3.10",
 )
diff --git a/examples/bzlmod/other_module/MODULE.bazel b/examples/bzlmod/other_module/MODULE.bazel
index 5fb7452..cc23a51 100644
--- a/examples/bzlmod/other_module/MODULE.bazel
+++ b/examples/bzlmod/other_module/MODULE.bazel
@@ -10,24 +10,16 @@ bazel_dep(name = "rules_python", version = "")
 # a submodule.  This code only exists to test that
 # we support doing this. This code is only for rules_python
 # testing purposes.
-PYTHON_NAME_39 = "python_39"
+PYTHON_NAME_39 = "python_3_9"
 
-PYTHON_NAME_311 = "python_311"
+PYTHON_NAME_311 = "python_3_11"
 
 python = use_extension("@rules_python//python/extensions:python.bzl", "python")
 python.toolchain(
-    # This name is used in the various use_repo statements
-    # below, and in the local extension that is in this
-    # example.
-    name = PYTHON_NAME_39,
     configure_coverage_tool = True,
     python_version = "3.9",
 )
 python.toolchain(
-    # This name is used in the various use_repo statements
-    # below, and in the local extension that is in this
-    # example.
-    name = PYTHON_NAME_311,
     configure_coverage_tool = True,
     # In a submodule this is ignored
     is_default = True,
diff --git a/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel b/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel
index 952a674..6e37df8 100644
--- a/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel
+++ b/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel
@@ -1,4 +1,4 @@
-load("@python_311//:defs.bzl", py_binary_311 = "py_binary")
+load("@python_3_11//:defs.bzl", py_binary_311 = "py_binary")
 load("@rules_python//python:defs.bzl", "py_library")
 
 py_library(
diff --git a/examples/bzlmod_build_file_generation/MODULE.bazel b/examples/bzlmod_build_file_generation/MODULE.bazel
index d69dd7d..fab2a26 100644
--- a/examples/bzlmod_build_file_generation/MODULE.bazel
+++ b/examples/bzlmod_build_file_generation/MODULE.bazel
@@ -46,14 +46,13 @@ python = use_extension("@rules_python//python/extensions:python.bzl", "python")
 
 # This name is passed into python.toolchain and it's use_repo statement.
 # We also use the same name for python.host_python_interpreter.
-PYTHON_NAME = "python"
+PYTHON_NAME = "python_3_9"
 
 INTERPRETER_NAME = "interpreter"
 
 # We next initialize the python toolchain using the extension.
 # You can set different Python versions in this block.
 python.toolchain(
-    name = PYTHON_NAME,
     configure_coverage_tool = True,
     is_default = True,
     python_version = "3.9",
diff --git a/examples/py_proto_library/MODULE.bazel b/examples/py_proto_library/MODULE.bazel
index 3116c40..feb938d 100644
--- a/examples/py_proto_library/MODULE.bazel
+++ b/examples/py_proto_library/MODULE.bazel
@@ -14,11 +14,10 @@ local_path_override(
 
 python = use_extension("@rules_python//python/extensions:python.bzl", "python")
 python.toolchain(
-    name = "python3_9",
     configure_coverage_tool = True,
     python_version = "3.9",
 )
-use_repo(python, "python3_9")
+use_repo(python, "python_3_9")
 
 # We are using rules_proto to define rules_proto targets to be consumed by py_proto_library.
 bazel_dep(name = "rules_proto", version = "5.3.0-21.7")
diff --git a/python/extensions/interpreter.bzl b/python/extensions/interpreter.bzl
index b9afe1a..bbeadc2 100644
--- a/python/extensions/interpreter.bzl
+++ b/python/extensions/interpreter.bzl
@@ -53,7 +53,7 @@ def _interpreter_repo_impl(rctx):
 
     actual_interpreter_label = INTERPRETER_LABELS.get(rctx.attr.python_name)
     if actual_interpreter_label == None:
-        fail("Unable to find interpreter with name {}".format(rctx.attr.python_name))
+        fail("Unable to find interpreter with name '{}'".format(rctx.attr.python_name))
 
     rctx.symlink(actual_interpreter_label, "python")
 
diff --git a/python/extensions/python.bzl b/python/extensions/python.bzl
index a604df6..bed6230 100644
--- a/python/extensions/python.bzl
+++ b/python/extensions/python.bzl
@@ -43,11 +43,11 @@ def _left_pad_zero(index, length):
 def _print_warn(msg):
     print("WARNING:", msg)
 
-def _python_register_toolchains(toolchain_attr, version_constraint):
+def _python_register_toolchains(name, toolchain_attr, version_constraint):
     """Calls python_register_toolchains and returns a struct used to collect the toolchains.
     """
     python_register_toolchains(
-        name = toolchain_attr.name,
+        name = name,
         python_version = toolchain_attr.python_version,
         register_coverage_tool = toolchain_attr.configure_coverage_tool,
         ignore_root_user_error = toolchain_attr.ignore_root_user_error,
@@ -56,7 +56,7 @@ def _python_register_toolchains(toolchain_attr, version_constraint):
     return struct(
         python_version = toolchain_attr.python_version,
         set_python_version_constraint = str(version_constraint),
-        name = toolchain_attr.name,
+        name = name,
     )
 
 def _python_impl(module_ctx):
@@ -67,38 +67,17 @@ def _python_impl(module_ctx):
     # toolchain added to toolchains.
     default_toolchain = None
 
-    # Map of toolchain name to registering module
-    global_toolchain_names = {}
-
     # Map of string Major.Minor to the toolchain name and module name
     global_toolchain_versions = {}
 
     for mod in module_ctx.modules:
-        module_toolchain_names = []
         module_toolchain_versions = []
 
         for toolchain_attr in mod.tags.toolchain:
-            toolchain_name = toolchain_attr.name
-
-            # Duplicate names within a module indicate a misconfigured module.
-            if toolchain_name in module_toolchain_names:
-                _fail_duplicate_module_toolchain_name(mod.name, toolchain_name)
-            module_toolchain_names.append(toolchain_name)
-
-            # Ignore name collisions in the global scope because there isn't
-            # much else that can be done. Modules don't know and can't control
-            # what other modules do, so the first in the dependency graph wins.
-            if toolchain_name in global_toolchain_names:
-                _warn_duplicate_global_toolchain_name(
-                    toolchain_name,
-                    first_module = global_toolchain_names[toolchain_name],
-                    second_module = mod.name,
-                )
-                continue
-            global_toolchain_names[toolchain_name] = mod.name
+            toolchain_version = toolchain_attr.python_version
+            toolchain_name = "python_" + toolchain_version.replace(".", "_")
 
             # Duplicate versions within a module indicate a misconfigured module.
-            toolchain_version = toolchain_attr.python_version
             if toolchain_version in module_toolchain_versions:
                 _fail_duplicate_module_toolchain_version(toolchain_version, mod.name)
             module_toolchain_versions.append(toolchain_version)
@@ -137,6 +116,7 @@ def _python_impl(module_ctx):
                 )
 
             toolchain_info = _python_register_toolchains(
+                toolchain_name,
                 toolchain_attr,
                 version_constraint = not is_default,
             )
@@ -182,23 +162,6 @@ def _python_impl(module_ctx):
         },
     )
 
-def _fail_duplicate_module_toolchain_name(module_name, toolchain_name):
-    fail(("Duplicate module toolchain name: module '{module}' attempted " +
-          "to use the name '{toolchain}' multiple times in itself").format(
-        toolchain = toolchain_name,
-        module = module_name,
-    ))
-
-def _warn_duplicate_global_toolchain_name(name, first_module, second_module):
-    _print_warn((
-        "Ignoring toolchain '{name}' from module '{second_module}': " +
-        "Toolchain with the same name from module '{first_module}' has precedence"
-    ).format(
-        name = name,
-        first_module = first_module,
-        second_module = second_module,
-    ))
-
 def _fail_duplicate_module_toolchain_version(version, module):
     fail(("Duplicate module toolchain version: module '{module}' attempted " +
           "to use version '{version}' multiple times in itself").format(
@@ -256,6 +219,12 @@ if the sub module toolchain is marked as the default version. If you have
 more than one toolchain in your root module, you need to set one of the
 toolchains as the default version.  If there is only one toolchain it
 is set as the default toolchain.
+
+Toolchain repository name
+
+A toolchain's repository name uses the format `python_{major}_{minor}`, e.g.
+`python_3_10`. The `major` and `minor` components are
+`major` and `minor` are the Python version from the `python_version` attribute.
 """,
             attrs = {
                 "configure_coverage_tool": attr.bool(
@@ -271,13 +240,9 @@ is set as the default toolchain.
                     mandatory = False,
                     doc = "Whether the toolchain is the default version",
                 ),
-                "name": attr.string(
-                    mandatory = True,
-                    doc = "Name of the toolchain",
-                ),
                 "python_version": attr.string(
                     mandatory = True,
-                    doc = "The Python version that we are creating the toolchain for.",
+                    doc = "The Python version, in `major.minor` format, e.g '3.12', to create a toolchain for.",
                 ),
             },
         ),