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.

Author: rickeylev

.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

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")
 

examples/bzlmod/MODULE.bazel

@@ -11,24 +11,20 @@ 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"
 
 # We next initialize the python toolchain using the extension.
 # 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",
 )

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,

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(

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",

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")

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")
 

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.",
                 ),
             },
         ),