Skip to content

Commit

Permalink
refactor(toolchain): use a helper method to convert an X.Y version to…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
aignas committed Sep 27, 2023
1 parent 49b21ce commit ee776d3
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 13 deletions.
11 changes: 3 additions & 8 deletions python/extensions/private/pythons_hub.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions python/pip.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
43 changes: 43 additions & 0 deletions python/private/full_version.bzl
Original file line number Diff line number Diff line change
@@ -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))
5 changes: 2 additions & 3 deletions python/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit ee776d3

Please sign in to comment.