fix: prefer X.Y over X.Y.Z versions in python.toolchain

Before this PR the `bzlmod` and legacy setups would only expose the
multi-version python toolchain aliases as the string that is passed to
the `python_register_multi_toolchains` function. This meant that if the
user decided to pass the full version (e.g. `3.11.1`) then they had to
import the version aware `py_*` defs via `@foo//3.11.1:defs.bzl`. This
PR changes it such that the user can still do the old way of importing
but we print a deprecation warning and ask the user to use
`3.11:defs.bzl` instead which better reflects how the multi-version
toolchain should be used. This also means that the PRs bumping the minor
versions like in #1352 won't need updating Python code elsewhere.

Whilst at it, we introduce a validation that disallows multiple Python
toolchains of the same `X.Y` version. This is handled in the `bzlmod` by
printing warnings that the toolchains are ignored, whilst the non-bzlmod
users will get validation errors, which is a potentially breaking
change.

Fixes #1339

Author: aignas

WORKSPACE

@@ -72,7 +72,7 @@ _py_gazelle_deps()
 # Install twine for our own runfiles wheel publishing.
 # Eventually we might want to install twine automatically for users too, see:
 # https://github.com/bazelbuild/rules_python/issues/1016.
-load("@python//3.11.1:defs.bzl", "interpreter")
+load("@python//3.11:defs.bzl", "interpreter")
 load("@rules_python//python:pip.bzl", "pip_parse")
 
 pip_parse(

examples/bzlmod/MODULE.bazel

@@ -28,7 +28,8 @@ python.toolchain(
 # work in progress.
 python.toolchain(
     configure_coverage_tool = True,
-    python_version = "3.10",
+    # One can also provide the full Python version.
+    python_version = "3.10.9",
 )
 
 # You only need to load this repositories if you are using multiple Python versions.

examples/bzlmod/other_module/MODULE.bazel

@@ -27,7 +27,7 @@ PYTHON_NAME_311 = "python_3_11"
 python = use_extension("@rules_python//python/extensions:python.bzl", "python")
 python.toolchain(
     configure_coverage_tool = True,
-    python_version = "3.9",
+    python_version = "3.9.10",
 )
 python.toolchain(
     configure_coverage_tool = True,

examples/multi_python_versions/WORKSPACE

@@ -22,7 +22,7 @@ python_register_multi_toolchains(
         "3.8",
         "3.9",
         "3.10",
-        "3.11",
+        "3.11.1",
     ],
     register_coverage_tool = True,
 )

python/extensions/private/pythons_hub.bzl

@@ -14,7 +14,8 @@
 
 "Repo rule used by bzlmod extension to create a repo that has a map of Python interpreters and their labels"
 
-load("//python:versions.bzl", "MINOR_MAPPING", "WINDOWS_NAME")
+load("//python:versions.bzl", "WINDOWS_NAME")
+load("//python/private:full_version.bzl", "full_version")
 load(
     "//python/private:toolchains_repo.bzl",
     "get_host_os_arch",
@@ -28,12 +29,6 @@ def _have_same_length(*lists):
         fail("expected at least one list")
     return len({len(length): None for length in lists}) == 1
 
-def _get_version(python_version):
-    # we need to get the MINOR_MAPPING or use the full version
-    if python_version in MINOR_MAPPING:
-        python_version = MINOR_MAPPING[python_version]
-    return python_version
-
 def _python_toolchain_build_file_content(
         prefixes,
         python_versions,
@@ -55,7 +50,7 @@ def _python_toolchain_build_file_content(
     # build the toolchain content by calling python_toolchain_build_file_content
     return "\n".join([python_toolchain_build_file_content(
         prefix = prefixes[i],
-        python_version = _get_version(python_versions[i]),
+        python_version = full_version(python_versions[i]),
         set_python_version_constraint = set_python_version_constraints[i],
         user_repository_name = user_repository_names[i],
         rules_python = rules_python,

python/extensions/python.bzl

@@ -16,7 +16,9 @@
 
 load("//python:repositories.bzl", "python_register_toolchains")
 load("//python/extensions/private:pythons_hub.bzl", "hub_repo")
+load("//python/private:full_version.bzl", "full_version")
 load("//python/private:toolchains_repo.bzl", "multi_toolchain_aliases")
+load("//python/private:version_label.bzl", "version_label")
 
 # This limit can be increased essentially arbitrarily, but doing so will cause a rebuild of all
 # targets using any of these toolchains due to the changed repository name.
@@ -74,28 +76,30 @@ def _python_impl(module_ctx):
         module_toolchain_versions = []
 
         for toolchain_attr in mod.tags.toolchain:
-            toolchain_version = toolchain_attr.python_version
-            toolchain_name = "python_" + toolchain_version.replace(".", "_")
+            toolchain_version = full_version(toolchain_attr.python_version)
+            toolchain_version_short, _, _ = toolchain_version.rpartition(".")
+            toolchain_name = "python_" + version_label(toolchain_version, sep = "_")
 
             # Duplicate versions within a module indicate a misconfigured module.
-            if toolchain_version in module_toolchain_versions:
-                _fail_duplicate_module_toolchain_version(toolchain_version, mod.name)
-            module_toolchain_versions.append(toolchain_version)
+            if toolchain_version_short in module_toolchain_versions:
+                _fail_duplicate_module_toolchain_version(toolchain_version_short, mod.name)
+            module_toolchain_versions.append(toolchain_version_short)
 
             # Ignore version 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_version in global_toolchain_versions:
+            if toolchain_version_short in global_toolchain_versions:
                 _warn_duplicate_global_toolchain_version(
-                    toolchain_version,
-                    first = global_toolchain_versions[toolchain_version],
+                    first = global_toolchain_versions[toolchain_version_short],
                     second_toolchain_name = toolchain_name,
                     second_module_name = mod.name,
+                    second_toolchain_version = toolchain_version,
                 )
                 continue
-            global_toolchain_versions[toolchain_version] = struct(
+            global_toolchain_versions[toolchain_version_short] = struct(
                 toolchain_name = toolchain_name,
                 module_name = mod.name,
+                version = toolchain_version,
             )
 
             # Only the root module and rules_python are allowed to specify the default
@@ -165,6 +169,8 @@ def _python_impl(module_ctx):
     multi_toolchain_aliases(
         name = "python_versions",
         python_versions = {
+            # NOTE @aignas 2023-07-24: we are effectively using the
+            # version_label function to drop the patch version information.
             version: entry.toolchain_name
             for version, entry in global_toolchain_versions.items()
         },
@@ -177,17 +183,18 @@ def _fail_duplicate_module_toolchain_version(version, module):
         module = module,
     ))
 
-def _warn_duplicate_global_toolchain_version(version, first, second_toolchain_name, second_module_name):
+def _warn_duplicate_global_toolchain_version(first, second_toolchain_name, second_module_name, second_toolchain_version):
     _print_warn((
-        "Ignoring toolchain '{second_toolchain}' from module '{second_module}': " +
+        "Ignoring toolchain '{second_toolchain}' ({second_version}) from module '{second_module}': " +
         "Toolchain '{first_toolchain}' from module '{first_module}' " +
-        "already registered Python version {version} and has precedence"
+        "already registered Python version {first_version} and has precedence"
     ).format(
         first_toolchain = first.toolchain_name,
         first_module = first.module_name,
         second_module = second_module_name,
         second_toolchain = second_toolchain_name,
-        version = version,
+        first_version = first.version,
+        second_version = second_toolchain_version,
     ))
 
 def _fail_multiple_default_toolchains(first, second):

python/pip.bzl

@@ -17,8 +17,9 @@ load("//python/pip_install:pip_repository.bzl", "pip_repository", _package_annot
 load("//python/pip_install:repositories.bzl", "pip_install_dependencies")
 load("//python/pip_install:requirements.bzl", _compile_pip_requirements = "compile_pip_requirements")
 load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED")
+load("//python/private:full_version.bzl", "full_version")
 load("//python/private:render_pkg_aliases.bzl", "NO_MATCH_ERROR_MESSAGE_TEMPLATE")
-load(":versions.bzl", "MINOR_MAPPING")
+load("//python/private:version_label.bzl", "version_label")
 
 compile_pip_requirements = _compile_pip_requirements
 package_annotation = _package_annotation
@@ -166,7 +167,7 @@ def _multi_pip_parse_impl(rctx):
     install_deps_calls = []
     process_requirements_calls = []
     for python_version, pypi_repository in rctx.attr.pip_parses.items():
-        sanitized_python_version = python_version.replace(".", "_")
+        sanitized_python_version = version_label(python_version, sep = "_")
         load_statement = """\
 load(
     "@{pypi_repository}//:requirements.bzl",
@@ -184,7 +185,7 @@ _process_requirements(
     repo_prefix = "{pypi_repository}_",
 )""".format(
             pypi_repository = pypi_repository,
-            python_version = python_version,
+            python_version = version_label(python_version, sep = "."),
             sanitized_python_version = sanitized_python_version,
         )
         process_requirements_calls.append(process_requirements_call)
@@ -295,7 +296,7 @@ alias(
     for [python_version, repo_prefix] in version_map:
         alias.append("""\
         "@{rules_python}//python/config_settings:is_python_{full_python_version}": "{actual}",""".format(
-            full_python_version = MINOR_MAPPING[python_version] if python_version in MINOR_MAPPING else python_version,
+            full_python_version = full_version(python_version),
             actual = "@{repo_prefix}{wheel_name}//:{alias_name}".format(
                 repo_prefix = repo_prefix,
                 wheel_name = wheel_name,
@@ -363,13 +364,21 @@ def multi_pip_parse(name, default_version, python_versions, python_interpreter_t
         The internal implementation of multi_pip_parse repository rule.
     """
     pip_parses = {}
+    python_interpreter_target = {
+        full_version(v): t
+        for v, t in python_interpreter_target.items()
+    }
+    requirements_lock = {
+        full_version(v): t
+        for v, t in requirements_lock.items()
+    }
     for python_version in python_versions:
         if not python_version in python_interpreter_target:
             fail("Missing python_interpreter_target for Python version %s in '%s'" % (python_version, name))
         if not python_version in requirements_lock:
             fail("Missing requirements_lock for Python version %s in '%s'" % (python_version, name))
 
-        pip_parse_name = name + "_" + python_version.replace(".", "_")
+        pip_parse_name = name + "_" + version_label(python_version, sep = "_")
         pip_parse(
             name = pip_parse_name,
             python_interpreter_target = python_interpreter_target[python_version],

python/private/full_version.bzl

@@ -0,0 +1,35 @@
+# Copyright 2022 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"A small helper to ensure that we are working with full versions."
+
+load("//python:versions.bzl", "MINOR_MAPPING")
+
+def full_version(version):
+    """Return a full version.
+
+    Args:
+        version: the version in `X.Y` or `X.Y.Z` format.
+
+    Returns:
+        a full version given the version string. If the string is already a
+        major version then we return it as is.
+    """
+    parts = version.split(".")
+    if len(parts) == 2:
+        return MINOR_MAPPING[version]
+    elif len(parts) == 3:
+        return version
+    else:
+        fail("Unknown version format: {}".format(version))

python/private/toolchains_repo.bzl

@@ -30,6 +30,8 @@ load(
     "PLATFORMS",
     "WINDOWS_NAME",
 )
+load("//python/private:full_version.bzl", "full_version")
+load("//python/private:version_label.bzl", "version_label")
 
 def get_repository_name(repository_workspace):
     dummy_label = "//:_"
@@ -232,12 +234,48 @@ toolchain_aliases = repository_rule(
     },
 )
 
+def _validate_uniqueness(python_versions):
+    """Validate that the Python versions have unique `X.Y` entries
+
+    Otherwise the repository creation will fail with weird "file-exists" errors
+    and this allows us to have an actionable error message to the user.
+
+    Args:
+        python_versions: The list of python versions
+    """
+    minor_versions = {}
+    for version in python_versions:
+        short_version = version_label(version, sep = ".")
+        versions = minor_versions.get(short_version, [])
+        versions.append(version)
+        minor_versions[short_version] = versions
+
+    non_unique_versions = {
+        k: v
+        for k, v in minor_versions.items()
+        if len(v) != 1
+    }
+
+    if non_unique_versions:
+        fail(
+            "Expected all python versions map to unique 'X.Y' values but found duplicates:\n" +
+            "".join([
+                "    '{}': {}\n".format(
+                    minor_version,
+                    ", ".join(versions),
+                )
+                for minor_version, versions in non_unique_versions.items()
+            ]) +
+            "Please remove duplicates.",
+        )
+
 def _multi_toolchain_aliases_impl(rctx):
     rules_python = rctx.attr._rules_python_workspace.workspace_name
 
+    _validate_uniqueness(rctx.attr.python_versions.keys())
+
     for python_version, repository_name in rctx.attr.python_versions.items():
-        file = "{}/defs.bzl".format(python_version)
-        rctx.file(file, content = """\
+        content = """\
 # Generated by python/private/toolchains_repo.bzl
 
 load(
@@ -256,9 +294,17 @@ py_binary = _py_binary
 py_test = _py_test
 """.format(
             repository_name = repository_name,
-        ))
+        )
+
+        short_version = version_label(python_version, sep = ".")
+        warning = "\nprint(\"DEPRECATED: please migrate your code to use '@{}//{}:defs.bzl'\")".format(rctx.attr.name, short_version)
+
+        rctx.file("{}/defs.bzl".format(python_version), content = content + warning)
         rctx.file("{}/BUILD.bazel".format(python_version), "")
 
+        rctx.file("{}/defs.bzl".format(short_version), content = content)
+        rctx.file("{}/BUILD.bazel".format(short_version), "")
+
     pip_bzl = """\
 # Generated by python/private/toolchains_repo.bzl
 
@@ -273,7 +319,7 @@ def multi_pip_parse(name, requirements_lock, **kwargs):
     )
 
 """.format(
-        python_versions = rctx.attr.python_versions.keys(),
+        python_versions = [full_version(v) for v in rctx.attr.python_versions.keys()],
         rules_python = rules_python,
     )
     rctx.file("pip.bzl", content = pip_bzl)