From ee776d362adff2e17af1fbc1e675a23bad60a2f9 Mon Sep 17 00:00:00 2001 From: Ignas <240938+aignas@users.noreply.github.com> Date: Thu, 21 Sep 2023 08:48:31 +0900 Subject: [PATCH] refactor(toolchain): use a helper method to convert an X.Y version to X.Y.Z Before this PR in numerous places we would check the MINOR_MAPPING dict. This PR adds a function that also fails if the X.Y format is not in the MINOR_MAPPING dict making the code more robust. Split from #1340 to unblock #1364. --- python/extensions/private/pythons_hub.bzl | 11 ++---- python/pip.bzl | 4 +-- python/private/full_version.bzl | 43 +++++++++++++++++++++++ python/repositories.bzl | 5 ++- 4 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 python/private/full_version.bzl diff --git a/python/extensions/private/pythons_hub.bzl b/python/extensions/private/pythons_hub.bzl index a64f203bd..f36ce4552 100644 --- a/python/extensions/private/pythons_hub.bzl +++ b/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, diff --git a/python/pip.bzl b/python/pip.bzl index 0c6e90f57..fb842cc4c 100644 --- a/python/pip.bzl +++ b/python/pip.bzl @@ -17,8 +17,8 @@ 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") compile_pip_requirements = _compile_pip_requirements package_annotation = _package_annotation @@ -295,7 +295,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, diff --git a/python/private/full_version.bzl b/python/private/full_version.bzl new file mode 100644 index 000000000..94f1fdba8 --- /dev/null +++ b/python/private/full_version.bzl @@ -0,0 +1,43 @@ +# 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. + """ + if version in MINOR_MAPPING: + return MINOR_MAPPING[version] + + parts = version.split(".") + if len(parts) == 3: + return version + elif len(parts) == 2: + fail( + "Unknown Python version '{}', available values are: {}".format( + version, + ",".join(MINOR_MAPPING.keys()), + ), + ) + else: + fail("Unknown version format: {}".format(version)) diff --git a/python/repositories.bzl b/python/repositories.bzl index 9b3926ac1..050ba14a7 100644 --- a/python/repositories.bzl +++ b/python/repositories.bzl @@ -21,6 +21,7 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", _http_archive = "http_archi load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe", "read_netrc", "read_user_netrc", "use_netrc") load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED") load("//python/private:coverage_deps.bzl", "coverage_dep") +load("//python/private:full_version.bzl", "full_version") load("//python/private:internal_config_repo.bzl", "internal_config_repo") load( "//python/private:toolchains_repo.bzl", @@ -32,7 +33,6 @@ load("//python/private:which.bzl", "which_with_fail") load( ":versions.bzl", "DEFAULT_RELEASE_BASE_URL", - "MINOR_MAPPING", "PLATFORMS", "TOOL_VERSIONS", "get_release_info", @@ -534,8 +534,7 @@ def python_register_toolchains( base_url = kwargs.pop("base_url", DEFAULT_RELEASE_BASE_URL) - if python_version in MINOR_MAPPING: - python_version = MINOR_MAPPING[python_version] + python_version = full_version(python_version) toolchain_repo_name = "{name}_toolchains".format(name = name)