refactor: consolidate version parsing (#2874)
This PR removes all of the custom version parsing functions where we try
to make sense about the version (e.g. extracting major/minor versions).
Whilst doing this I actually think that I made it easier to support
#2837.
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
index e72a8fc..0b50ccf 100644
--- a/python/private/BUILD.bazel
+++ b/python/private/BUILD.bazel
@@ -139,7 +139,7 @@
name = "config_settings_bzl",
srcs = ["config_settings.bzl"],
deps = [
- ":semver_bzl",
+ ":version_bzl",
"@bazel_skylib//lib:selects",
"@bazel_skylib//rules:common_settings",
],
@@ -249,9 +249,9 @@
":python_register_toolchains_bzl",
":pythons_hub_bzl",
":repo_utils_bzl",
- ":semver_bzl",
":toolchains_repo_bzl",
":util_bzl",
+ ":version_bzl",
"@bazel_features//:features",
],
)
@@ -611,11 +611,6 @@
)
bzl_library(
- name = "semver_bzl",
- srcs = ["semver.bzl"],
-)
-
-bzl_library(
name = "sentinel_bzl",
srcs = ["sentinel.bzl"],
)
diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl
index 5eb858e..aff5d01 100644
--- a/python/private/config_settings.bzl
+++ b/python/private/config_settings.bzl
@@ -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")
@@ -181,8 +181,8 @@
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 = ""
diff --git a/python/private/hermetic_runtime_repo_setup.bzl b/python/private/hermetic_runtime_repo_setup.bzl
index 64d721e..f944b0b 100644
--- a/python/private/hermetic_runtime_repo_setup.bzl
+++ b/python/private/hermetic_runtime_repo_setup.bzl
@@ -20,7 +20,7 @@
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")
@@ -53,8 +53,11 @@
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(
@@ -198,9 +201,9 @@
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
diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel
index 06ca3a8..84e0535 100644
--- a/python/private/pypi/BUILD.bazel
+++ b/python/private/pypi/BUILD.bazel
@@ -116,7 +116,7 @@
":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",
@@ -256,7 +256,7 @@
srcs = ["pep508_evaluate.bzl"],
deps = [
"//python/private:enum_bzl",
- "//python/private:semver_bzl",
+ "//python/private:version_bzl",
],
)
diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl
index 84caa0a..3896f29 100644
--- a/python/private/pypi/extension.bzl
+++ b/python/private/pypi/extension.bzl
@@ -22,7 +22,7 @@
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")
@@ -36,9 +36,9 @@
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.
diff --git a/python/private/python.bzl b/python/private/python.bzl
index 53cd5e9..c187904 100644
--- a/python/private/python.bzl
+++ b/python/private/python.bzl
@@ -21,9 +21,9 @@
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.
@@ -458,16 +458,20 @@
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"]
@@ -517,7 +521,7 @@
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"]
@@ -558,12 +562,12 @@
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
@@ -651,8 +655,11 @@
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]
diff --git a/python/private/semver.bzl b/python/private/semver.bzl
deleted file mode 100644
index 0cbd172..0000000
--- a/python/private/semver.bzl
+++ /dev/null
@@ -1,85 +0,0 @@
-# Copyright 2024 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 semver version parser"
-
-def _key(version):
- return (
- version.major,
- version.minor or 0,
- version.patch or 0,
- # non pre-release versions are higher
- version.pre_release == "",
- # then we compare each element of the pre_release tag separately
- tuple([
- (
- i if not i.isdigit() else "",
- # digit values take precedence
- int(i) if i.isdigit() else 0,
- )
- for i in version.pre_release.split(".")
- ]) if version.pre_release else None,
- # And build info is just alphabetic
- version.build,
- )
-
-def _to_dict(self):
- return {
- "build": self.build,
- "major": self.major,
- "minor": self.minor,
- "patch": self.patch,
- "pre_release": self.pre_release,
- }
-
-def _new(*, major, minor, patch, pre_release, build, version = None):
- # buildifier: disable=uninitialized
- self = struct(
- major = int(major),
- minor = None if minor == None else int(minor),
- # NOTE: this is called `micro` in the Python interpreter versioning scheme
- patch = None if patch == None else int(patch),
- pre_release = pre_release,
- build = build,
- # buildifier: disable=uninitialized
- key = lambda: _key(self),
- str = lambda: version,
- to_dict = lambda: _to_dict(self),
- )
- return self
-
-def semver(version):
- """Parse the semver version and return the values as a struct.
-
- Args:
- version: {type}`str` the version string.
-
- Returns:
- A {type}`struct` with `major`, `minor`, `patch` and `build` attributes.
- """
-
- # Implement the https://semver.org/ spec
- major, _, tail = version.partition(".")
- minor, _, tail = tail.partition(".")
- patch, _, build = tail.partition("+")
- patch, _, pre_release = patch.partition("-")
-
- return _new(
- major = int(major),
- minor = int(minor) if minor.isdigit() else None,
- patch = int(patch) if patch.isdigit() else None,
- build = build,
- pre_release = pre_release,
- version = version,
- )
diff --git a/python/private/version.bzl b/python/private/version.bzl
index 4425cc7..f98165d 100644
--- a/python/private/version.bzl
+++ b/python/private/version.bzl
@@ -510,7 +510,7 @@
"""
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
@@ -519,6 +519,7 @@
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.
@@ -544,7 +545,7 @@
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"]:],
)
@@ -554,7 +555,7 @@
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
@@ -563,6 +564,7 @@
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:
@@ -580,29 +582,29 @@
* `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
@@ -614,7 +616,7 @@
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
@@ -624,7 +626,7 @@
# 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
@@ -646,7 +648,7 @@
return (prefix, int(value[len(prefix):]))
-def _parse_post(value):
+def _parse_post(value, fail):
if not value:
return None
diff --git a/tests/python/python_tests.bzl b/tests/python/python_tests.bzl
index 97c47b5..443174c 100644
--- a/tests/python/python_tests.bzl
+++ b/tests/python/python_tests.bzl
@@ -746,12 +746,6 @@
],
want_error = "Only a single 'python.single_version_override' can be present for '3.12.4'",
),
- struct(
- overrides = [
- _single_version_override(python_version = "3.12.4+3", distutils_content = "foo"),
- ],
- want_error = "The 'python_version' attribute needs to specify an 'X.Y.Z' semver-compatible version, got: '3.12.4+3'",
- ),
]:
errors = []
parse_modules(
@@ -781,13 +775,13 @@
overrides = [
_single_version_platform_override(python_version = "3.12", platform = "foo"),
],
- want_error = "The 'python_version' attribute needs to specify an 'X.Y.Z' semver-compatible version, got: '3.12'",
+ want_error = "The 'python_version' attribute needs to specify the full version in at least 'X.Y.Z' format, got: '3.12'",
),
struct(
overrides = [
- _single_version_platform_override(python_version = "3.12.1+my_build", platform = "foo"),
+ _single_version_platform_override(python_version = "foo", platform = "foo"),
],
- want_error = "The 'python_version' attribute needs to specify an 'X.Y.Z' semver-compatible version, got: '3.12.1+my_build'",
+ want_error = "Failed to parse PEP 440 version identifier 'foo'. Parse error at 'foo'",
),
]:
errors = []
@@ -799,7 +793,7 @@
single_version_platform_override = test.overrides,
),
),
- _fail = errors.append,
+ _fail = lambda *a: errors.append(" ".join(a)),
)
env.expect.that_collection(errors).contains_exactly([test.want_error])
diff --git a/tests/semver/BUILD.bazel b/tests/semver/BUILD.bazel
deleted file mode 100644
index e12b1e5..0000000
--- a/tests/semver/BUILD.bazel
+++ /dev/null
@@ -1,17 +0,0 @@
-# Copyright 2024 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.
-
-load(":semver_test.bzl", "semver_test_suite")
-
-semver_test_suite(name = "semver_tests")
diff --git a/tests/semver/semver_test.bzl b/tests/semver/semver_test.bzl
deleted file mode 100644
index 9d13402..0000000
--- a/tests/semver/semver_test.bzl
+++ /dev/null
@@ -1,113 +0,0 @@
-# Copyright 2023 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.
-
-""
-
-load("@rules_testing//lib:test_suite.bzl", "test_suite")
-load("//python/private:semver.bzl", "semver") # buildifier: disable=bzl-visibility
-
-_tests = []
-
-def _test_semver_from_major(env):
- actual = semver("3")
- env.expect.that_int(actual.major).equals(3)
- env.expect.that_int(actual.minor).equals(None)
- env.expect.that_int(actual.patch).equals(None)
- env.expect.that_str(actual.build).equals("")
-
-_tests.append(_test_semver_from_major)
-
-def _test_semver_from_major_minor_version(env):
- actual = semver("4.9")
- env.expect.that_int(actual.major).equals(4)
- env.expect.that_int(actual.minor).equals(9)
- env.expect.that_int(actual.patch).equals(None)
- env.expect.that_str(actual.build).equals("")
-
-_tests.append(_test_semver_from_major_minor_version)
-
-def _test_semver_with_build_info(env):
- actual = semver("1.2.3+mybuild")
- env.expect.that_int(actual.major).equals(1)
- env.expect.that_int(actual.minor).equals(2)
- env.expect.that_int(actual.patch).equals(3)
- env.expect.that_str(actual.build).equals("mybuild")
-
-_tests.append(_test_semver_with_build_info)
-
-def _test_semver_with_build_info_multiple_pluses(env):
- actual = semver("1.2.3-rc0+build+info")
- env.expect.that_int(actual.major).equals(1)
- env.expect.that_int(actual.minor).equals(2)
- env.expect.that_int(actual.patch).equals(3)
- env.expect.that_str(actual.pre_release).equals("rc0")
- env.expect.that_str(actual.build).equals("build+info")
-
-_tests.append(_test_semver_with_build_info_multiple_pluses)
-
-def _test_semver_alpha_beta(env):
- actual = semver("1.2.3-alpha.beta")
- env.expect.that_int(actual.major).equals(1)
- env.expect.that_int(actual.minor).equals(2)
- env.expect.that_int(actual.patch).equals(3)
- env.expect.that_str(actual.pre_release).equals("alpha.beta")
-
-_tests.append(_test_semver_alpha_beta)
-
-def _test_semver_sort(env):
- want = [
- semver(item)
- for item in [
- # The items are sorted from lowest to highest version
- "0.0.1",
- "0.1.0-rc",
- "0.1.0",
- "0.9.11",
- "0.9.12",
- "1.0.0-alpha",
- "1.0.0-alpha.1",
- "1.0.0-alpha.beta",
- "1.0.0-beta",
- "1.0.0-beta.2",
- "1.0.0-beta.11",
- "1.0.0-rc.1",
- "1.0.0-rc.2",
- "1.0.0",
- # Also handle missing minor and patch version strings
- "2.0",
- "3",
- # Alphabetic comparison for different builds
- "3.0.0+build0",
- "3.0.0+build1",
- ]
- ]
- actual = sorted(want, key = lambda x: x.key())
- env.expect.that_collection(actual).contains_exactly(want).in_order()
- for i, greater in enumerate(want[1:]):
- smaller = actual[i]
- if greater.key() <= smaller.key():
- env.fail("Expected '{}' to be smaller than '{}', but got otherwise".format(
- smaller.str(),
- greater.str(),
- ))
-
-_tests.append(_test_semver_sort)
-
-def semver_test_suite(name):
- """Create the test suite.
-
- Args:
- name: the name of the test suite
- """
- test_suite(name = name, basic_tests = _tests)
