Skip to content

refactor: consolidate version parsing #2874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions python/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ bzl_library(
name = "config_settings_bzl",
srcs = ["config_settings.bzl"],
deps = [
":semver_bzl",
":version_bzl",
"@bazel_skylib//lib:selects",
"@bazel_skylib//rules:common_settings",
],
Expand Down Expand Up @@ -249,9 +249,9 @@ bzl_library(
":python_register_toolchains_bzl",
":pythons_hub_bzl",
":repo_utils_bzl",
":semver_bzl",
":toolchains_repo_bzl",
":util_bzl",
":version_bzl",
"@bazel_features//:features",
],
)
Expand Down Expand Up @@ -610,11 +610,6 @@ bzl_library(
],
)

bzl_library(
name = "semver_bzl",
srcs = ["semver.bzl"],
)

bzl_library(
name = "sentinel_bzl",
srcs = ["sentinel.bzl"],
Expand Down
6 changes: 3 additions & 3 deletions python/private/config_settings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
load("@bazel_skylib//lib:selects.bzl", "selects")
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load("//python/private:text_util.bzl", "render")
load(":semver.bzl", "semver")
load(":version.bzl", "version")

_PYTHON_VERSION_FLAG = Label("//python/config_settings:python_version")
_PYTHON_VERSION_MAJOR_MINOR_FLAG = Label("//python/config_settings:python_version_major_minor")
Expand Down Expand Up @@ -181,8 +181,8 @@ _python_version_flag = rule(
def _python_version_major_minor_flag_impl(ctx):
input = _flag_value(ctx.attr._python_version_flag)
if input:
version = semver(input)
value = "{}.{}".format(version.major, version.minor)
ver = version.parse(input)
value = "{}.{}".format(ver.release[0], ver.release[1])
else:
value = ""

Expand Down
15 changes: 9 additions & 6 deletions python/private/hermetic_runtime_repo_setup.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ load("//python:py_runtime_pair.bzl", "py_runtime_pair")
load("//python/cc:py_cc_toolchain.bzl", "py_cc_toolchain")
load(":glob_excludes.bzl", "glob_excludes")
load(":py_exec_tools_toolchain.bzl", "py_exec_tools_toolchain")
load(":semver.bzl", "semver")
load(":version.bzl", "version")

_IS_FREETHREADED = Label("//python/config_settings:is_py_freethreaded")

Expand Down Expand Up @@ -53,8 +53,11 @@ def define_hermetic_runtime_toolchain_impl(
use.
"""
_ = name # @unused
version_info = semver(python_version)
version_dict = version_info.to_dict()
version_info = version.parse(python_version)
version_dict = {
"major": version_info.release[0],
"minor": version_info.release[1],
}
native.filegroup(
name = "files",
srcs = native.glob(
Expand Down Expand Up @@ -198,9 +201,9 @@ def define_hermetic_runtime_toolchain_impl(
files = [":files"],
interpreter = python_bin,
interpreter_version_info = {
"major": str(version_info.major),
"micro": str(version_info.patch),
"minor": str(version_info.minor),
"major": str(version_info.release[0]),
"micro": str(version_info.release[2]),
"minor": str(version_info.release[1]),
},
coverage_tool = select({
# Convert empty string to None
Expand Down
4 changes: 2 additions & 2 deletions python/private/pypi/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ bzl_library(
":whl_target_platforms_bzl",
"//python/private:full_version_bzl",
"//python/private:normalize_name_bzl",
"//python/private:semver_bzl",
"//python/private:version_bzl",
"//python/private:version_label_bzl",
"@bazel_features//:features",
"@pythons_hub//:interpreters_bzl",
Expand Down Expand Up @@ -256,7 +256,7 @@ bzl_library(
srcs = ["pep508_evaluate.bzl"],
deps = [
"//python/private:enum_bzl",
"//python/private:semver_bzl",
"//python/private:version_bzl",
],
)

Expand Down
8 changes: 4 additions & 4 deletions python/private/pypi/extension.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ load("//python/private:auth.bzl", "AUTH_ATTRS")
load("//python/private:full_version.bzl", "full_version")
load("//python/private:normalize_name.bzl", "normalize_name")
load("//python/private:repo_utils.bzl", "repo_utils")
load("//python/private:semver.bzl", "semver")
load("//python/private:version.bzl", "version")
load("//python/private:version_label.bzl", "version_label")
load(":attrs.bzl", "use_isolated")
load(":evaluate_markers.bzl", "evaluate_markers_py", EVALUATE_MARKERS_SRCS = "SRCS")
Expand All @@ -36,9 +36,9 @@ load(":whl_config_setting.bzl", "whl_config_setting")
load(":whl_library.bzl", "whl_library")
load(":whl_repo_name.bzl", "pypi_repo_name", "whl_repo_name")

def _major_minor_version(version):
version = semver(version)
return "{}.{}".format(version.major, version.minor)
def _major_minor_version(version_str):
ver = version.parse(version_str)
return "{}.{}".format(ver.release[0], ver.release[1])

def _whl_mods_impl(whl_mods_dict):
"""Implementation of the pip.whl_mods tag class.
Expand Down
37 changes: 22 additions & 15 deletions python/private/python.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ load(":full_version.bzl", "full_version")
load(":python_register_toolchains.bzl", "python_register_toolchains")
load(":pythons_hub.bzl", "hub_repo")
load(":repo_utils.bzl", "repo_utils")
load(":semver.bzl", "semver")
load(":toolchains_repo.bzl", "multi_toolchain_aliases")
load(":util.bzl", "IS_BAZEL_6_4_OR_HIGHER")
load(":version.bzl", "version")

def parse_modules(*, module_ctx, _fail = fail):
"""Parse the modules and return a struct for registrations.
Expand Down Expand Up @@ -458,16 +458,20 @@ def _fail_multiple_default_toolchains(first, second):
second = second,
))

def _validate_version(*, version, _fail = fail):
parsed = semver(version)
if parsed.patch == None or parsed.build or parsed.pre_release:
_fail("The 'python_version' attribute needs to specify an 'X.Y.Z' semver-compatible version, got: '{}'".format(version))
def _validate_version(version_str, *, _fail = fail):
v = version.parse(version_str, strict = True, _fail = _fail)
if v == None:
# Only reachable in tests
return False

if len(v.release) < 3:
_fail("The 'python_version' attribute needs to specify the full version in at least 'X.Y.Z' format, got: '{}'".format(v.string))
return False

return True

def _process_single_version_overrides(*, tag, _fail = fail, default):
if not _validate_version(version = tag.python_version, _fail = _fail):
if not _validate_version(tag.python_version, _fail = _fail):
return

available_versions = default["tool_versions"]
Expand Down Expand Up @@ -517,7 +521,7 @@ def _process_single_version_overrides(*, tag, _fail = fail, default):
kwargs.setdefault(tag.python_version, {})["distutils"] = tag.distutils

def _process_single_version_platform_overrides(*, tag, _fail = fail, default):
if not _validate_version(version = tag.python_version, _fail = _fail):
if not _validate_version(tag.python_version, _fail = _fail):
return

available_versions = default["tool_versions"]
Expand Down Expand Up @@ -558,12 +562,12 @@ def _process_global_overrides(*, tag, default, _fail = fail):

if tag.minor_mapping:
for minor_version, full_version in tag.minor_mapping.items():
parsed = semver(minor_version)
if parsed.patch != None or parsed.build or parsed.pre_release:
fail("Expected the key to be of `X.Y` format but got `{}`".format(minor_version))
parsed = semver(full_version)
if parsed.patch == None:
fail("Expected the value to at least be of `X.Y.Z` format but got `{}`".format(minor_version))
parsed = version.parse(minor_version, strict = True, _fail = _fail)
if len(parsed.release) > 2 or parsed.pre or parsed.post or parsed.dev or parsed.local:
fail("Expected the key to be of `X.Y` format but got `{}`".format(parsed.string))

# Ensure that the version is valid
version.parse(full_version, strict = True, _fail = _fail)

default["minor_mapping"] = tag.minor_mapping

Expand Down Expand Up @@ -651,8 +655,11 @@ def _get_toolchain_config(*, modules, _fail = fail):

versions = {}
for version_string in available_versions:
v = semver(version_string)
versions.setdefault("{}.{}".format(v.major, v.minor), []).append((int(v.patch), version_string))
v = version.parse(version_string, strict = True)
versions.setdefault(
"{}.{}".format(v.release[0], v.release[1]),
[],
).append((version.key(v), v.string))

minor_mapping = {
major_minor: max(subset)[1]
Expand Down
85 changes: 0 additions & 85 deletions python/private/semver.bzl

This file was deleted.

28 changes: 15 additions & 13 deletions python/private/version.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ def normalize_pep440(version):
"""
return _parse(version, strict = True)["norm"]

def _parse(version_str, strict = True):
def _parse(version_str, strict = True, _fail = fail):
"""Escape the version component of a filename.

See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode
Expand All @@ -519,6 +519,7 @@ def _parse(version_str, strict = True):
Args:
version_str: version string to be normalized according to PEP 440.
strict: fail if the version is invalid, defaults to True.
_fail: Used for tests

Returns:
string containing the normalized version.
Expand All @@ -544,7 +545,7 @@ def _parse(version_str, strict = True):
parser_ctx = parser.context()
if parser.input[parser_ctx["start"]:]:
if strict:
fail(
_fail(
"Failed to parse PEP 440 version identifier '%s'." % parser.input,
"Parse error at '%s'" % parser.input[parser_ctx["start"]:],
)
Expand All @@ -554,7 +555,7 @@ def _parse(version_str, strict = True):
parser_ctx["is_prefix"] = is_prefix
return parser_ctx

def parse(version_str, strict = False):
def parse(version_str, strict = False, _fail = fail):
"""Parse a PEP4408 compliant version.

This is similar to `normalize_pep440`, but it parses individual components to
Expand All @@ -563,6 +564,7 @@ def parse(version_str, strict = False):
Args:
version_str: version string to be normalized according to PEP 440.
strict: fail if the version is invalid.
_fail: used for tests

Returns:
a struct with individual components of a version:
Expand All @@ -580,29 +582,29 @@ def parse(version_str, strict = False):
* `string` {type}`str` normalized value of the input.
"""

parts = _parse(version_str, strict = strict)
parts = _parse(version_str, strict = strict, _fail = _fail)
if not parts:
return None

if parts["is_prefix"] and (parts["local"] or parts["post"] or parts["dev"] or parts["pre"]):
if strict:
fail("local version part has been obtained, but only public segments can have prefix matches")
_fail("local version part has been obtained, but only public segments can have prefix matches")

# https://peps.python.org/pep-0440/#public-version-identifiers
return None

return struct(
epoch = _parse_epoch(parts["epoch"]),
epoch = _parse_epoch(parts["epoch"], _fail),
release = _parse_release(parts["release"]),
pre = _parse_pre(parts["pre"]),
post = _parse_post(parts["post"]),
dev = _parse_dev(parts["dev"]),
local = _parse_local(parts["local"]),
post = _parse_post(parts["post"], _fail),
dev = _parse_dev(parts["dev"], _fail),
local = _parse_local(parts["local"], _fail),
string = parts["norm"],
is_prefix = parts["is_prefix"],
)

def _parse_epoch(value):
def _parse_epoch(value, fail):
if not value:
return 0

Expand All @@ -614,7 +616,7 @@ def _parse_epoch(value):
def _parse_release(value):
return tuple([int(d) for d in value.split(".")])

def _parse_local(value):
def _parse_local(value, fail):
if not value:
return None

Expand All @@ -624,7 +626,7 @@ def _parse_local(value):
# If the part is numerical, handle it as a number
return tuple([int(part) if part.isdigit() else part for part in value[1:].split(".")])

def _parse_dev(value):
def _parse_dev(value, fail):
if not value:
return None

Expand All @@ -646,7 +648,7 @@ def _parse_pre(value):

return (prefix, int(value[len(prefix):]))

def _parse_post(value):
def _parse_post(value, fail):
if not value:
return None

Expand Down
Loading