From 2c124969a02d0fcde283b2520ee2d15b6bc6c7f3 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 May 2025 23:20:36 +0900 Subject: [PATCH 01/57] fix(pypi): default platform_version to 0 Work towards #2826 --- python/private/pypi/pep508_env.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/pypi/pep508_env.bzl b/python/private/pypi/pep508_env.bzl index 3708c46f1d..f6744a6e1e 100644 --- a/python/private/pypi/pep508_env.bzl +++ b/python/private/pypi/pep508_env.bzl @@ -174,7 +174,7 @@ def env(target_platform, *, extra = None): "implementation_name": "cpython", "platform_python_implementation": "CPython", "platform_release": "", - "platform_version": "", + "platform_version": "0", } if type(target_platform) == type(""): From 118590c1a29ea41ad94f342f5e5c53c1c719bdd3 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 4 May 2025 00:57:02 +0900 Subject: [PATCH 02/57] wip: finish PEP508/PEP440 impl for version matching --- python/private/py_wheel_normalize_pep440.bzl | 125 +++++++++++++++++++ python/private/pypi/pep508_evaluate.bzl | 42 ++++--- python/private/semver.bzl | 27 ---- tests/pypi/pep508/evaluate_tests.bzl | 33 +++++ tests/semver/semver_test.bzl | 18 --- 5 files changed, 180 insertions(+), 65 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 9566348987..40e4e8ea90 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -517,3 +517,128 @@ def normalize_pep440(version): "Parse error at '%s'" % parser.input[parser.context()["start"]:], ) return parser.context()["norm"] + +def _upper(*, epoch = 0, release, pre = "", post = "", dev = "", local = ""): + epoch = epoch + _release = list(release[:-1]) + pre = pre + post = post + dev = dev + local = local + + if pre or dev or local: + return _new_version( + epoch = epoch, + release = release, + ) + + _release[-1] = _release[-1] + 1 + release = ".".join([str(d) for d in _release]) + + return _new_version( + epoch = epoch, + release = release, + ) + +def _version_eq(left, right): + if left.epoch != right.epoch: + return False + + # Check at most 3 terms and check the same number of terms + check_len = min(min(len(left.release), len(right.release)), 3) + + return left.release[:check_len] == right.release[:check_len] + +def _version_lt(left, right): + if left.epoch < right.epoch: + return True + elif left.epoch > right.epoch: + return False + + return left.release < right.release + +def _version_gt(left, right): + if left.epoch > right.epoch: + return True + elif left.epoch < right.epoch: + return False + + return left.release > right.release + +def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = ""): + epoch = epoch or 0 + _release = tuple([int(d) for d in release.split(".")]) + pre = pre or "" + post = post or "" + dev = dev or "" + local = local or "" + + self = struct( + epoch = epoch, + release = _release, + pre = pre, + post = post, + dev = dev, + local = local, + upper = lambda: _upper( + epoch = epoch, + release = _release, + pre = pre, + post = post, + dev = dev, + local = local, + ), + key = lambda: ( + epoch, + _release, + pre, + post, + dev, + local, + ), + eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized + ne = lambda x: not _version_eq(self, x), # buildifier: disable=uninitialized + lt = lambda x: _version_lt(self, x), # buildifier: disable=uninitialized + gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized + le = lambda x: not _version_gt(self, x), # buildifier: disable=uninitialized + ge = lambda x: not _version_lt(self, x), # buildifier: disable=uninitialized + eqq = lambda x: _version_eqq(self, x), # buildifier: disable=uninitialized + ) + + return self + +def parse_version(version): + """Escape the version component of a filename. + + See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode + and https://peps.python.org/pep-0440/ + + Args: + version: version string to be normalized according to PEP 440. + + Returns: + string containing the normalized version. + """ + parser = _new(version.strip()) # PEP 440: Leading and Trailing Whitespace + accept(parser, _is("v"), "") # PEP 440: Preceding v character + + parts = {} + fns = [ + ("epoch", accept_epoch), + ("release", accept_release), + ("pre", accept_prerelease), + ("post", accept_postrelease), + ("dev", accept_devrelease), + ("local", accept_local), + ] + + for p, fn in fns: + fn(parser) + parts[p] = parser.context()["norm"] + parser.context()["norm"] = "" + + if parser.input[parser.context()["start"]:]: + # If we fail to parse the version return None + return None + + return _new_version(**parts) diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index 70840c76c6..60f06c2750 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -16,7 +16,8 @@ """ load("//python/private:enum.bzl", "enum") -load("//python/private:semver.bzl", "semver") +load("//python/private:py_wheel_normalize_pep440.bzl", "parse_version") + # The expression parsing and resolution for the PEP508 is below # @@ -353,36 +354,37 @@ def _env_expr(left, op, right): elif op == ">=": return left >= right else: - return fail("TODO: op unsupported: '{}'".format(op)) + return fail("unsupported op: '{}' {} '{}'".format(left, op, right)) def _version_expr(left, op, right): """Evaluate a version comparison expression""" - left = semver(left) - right = semver(right) - _left = left.key() - _right = right.key() + if op == "===": + # https://peps.python.org/pep-0440/#arbitrary-equality + # > simple string equality operations + return _env_expr(left, "==", right) + + _left = parse_version(left) + _right = parse_version(right) + if _left == None or _right == None: + # Sometimes `platform_version` is not a true PEP440 version, + # so then we fallback to a simple string expression evaluation + return _env_expr(left, op, right) + if op == "<": - return _left < _right + return _left.lt(_right) elif op == ">": - return _left > _right + return _left.gt(_right) elif op == "<=": - return _left <= _right + return _left.le(_right) elif op == ">=": - return _left >= _right + return _left.ge(_right) elif op == "!=": - return _left != _right + return _left.ne(_right) elif op == "==": # Matching of major, minor, patch only - return _left[:3] == _right[:3] + return _left.eq(_right) elif op == "~=": - right_plus = right.upper() - _right_plus = right_plus.key() - return _left >= _right and _left < _right_plus - elif op == "===": - # Strict matching - return _left == _right - elif op in _VERSION_CMP: - fail("TODO: op unsupported: '{}'".format(op)) + return _left.ge(_right) and _left.lt(_right.upper()) else: return False # Let's just ignore the invalid ops diff --git a/python/private/semver.bzl b/python/private/semver.bzl index cc9ae6ecb6..0cbd172348 100644 --- a/python/private/semver.bzl +++ b/python/private/semver.bzl @@ -43,32 +43,6 @@ def _to_dict(self): "pre_release": self.pre_release, } -def _upper(self): - major = self.major - minor = self.minor - patch = self.patch - build = "" - pre_release = "" - version = self.str() - - if patch != None: - minor = minor + 1 - patch = 0 - elif minor != None: - major = major + 1 - minor = 0 - elif minor == None: - major = major + 1 - - return _new( - major = major, - minor = minor, - patch = patch, - build = build, - pre_release = pre_release, - version = "~" + version, - ) - def _new(*, major, minor, patch, pre_release, build, version = None): # buildifier: disable=uninitialized self = struct( @@ -82,7 +56,6 @@ def _new(*, major, minor, patch, pre_release, build, version = None): key = lambda: _key(self), str = lambda: version, to_dict = lambda: _to_dict(self), - upper = lambda: _upper(self), ) return self diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 303c167900..952d07ab75 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -123,6 +123,7 @@ def _evaluate_version_env_tests(env): "{} <= '3.7.10'".format(var_name): True, "{} <= '3.7.8'".format(var_name): False, "{} == '3.7.9'".format(var_name): True, + "{} == '3.7.*'".format(var_name): True, "{} != '3.7.9'".format(var_name): False, "{} ~= '3.7.1'".format(var_name): True, "{} ~= '3.7.10'".format(var_name): False, @@ -148,6 +149,38 @@ def _evaluate_version_env_tests(env): _tests.append(_evaluate_version_env_tests) +def _evaluate_platform_version_is_special(env): + # Given + marker_env = {"platform_version": "FooBar Linux v1.2.3"} + + # When the platform version is not + input = "platform_version == '0'" + got = evaluate( + input, + env = marker_env, + ) + env.expect.that_collection((input, got)).contains_exactly((input, False)) + + # And when I compare it as string + input = "'FooBar' in platform_version" + got = evaluate( + input, + env = marker_env, + ) + env.expect.that_collection((input, got)).contains_exactly((input, True)) + + + # Check that the non-strict eval gives us back the input when no + # env is supplied. + got = evaluate( + input, + env = {}, + strict = False, + ) + env.expect.that_bool(got).equals(input.replace("'", '"')) + +_tests.append(_evaluate_platform_version_is_special) + def _logical_expression_tests(env): for input, want in { # Basic diff --git a/tests/semver/semver_test.bzl b/tests/semver/semver_test.bzl index aef3deca82..9d13402c92 100644 --- a/tests/semver/semver_test.bzl +++ b/tests/semver/semver_test.bzl @@ -104,24 +104,6 @@ def _test_semver_sort(env): _tests.append(_test_semver_sort) -def _test_upper(env): - for input, want in { - # Depending on how many version numbers are specified we will increase - # the upper bound differently. See https://packaging.python.org/en/latest/specifications/version-specifiers/#compatible-release for docs - "0.0.1": "0.1.0", - "0.1": "1.0", - "0.1.0": "0.2.0", - "1": "2", - "1.0.0-pre": "1.1.0", # pre-release info is dropped - "1.2.0": "1.3.0", - "2.0.0+build0": "2.1.0", # build info is dropped - }.items(): - actual = semver(input).upper().key() - want = semver(want).key() - env.expect.that_collection(actual).contains_exactly(want).in_order() - -_tests.append(_test_upper) - def semver_test_suite(name): """Create the test suite. From aa613f866d504094b6acf6abab5ba00c9d7b1c36 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 4 May 2025 01:05:54 +0900 Subject: [PATCH 03/57] wip --- python/private/py_wheel_normalize_pep440.bzl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 40e4e8ea90..aab36bcc36 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -602,7 +602,6 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized le = lambda x: not _version_gt(self, x), # buildifier: disable=uninitialized ge = lambda x: not _version_lt(self, x), # buildifier: disable=uninitialized - eqq = lambda x: _version_eqq(self, x), # buildifier: disable=uninitialized ) return self @@ -619,7 +618,10 @@ def parse_version(version): Returns: string containing the normalized version. """ - parser = _new(version.strip()) # PEP 440: Leading and Trailing Whitespace + + # TODO @aignas 2025-05-04: this is discarding '.*', but per spec this should be only done + # for public version segments + parser = _new(version.strip(" .*")) # PEP 440: Leading and Trailing Whitespace accept(parser, _is("v"), "") # PEP 440: Preceding v character parts = {} From 28ff6be4dfb338c791a9e4159ad366bbfbb3cd1f Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 4 May 2025 01:39:38 +0900 Subject: [PATCH 04/57] wip --- python/private/py_wheel_normalize_pep440.bzl | 92 ++++++++++++++++---- 1 file changed, 73 insertions(+), 19 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index aab36bcc36..a84ee8f430 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -540,23 +540,75 @@ def _upper(*, epoch = 0, release, pre = "", post = "", dev = "", local = ""): release = release, ) +def _pad_zeros(release, n): + if len(release) >= n: + return release + + release = list(release) + release += [0] * len(release) + return tuple(release) + +# TODO @aignas 2025-05-04: add tests for the comparison def _version_eq(left, right): if left.epoch != right.epoch: return False - # Check at most 3 terms and check the same number of terms - check_len = min(min(len(left.release), len(right.release)), 3) + if left.is_prefix: + right_release = right.release[:len(left.release)] + else: + right_release = _pad_zeros(right.release, len(left.release)) - return left.release[:check_len] == right.release[:check_len] + if right.is_prefix: + left_release = left.release[:len(right.release)] + else: + left_release = _pad_zeros(left.release, len(right.release)) + if left_release != right_release: + return False + + if left.is_prefix or right.is_prefix: + return True + + return ( + left.pre == right.pre and + left.post == right.post and + left.dev == right.dev and + left.local == right.local + ) + +# TODO @aignas 2025-05-04: add tests for the comparison def _version_lt(left, right): + if left.epoch > right.epoch: + return False if left.epoch < right.epoch: return True - elif left.epoch > right.epoch: + + if left.is_prefix: + right_release = right.release[:len(left.release)] + else: + right_release = _pad_zeros(right.release, len(left.release)) + + if right.is_prefix: + left_release = left.release[:len(right.release)] + else: + left_release = _pad_zeros(left.release, len(right.release)) + + if left_release > right_release: return False + elif left_release < right_release: + return True + + if left.is_prefix or right.is_prefix: + return True - return left.release < right.release + return ( + left.pre < right.pre and + left.post < right.post and + left.dev < right.dev and + left.local < right.local + ) +# TODO @aignas 2025-05-04: add tests for the comparison def _version_gt(left, right): if left.epoch > right.epoch: return True @@ -565,7 +617,7 @@ def _version_gt(left, right): return left.release > right.release -def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = ""): +def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False): epoch = epoch or 0 _release = tuple([int(d) for d in release.split(".")]) pre = pre or "" @@ -580,6 +632,7 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " post = post, dev = dev, local = local, + is_prefix = is_prefix, upper = lambda: _upper( epoch = epoch, release = _release, @@ -588,14 +641,7 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " dev = dev, local = local, ), - key = lambda: ( - epoch, - _release, - pre, - post, - dev, - local, - ), + # TODO @aignas 2025-05-04: add tests for the comparison eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized ne = lambda x: not _version_eq(self, x), # buildifier: disable=uninitialized lt = lambda x: _version_lt(self, x), # buildifier: disable=uninitialized @@ -607,7 +653,9 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " return self def parse_version(version): - """Escape the version component of a filename. + """Parse a PEP4408 compliant version + + TODO: finish See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode and https://peps.python.org/pep-0440/ @@ -619,9 +667,7 @@ def parse_version(version): string containing the normalized version. """ - # TODO @aignas 2025-05-04: this is discarding '.*', but per spec this should be only done - # for public version segments - parser = _new(version.strip(" .*")) # PEP 440: Leading and Trailing Whitespace + parser = _new(version.strip(" .*")) # PEP 440: Leading and Trailing Whitespace and .* accept(parser, _is("v"), "") # PEP 440: Preceding v character parts = {} @@ -637,7 +683,15 @@ def parse_version(version): for p, fn in fns: fn(parser) parts[p] = parser.context()["norm"] - parser.context()["norm"] = "" + parser.context()["norm"] = "" # Clear out the buffer so that it is easy to separate the fields + + is_prefix = version.endswith(".*") + parts["is_prefix"] = is_prefix + if is_prefix and (parts["local"] or parts["post"] or parts["dev"] or parts["pre"]): + # local version part has been obtained, but only public segments can have prefix + # matches. Just return None. + # https://peps.python.org/pep-0440/#public-version-identifiers + return None if parser.input[parser.context()["start"]:]: # If we fail to parse the version return None From 2fc298bbc8e103ba9d8c7c6fb2dfbcc7e5a85219 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 14:12:07 -0700 Subject: [PATCH 05/57] Clarify comment about fallback behavior --- python/private/pypi/pep508_evaluate.bzl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index 60f06c2750..ee82da9105 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -366,8 +366,9 @@ def _version_expr(left, op, right): _left = parse_version(left) _right = parse_version(right) if _left == None or _right == None: - # Sometimes `platform_version` is not a true PEP440 version, - # so then we fallback to a simple string expression evaluation + # Per spec, if either can't be normalized to a version, then + # fallback to simple string comparison. Usually this is `platform_version` + # or `platform_release`, which vary depending on platform. return _env_expr(left, op, right) if op == "<": From fb92a2cd2e6ea38e6c5656794f98c3a6afc1d79f Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 14:24:42 -0700 Subject: [PATCH 06/57] factor out helper evaluate assert --- tests/pypi/pep508/evaluate_tests.bzl | 76 ++++++---------------------- 1 file changed, 16 insertions(+), 60 deletions(-) diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 952d07ab75..0bcaf8d8b7 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -19,6 +19,11 @@ load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize") # bui _tests = [] +def _check_evaluate(env, expr, expected, values, strict=True): + env.expect.where( + expression=expr, values=values + ).that_bool(evaluate(expr, env=values, strict=strict)).equals(expected) + def _tokenize_tests(env): for input, want in { "": [], @@ -82,23 +87,11 @@ def _evaluate_non_version_env_tests(env): "{} > 'osx'".format(var_name): False, "{} >= 'osx'".format(var_name): True, }.items(): - got = evaluate( - input, - env = marker_env, - ) - env.expect.where( - expr = input, - env = marker_env, - ).that_bool(got).equals(want) + _check_evaluate(env, input, want, marker_env) # Check that the non-strict eval gives us back the input when no # env is supplied. - got = evaluate( - input, - env = {}, - strict = False, - ) - env.expect.that_bool(got).equals(input.replace("'", '"')) + _check_evaluate(env, input, input.replace("'", '"'), {}, strict=False) _tests.append(_evaluate_non_version_env_tests) @@ -132,20 +125,11 @@ def _evaluate_version_env_tests(env): "{} === '3.7.9'".format(var_name): True, "{} == '3.7.9+rc2'".format(var_name): True, }.items(): # buildifier: @unsorted-dict-items - got = evaluate( - input, - env = marker_env, - ) - env.expect.that_collection((input, got)).contains_exactly((input, want)) + _check_evaluate(env, input, want, marker_env) # Check that the non-strict eval gives us back the input when no # env is supplied. - got = evaluate( - input, - env = {}, - strict = False, - ) - env.expect.that_bool(got).equals(input.replace("'", '"')) + _check_evaluate(env, input, input.replace("'", '"'), {}, strict=False) _tests.append(_evaluate_version_env_tests) @@ -155,29 +139,15 @@ def _evaluate_platform_version_is_special(env): # When the platform version is not input = "platform_version == '0'" - got = evaluate( - input, - env = marker_env, - ) - env.expect.that_collection((input, got)).contains_exactly((input, False)) + _check_evaluate(env, input, False, marker_env) # And when I compare it as string input = "'FooBar' in platform_version" - got = evaluate( - input, - env = marker_env, - ) - env.expect.that_collection((input, got)).contains_exactly((input, True)) - + _check_evaluate(env, input, True, marker_env) # Check that the non-strict eval gives us back the input when no # env is supplied. - got = evaluate( - input, - env = {}, - strict = False, - ) - env.expect.that_bool(got).equals(input.replace("'", '"')) + _check_evaluate(env, input, input.replace("'", '"'), {}, strict=False) _tests.append(_evaluate_platform_version_is_special) @@ -228,13 +198,7 @@ def _logical_expression_tests(env): "not not os_name == 'foo'": True, "not not not os_name == 'foo'": False, }.items(): # buildifier: @unsorted-dict-items - got = evaluate( - input, - env = { - "os_name": "foo", - }, - ) - env.expect.that_collection((input, got)).contains_exactly((input, want)) + _check_evaluate(env, input, want, {"os_name": "foo"}) if not input.strip("()"): # These cases will just return True, because they will be evaluated @@ -243,12 +207,7 @@ def _logical_expression_tests(env): # Check that the non-strict eval gives us back the input when no env # is supplied. - got = evaluate( - input, - env = {}, - strict = False, - ) - env.expect.that_bool(got).equals(input.replace("'", '"')) + _check_evaluate(env, input, input.replace("'", '"'), {}, strict=False) _tests.append(_logical_expression_tests) @@ -277,6 +236,7 @@ def _evaluate_partial_only_extra(env): strict = False, ) env.expect.that_bool(got).equals(want) + _check_evaluate(env, input, want, env = {"extra": extra}, strict=False) _tests.append(_evaluate_partial_only_extra) @@ -301,11 +261,7 @@ def _evaluate_with_aliases(env): }, }.items(): # buildifier: @unsorted-dict-items for input, want in tests.items(): - got = evaluate( - input, - env = pep508_env(target_platform), - ) - env.expect.that_bool(got).equals(want) + _check_evaluate(env, input, want, pep508_env(target_platform)) _tests.append(_evaluate_with_aliases) From 6d511c63d51e6c9d39b8c2c0f5f7f9a8e8d5cf43 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 14:37:11 -0700 Subject: [PATCH 07/57] add generic test for arbitrary expressions --- tests/pypi/pep508/evaluate_tests.bzl | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 0bcaf8d8b7..5ee4f18054 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -265,6 +265,18 @@ def _evaluate_with_aliases(env): _tests.append(_evaluate_with_aliases) +def _expr_case(expr, want, env): + return struct(expr=expr, want=want, env=env) + +_MISC_EXPRESSIONS = [ +] + +def _misc_expressions(env): + for case in _MISC_EXPRESSIONS: + _check_evaluation(env, case.expr, case.want, case.env) + +_tests.append(_misc_expressions) + def evaluate_test_suite(name): # buildifier: disable=function-docstring test_suite( name = name, From 1c45c9e042d401b7f4534de1178f7823c5d31902 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 14:41:21 -0700 Subject: [PATCH 08/57] add missing dep to build target --- python/private/pypi/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel index 9216134857..aacbc99f70 100644 --- a/python/private/pypi/BUILD.bazel +++ b/python/private/pypi/BUILD.bazel @@ -232,6 +232,7 @@ bzl_library( srcs = ["pep508_env.bzl"], deps = [ ":pep508_platform_bzl", + "//python/private:py_wheel_normalize_pep440_bzl", ], ) From 7fef735a4124ec983306c5847449dd347dd4339a Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 14:49:25 -0700 Subject: [PATCH 09/57] removed unused _VERSION_CMP --- python/private/pypi/pep508_evaluate.bzl | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index ee82da9105..47fd0d180e 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -22,18 +22,6 @@ load("//python/private:py_wheel_normalize_pep440.bzl", "parse_version") # The expression parsing and resolution for the PEP508 is below # -# Taken from -# https://peps.python.org/pep-0508/#grammar -# -# version_cmp = wsp* '<' | '<=' | '!=' | '==' | '>=' | '>' | '~=' | '===' -_VERSION_CMP = sorted( - [ - i.strip(" '") - for i in "'<' | '<=' | '!=' | '==' | '>=' | '>' | '~=' | '==='".split(" | ") - ], - key = lambda x: (-len(x), x), -) - _STATE = enum( STRING = "string", VAR = "var", From 68178a36892d0f79e1e5a3d09b7c929236f72ca3 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 19:59:40 -0700 Subject: [PATCH 10/57] add bzl_library for pep440 code --- python/private/BUILD.bazel | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index 9cc8ffc62c..208916b200 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -578,6 +578,11 @@ bzl_library( ], ) +bzl_library( + name = "py_wheel_normalize_pep440_bzl", + srcs = ["py_wheel_normalize_pep440.bzl"], +) + bzl_library( name = "reexports_bzl", srcs = ["reexports.bzl"], From 7c6699b56149986ef6d10663743e08fc20b4728e Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 20:06:26 -0700 Subject: [PATCH 11/57] run pre-commit formatting --- python/private/py_wheel_normalize_pep440.bzl | 2 +- python/private/pypi/pep508_evaluate.bzl | 1 - tests/pypi/pep508/evaluate_tests.bzl | 20 +++++++++++--------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index a84ee8f430..42963524d6 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -683,7 +683,7 @@ def parse_version(version): for p, fn in fns: fn(parser) parts[p] = parser.context()["norm"] - parser.context()["norm"] = "" # Clear out the buffer so that it is easy to separate the fields + parser.context()["norm"] = "" # Clear out the buffer so that it is easy to separate the fields is_prefix = version.endswith(".*") parts["is_prefix"] = is_prefix diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index 47fd0d180e..8f68ad6988 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -18,7 +18,6 @@ load("//python/private:enum.bzl", "enum") load("//python/private:py_wheel_normalize_pep440.bzl", "parse_version") - # The expression parsing and resolution for the PEP508 is below # diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 5ee4f18054..7bda05bc66 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -19,10 +19,11 @@ load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize") # bui _tests = [] -def _check_evaluate(env, expr, expected, values, strict=True): +def _check_evaluate(env, expr, expected, values, strict = True): env.expect.where( - expression=expr, values=values - ).that_bool(evaluate(expr, env=values, strict=strict)).equals(expected) + expression = expr, + values = values, + ).that_bool(evaluate(expr, env = values, strict = strict)).equals(expected) def _tokenize_tests(env): for input, want in { @@ -91,7 +92,7 @@ def _evaluate_non_version_env_tests(env): # Check that the non-strict eval gives us back the input when no # env is supplied. - _check_evaluate(env, input, input.replace("'", '"'), {}, strict=False) + _check_evaluate(env, input, input.replace("'", '"'), {}, strict = False) _tests.append(_evaluate_non_version_env_tests) @@ -129,7 +130,7 @@ def _evaluate_version_env_tests(env): # Check that the non-strict eval gives us back the input when no # env is supplied. - _check_evaluate(env, input, input.replace("'", '"'), {}, strict=False) + _check_evaluate(env, input, input.replace("'", '"'), {}, strict = False) _tests.append(_evaluate_version_env_tests) @@ -147,7 +148,7 @@ def _evaluate_platform_version_is_special(env): # Check that the non-strict eval gives us back the input when no # env is supplied. - _check_evaluate(env, input, input.replace("'", '"'), {}, strict=False) + _check_evaluate(env, input, input.replace("'", '"'), {}, strict = False) _tests.append(_evaluate_platform_version_is_special) @@ -207,7 +208,7 @@ def _logical_expression_tests(env): # Check that the non-strict eval gives us back the input when no env # is supplied. - _check_evaluate(env, input, input.replace("'", '"'), {}, strict=False) + _check_evaluate(env, input, input.replace("'", '"'), {}, strict = False) _tests.append(_logical_expression_tests) @@ -236,7 +237,7 @@ def _evaluate_partial_only_extra(env): strict = False, ) env.expect.that_bool(got).equals(want) - _check_evaluate(env, input, want, env = {"extra": extra}, strict=False) + _check_evaluate(env, input, want, env = {"extra": extra}, strict = False) _tests.append(_evaluate_partial_only_extra) @@ -266,9 +267,10 @@ def _evaluate_with_aliases(env): _tests.append(_evaluate_with_aliases) def _expr_case(expr, want, env): - return struct(expr=expr, want=want, env=env) + return struct(expr = expr.strip(), want = want, env = env) _MISC_EXPRESSIONS = [ + _expr_case('python_version == "3.*"', True, {"python_version": "3.10.1"}), ] def _misc_expressions(env): From 22310247aa592f955b2c64ae123b55083207cad9 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 20:12:18 -0700 Subject: [PATCH 12/57] fix bad evaluate call --- tests/pypi/pep508/evaluate_tests.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 7bda05bc66..0427021063 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -237,7 +237,7 @@ def _evaluate_partial_only_extra(env): strict = False, ) env.expect.that_bool(got).equals(want) - _check_evaluate(env, input, want, env = {"extra": extra}, strict = False) + _check_evaluate(env, input, want, {"extra": extra}, strict = False) _tests.append(_evaluate_partial_only_extra) @@ -275,7 +275,7 @@ _MISC_EXPRESSIONS = [ def _misc_expressions(env): for case in _MISC_EXPRESSIONS: - _check_evaluation(env, case.expr, case.want, case.env) + _check_evaluate(env, case.expr, case.want, case.env) _tests.append(_misc_expressions) From a302509bc0553666a0aae94d7b2bb2a0d7314d06 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 20:44:18 -0700 Subject: [PATCH 13/57] maybe fix == comparison wth star suffix --- python/private/py_wheel_normalize_pep440.bzl | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 42963524d6..890d1ac895 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -556,24 +556,35 @@ def _version_eq(left, right): if left.is_prefix: right_release = right.release[:len(left.release)] else: - right_release = _pad_zeros(right.release, len(left.release)) + ##right_release = _pad_zeros(right.release, len(left.release)) + right_release = right.release if right.is_prefix: left_release = left.release[:len(right.release)] else: - left_release = _pad_zeros(left.release, len(right.release)) + ##left_release = _pad_zeros(left.release, len(right.release)) + left_release = left.release + + ##print("left is prefix?", left.is_prefix) + ##print("left.release:", left.release, len(left.release)) + ##print("left_release:", left_release) + ##print("right is prefix?", right.is_prefix) + ##print("right.release:", right.release, len(right.release)) + ##print("right_release:", right_release) if left_release != right_release: return False + print("is prefix?", left.is_prefix, right.is_prefix) if left.is_prefix or right.is_prefix: return True return ( left.pre == right.pre and left.post == right.post and - left.dev == right.dev and - left.local == right.local + left.dev == right.dev + # local is ignored for == checks + ##and left.local == right.local ) # TODO @aignas 2025-05-04: add tests for the comparison From b7860aef272c5ab2182e245dc3fb0153ad2d7197 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 5 May 2025 12:09:54 +0900 Subject: [PATCH 14/57] remove 'upper' helper and simplify prefix matching --- python/private/py_wheel_normalize_pep440.bzl | 69 +++++--------------- python/private/pypi/pep508_evaluate.bzl | 32 +++++++-- tests/pypi/pep508/evaluate_tests.bzl | 2 + 3 files changed, 44 insertions(+), 59 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 890d1ac895..0e378ea9e9 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -518,28 +518,6 @@ def normalize_pep440(version): ) return parser.context()["norm"] -def _upper(*, epoch = 0, release, pre = "", post = "", dev = "", local = ""): - epoch = epoch - _release = list(release[:-1]) - pre = pre - post = post - dev = dev - local = local - - if pre or dev or local: - return _new_version( - epoch = epoch, - release = release, - ) - - _release[-1] = _release[-1] + 1 - release = ".".join([str(d) for d in _release]) - - return _new_version( - epoch = epoch, - release = release, - ) - def _pad_zeros(release, n): if len(release) >= n: return release @@ -550,35 +528,25 @@ def _pad_zeros(release, n): # TODO @aignas 2025-05-04: add tests for the comparison def _version_eq(left, right): + if left.is_prefix and right.is_prefix: + fail("Invalid comparison: both versions cannot be prefix matching") + if left.is_prefix: + return right.norm.startswith("{}.".format(left.norm)) + if right.is_prefix: + return left.norm.startswith("{}.".format(right.norm)) + if left.epoch != right.epoch: return False - if left.is_prefix: - right_release = right.release[:len(left.release)] - else: - ##right_release = _pad_zeros(right.release, len(left.release)) - right_release = right.release + ##right_release = _pad_zeros(right.release, len(left.release)) + right_release = right.release - if right.is_prefix: - left_release = left.release[:len(right.release)] - else: - ##left_release = _pad_zeros(left.release, len(right.release)) - left_release = left.release + ##left_release = _pad_zeros(left.release, len(right.release)) + left_release = left.release - ##print("left is prefix?", left.is_prefix) - ##print("left.release:", left.release, len(left.release)) - ##print("left_release:", left_release) - - ##print("right is prefix?", right.is_prefix) - ##print("right.release:", right.release, len(right.release)) - ##print("right_release:", right_release) if left_release != right_release: return False - print("is prefix?", left.is_prefix, right.is_prefix) - if left.is_prefix or right.is_prefix: - return True - return ( left.pre == right.pre and left.post == right.post and @@ -628,7 +596,7 @@ def _version_gt(left, right): return left.release > right.release -def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False): +def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): epoch = epoch or 0 _release = tuple([int(d) for d in release.split(".")]) pre = pre or "" @@ -644,14 +612,7 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " dev = dev, local = local, is_prefix = is_prefix, - upper = lambda: _upper( - epoch = epoch, - release = _release, - pre = pre, - post = post, - dev = dev, - local = local, - ), + norm = norm, # TODO @aignas 2025-05-04: add tests for the comparison eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized ne = lambda x: not _version_eq(self, x), # buildifier: disable=uninitialized @@ -679,7 +640,9 @@ def parse_version(version): """ parser = _new(version.strip(" .*")) # PEP 440: Leading and Trailing Whitespace and .* + parser_2 = _new(version.strip(" .*")) # PEP 440: Leading and Trailing Whitespace and .* accept(parser, _is("v"), "") # PEP 440: Preceding v character + accept(parser_2, _is("v"), "") # PEP 440: Preceding v character parts = {} fns = [ @@ -693,6 +656,7 @@ def parse_version(version): for p, fn in fns: fn(parser) + fn(parser_2) parts[p] = parser.context()["norm"] parser.context()["norm"] = "" # Clear out the buffer so that it is easy to separate the fields @@ -708,4 +672,5 @@ def parse_version(version): # If we fail to parse the version return None return None + parts["norm"] = parser_2.context()["norm"] return _new_version(**parts) diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index 8f68ad6988..d15042737c 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -350,6 +350,13 @@ def _version_expr(left, op, right): # > simple string equality operations return _env_expr(left, "==", right) + if left.endswith(".*") and right.endswith(".*"): + fail("PEP440: only one of the sides can have '.*' suffix: {} {} {}".format( + left, + op, + right, + )) + _left = parse_version(left) _right = parse_version(right) if _left == None or _right == None: @@ -358,7 +365,18 @@ def _version_expr(left, op, right): # or `platform_release`, which vary depending on platform. return _env_expr(left, op, right) - if op == "<": + if op == "!=": + return _left.ne(_right) + elif op == "==": + # Matching of major, minor, patch only + return _left.eq(_right) + elif left.endswith(".*") or right.endswith(".*"): + fail("PEP440: only '==' and '!=' operators can use prefix matching: {} {} {}".format( + left, + op, + right, + )) + elif op == "<": return _left.lt(_right) elif op == ">": return _left.gt(_right) @@ -366,13 +384,13 @@ def _version_expr(left, op, right): return _left.le(_right) elif op == ">=": return _left.ge(_right) - elif op == "!=": - return _left.ne(_right) - elif op == "==": - # Matching of major, minor, patch only - return _left.eq(_right) elif op == "~=": - return _left.ge(_right) and _left.lt(_right.upper()) + # https://peps.python.org/pep-0440/#compatible-release + # Note, the ~= operator can be also expressed as: + # >= V.N, == V.* + head, _, _ = right.partition(".") + _right_star = parse_version("{}.*".format(head)) + return _left.ge(_right) and _left.eq(_right_star) else: return False # Let's just ignore the invalid ops diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 0427021063..97652572d5 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -271,6 +271,8 @@ def _expr_case(expr, want, env): _MISC_EXPRESSIONS = [ _expr_case('python_version == "3.*"', True, {"python_version": "3.10.1"}), + _expr_case('python_version != "3.10.*"', False, {"python_version": "3.10.1"}), + _expr_case('python_version != "3.11.*"', True, {"python_version": "3.10.1"}), ] def _misc_expressions(env): From 0666d56af3de1daaa3845233c9d8530fbb3ba2b6 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 5 May 2025 12:17:11 +0900 Subject: [PATCH 15/57] handle zero padding correctly --- python/private/py_wheel_normalize_pep440.bzl | 14 ++++++-------- tests/pypi/pep508/evaluate_tests.bzl | 2 ++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 0e378ea9e9..3a142109ca 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -519,11 +519,11 @@ def normalize_pep440(version): return parser.context()["norm"] def _pad_zeros(release, n): - if len(release) >= n: + padding = n - len(release) + if padding <= 0: return release - release = list(release) - release += [0] * len(release) + release = list(release) + [0] * padding return tuple(release) # TODO @aignas 2025-05-04: add tests for the comparison @@ -538,11 +538,9 @@ def _version_eq(left, right): if left.epoch != right.epoch: return False - ##right_release = _pad_zeros(right.release, len(left.release)) - right_release = right.release - - ##left_release = _pad_zeros(left.release, len(right.release)) - left_release = left.release + release_len = max(len(left.release), len(right.release)) + left_release = _pad_zeros(left.release, release_len) + right_release = _pad_zeros(right.release, release_len) if left_release != right_release: return False diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 97652572d5..4ffc09ac8c 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -273,6 +273,8 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version == "3.*"', True, {"python_version": "3.10.1"}), _expr_case('python_version != "3.10.*"', False, {"python_version": "3.10.1"}), _expr_case('python_version != "3.11.*"', True, {"python_version": "3.10.1"}), + _expr_case('python_version != "3.10"', False, {"python_version": "3.10.0"}), + _expr_case('python_version == "3.10"', True, {"python_version": "3.10.0"}), ] def _misc_expressions(env): From a98f65427a39f873f7350b4ae586d5dc9e34c43f Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 5 May 2025 20:06:06 +0900 Subject: [PATCH 16/57] implement more of the test cases for the gt operator --- python/private/py_wheel_normalize_pep440.bzl | 43 +++++++++++++------- tests/pypi/pep508/evaluate_tests.bzl | 9 ++++ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 3a142109ca..5cd9cc81d8 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -560,24 +560,15 @@ def _version_lt(left, right): if left.epoch < right.epoch: return True - if left.is_prefix: - right_release = right.release[:len(left.release)] - else: - right_release = _pad_zeros(right.release, len(left.release)) - - if right.is_prefix: - left_release = left.release[:len(right.release)] - else: - left_release = _pad_zeros(left.release, len(right.release)) + release_len = max(len(left.release), len(right.release)) + left_release = _pad_zeros(left.release, release_len) + right_release = _pad_zeros(right.release, release_len) if left_release > right_release: return False elif left_release < right_release: return True - if left.is_prefix or right.is_prefix: - return True - return ( left.pre < right.pre and left.post < right.post and @@ -592,13 +583,37 @@ def _version_gt(left, right): elif left.epoch < right.epoch: return False - return left.release > right.release + release_len = max(len(left.release), len(right.release)) + left_release = _pad_zeros(left.release, release_len) + right_release = _pad_zeros(right.release, release_len) + + # It cannot be pre and post release at the same time, so the following should be fine + if not left.post and right.post: + return False + + if left_release > right_release: + return True + elif left_release < right_release: + return False + + if right.post: + if left.post > right.post: + return True + elif left.post < right.post: + return False + + return False def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): epoch = epoch or 0 _release = tuple([int(d) for d in release.split(".")]) pre = pre or "" - post = post or "" + if post: + if not post.startswith(".post"): + fail("post release identifier must start with '.post', got: {}".format(post)) + post = int(post[len(".post"):]) + else: + post = None dev = dev or "" local = local or "" diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 4ffc09ac8c..30b72b8111 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -275,6 +275,15 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version != "3.11.*"', True, {"python_version": "3.10.1"}), _expr_case('python_version != "3.10"', False, {"python_version": "3.10.0"}), _expr_case('python_version == "3.10"', True, {"python_version": "3.10.0"}), + # Taken from spec: https://peps.python.org/pep-0440/#exclusive-ordered-comparison + _expr_case('python_version > "1.7"', True, {"python_version": "1.7.1"}), + _expr_case('python_version > "1.7"', False, {"python_version": "1.7.0.post0"}), + _expr_case('python_version > "1.7"', True, {"python_version": "1.7.1"}), + _expr_case('python_version > "1.7.post2"', False, {"python_version": "1.7.0"}), + _expr_case('python_version > "1.7.post2"', True, {"python_version": "1.7.post3"}), + _expr_case('python_version > "1.7.post2"', False, {"python_version": "1.7.1"}), + _expr_case('python_version > "1.7+local"', True, {"python_version": "1.7.1"}), + # TODO @aignas 2025-05-05: add tests for pre-releases ] def _misc_expressions(env): From db949b2ebeab4508590c92fd5b37666459dac624 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 10:28:07 +0900 Subject: [PATCH 17/57] fix tests --- python/private/py_wheel_normalize_pep440.bzl | 11 ++++++----- tests/pypi/pep508/evaluate_tests.bzl | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 5cd9cc81d8..3cfc387430 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -587,17 +587,18 @@ def _version_gt(left, right): left_release = _pad_zeros(left.release, release_len) right_release = _pad_zeros(right.release, release_len) - # It cannot be pre and post release at the same time, so the following should be fine - if not left.post and right.post: - return False - if left_release > right_release: return True elif left_release < right_release: return False + # the release is equal, check for post version if right.post: - if left.post > right.post: + if not left.post: + # PEP440: The exclusive ordered comparison >V MUST NOT allow a post-release of the + # given version unless V itself is a post release. + return False + elif left.post > right.post: return True elif left.post < right.post: return False diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 30b72b8111..18f0b1fc95 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -279,10 +279,10 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version > "1.7"', True, {"python_version": "1.7.1"}), _expr_case('python_version > "1.7"', False, {"python_version": "1.7.0.post0"}), _expr_case('python_version > "1.7"', True, {"python_version": "1.7.1"}), - _expr_case('python_version > "1.7.post2"', False, {"python_version": "1.7.0"}), + _expr_case('python_version > "1.7.post2"', True, {"python_version": "1.7.1"}), _expr_case('python_version > "1.7.post2"', True, {"python_version": "1.7.post3"}), - _expr_case('python_version > "1.7.post2"', False, {"python_version": "1.7.1"}), - _expr_case('python_version > "1.7+local"', True, {"python_version": "1.7.1"}), + _expr_case('python_version > "1.7.post2"', False, {"python_version": "1.7.0"}), + _expr_case('python_version > "1.7.1+local"', False, {"python_version": "1.7.1"}), # TODO @aignas 2025-05-05: add tests for pre-releases ] From fe0ca65e57c6a6b2896ccefa34dc158656f6ba9b Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 11:32:43 +0900 Subject: [PATCH 18/57] Add more implementation for the version comparisons --- python/private/py_wheel_normalize_pep440.bzl | 78 +++++++++++++++----- python/private/pypi/pep508_evaluate.bzl | 3 + tests/pypi/pep508/evaluate_tests.bzl | 66 ++++++++++++++++- 3 files changed, 128 insertions(+), 19 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 3cfc387430..506afdddab 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -553,11 +553,10 @@ def _version_eq(left, right): ##and left.local == right.local ) -# TODO @aignas 2025-05-04: add tests for the comparison def _version_lt(left, right): if left.epoch > right.epoch: return False - if left.epoch < right.epoch: + elif left.epoch < right.epoch: return True release_len = max(len(left.release), len(right.release)) @@ -569,14 +568,22 @@ def _version_lt(left, right): elif left_release < right_release: return True - return ( - left.pre < right.pre and - left.post < right.post and - left.dev < right.dev and - left.local < right.local - ) + # the release is equal, check for pre version + if right.pre: + if left.pre != None: + # PEP440: The exclusive ordered comparison right.pre: + return False + elif left.pre < right.pre: + return True + elif left.pre: + return True + + return False -# TODO @aignas 2025-05-04: add tests for the comparison def _version_gt(left, right): if left.epoch > right.epoch: return True @@ -603,19 +610,39 @@ def _version_gt(left, right): elif left.post < right.post: return False + if right.pre: + return True + return False def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): epoch = epoch or 0 _release = tuple([int(d) for d in release.split(".")]) - pre = pre or "" + + if pre: + if pre.startswith("rc"): + prefix = "rc" + else: + prefix = pre[0] + + pre = (prefix, int(pre[len(prefix):])) + else: + pre = None + if post: if not post.startswith(".post"): fail("post release identifier must start with '.post', got: {}".format(post)) post = int(post[len(".post"):]) else: post = None - dev = dev or "" + + if dev: + if not dev.startswith(".dev"): + fail("dev release identifier must start with '.dev', got: {}".format(dev)) + dev = int(dev[len(".dev"):]) + else: + dev = None + local = local or "" self = struct( @@ -634,11 +661,20 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized le = lambda x: not _version_gt(self, x), # buildifier: disable=uninitialized ge = lambda x: not _version_lt(self, x), # buildifier: disable=uninitialized + str = lambda: norm, + key = lambda: ( + epoch, + release, + pre or ("release",), + post if post != None else -1, + dev if dev != None else -1, + local, + ), ) return self -def parse_version(version): +def parse_version(version, strict = False): """Parse a PEP4408 compliant version TODO: finish @@ -648,13 +684,14 @@ def parse_version(version): Args: version: version string to be normalized according to PEP 440. + strict: fail if the version is invalid. Returns: string containing the normalized version. """ - parser = _new(version.strip(" .*")) # PEP 440: Leading and Trailing Whitespace and .* - parser_2 = _new(version.strip(" .*")) # PEP 440: Leading and Trailing Whitespace and .* + parser = _new(version.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* + parser_2 = _new(version.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* accept(parser, _is("v"), "") # PEP 440: Preceding v character accept(parser_2, _is("v"), "") # PEP 440: Preceding v character @@ -677,12 +714,19 @@ def parse_version(version): is_prefix = version.endswith(".*") parts["is_prefix"] = is_prefix if is_prefix and (parts["local"] or parts["post"] or parts["dev"] or parts["pre"]): - # local version part has been obtained, but only public segments can have prefix - # matches. Just return None. + if strict: + 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 - if parser.input[parser.context()["start"]:]: + if parser_2.input[parser.context()["start"]:]: + if strict: + fail( + "Failed to parse PEP 440 version identifier '%s'." % parser.input, + "Parse error at '%s'" % parser.input[parser.context()["start"]:], + ) + # If we fail to parse the version return None return None diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index d15042737c..304419435b 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -18,6 +18,9 @@ load("//python/private:enum.bzl", "enum") load("//python/private:py_wheel_normalize_pep440.bzl", "parse_version") +# TODO @aignas 2025-05-06: this is exposed for tests only +version = parse_version + # The expression parsing and resolution for the PEP508 is below # diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 18f0b1fc95..d81363b0ed 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -15,7 +15,7 @@ load("@rules_testing//lib:test_suite.bzl", "test_suite") load("//python/private/pypi:pep508_env.bzl", pep508_env = "env") # buildifier: disable=bzl-visibility -load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize") # buildifier: disable=bzl-visibility +load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize", "version") # buildifier: disable=bzl-visibility _tests = [] @@ -283,7 +283,15 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version > "1.7.post2"', True, {"python_version": "1.7.post3"}), _expr_case('python_version > "1.7.post2"', False, {"python_version": "1.7.0"}), _expr_case('python_version > "1.7.1+local"', False, {"python_version": "1.7.1"}), - # TODO @aignas 2025-05-05: add tests for pre-releases + _expr_case('python_version > "1.7.1+local"', True, {"python_version": "1.7.2"}), + _expr_case('python_version < "1.7.1"', False, {"python_version": "1.7.2"}), + _expr_case('python_version < "1.7.3"', True, {"python_version": "1.7.2"}), + _expr_case('python_version < "1.7.1"', True, {"python_version": "1.7"}), + _expr_case('python_version < "1.7.1-rc2"', True, {"python_version": "1.7"}), + _expr_case('python_version < "1.7-rc2"', False, {"python_version": "1.7"}), + _expr_case('python_version < "1.7-rc2"', True, {"python_version": "1.7-rc1"}), + _expr_case('python_version < "1.7-rc2"', False, {"python_version": "1.7-rc3"}), + _expr_case('python_version < "1.7-rc12"', True, {"python_version": "1.7-rc3"}), ] def _misc_expressions(env): @@ -292,6 +300,60 @@ def _misc_expressions(env): _tests.append(_misc_expressions) +def _test_ordering(env): + want = [ + # 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.dev", + "0.9.12", + "1.0.0-alpha", + "1.0.0-alpha1", + "1.0.0-beta", + "1.0.0-beta.dev", + "1.0.0-beta2", + "1.0.0-beta11", + "1.0.0-rc1", + "1.0.0-rc2.dev", + "1.0.0-rc2", + "1.0.0", + # Also handle missing minor and patch version strings + "2.0", + "3", + ] + + for lower, higher in zip(want[:-1], want[1:]): + lower = version(lower, strict = True) + higher = version(higher, strict = True) + + if not lower.key() < higher.key(): + env.fail("Expected '{}'.key() to be smaller than '{}'.key(), but got otherwise".format( + lower.str(), + higher.str(), + )) + + if not lower.lt(higher): + env.fail("Expected '{}' to be smaller than '{}', but got otherwise".format( + lower.str(), + higher.str(), + )) + + if not (higher.key() > lower.key()): + env.fail("Expected '{}'.key() to be greater than '{}'.key(), but got otherwise".format( + higher.str(), + lower.str(), + )) + + if not (higher.gt(lower)): + env.fail("Expected '{}' to be greater than '{}', but got otherwise".format( + higher.str(), + lower.str(), + )) + +_tests.append(_test_ordering) + def evaluate_test_suite(name): # buildifier: disable=function-docstring test_suite( name = name, From f357a326ea28c293074f035d49fc9cca1b3bd6b2 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 11:41:28 +0900 Subject: [PATCH 19/57] add tests from the page --- python/private/py_wheel_normalize_pep440.bzl | 19 +++++++-- tests/pypi/pep508/evaluate_tests.bzl | 41 ++++++++++---------- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 506afdddab..626d8dc834 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -569,8 +569,8 @@ def _version_lt(left, right): return True # the release is equal, check for pre version - if right.pre: - if left.pre != None: + if right.pre != None: + if left.pre == None: # PEP440: The exclusive ordered comparison right.dev: + return False + elif left.dev < right.dev: + return True + elif left.dev != None: return True return False diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index d81363b0ed..be9244f063 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -302,26 +302,27 @@ _tests.append(_misc_expressions) def _test_ordering(env): want = [ - # 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.dev", - "0.9.12", - "1.0.0-alpha", - "1.0.0-alpha1", - "1.0.0-beta", - "1.0.0-beta.dev", - "1.0.0-beta2", - "1.0.0-beta11", - "1.0.0-rc1", - "1.0.0-rc2.dev", - "1.0.0-rc2", - "1.0.0", - # Also handle missing minor and patch version strings - "2.0", - "3", + # Taken from https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering + "1.dev0", + "1.0.dev456", + "1.0a1", + "1.0a2.dev456", + "1.0a12.dev456", + "1.0a12", + "1.0b1.dev456", + "1.0b2", + "1.0b2.post345.dev456", + "1.0b2.post345", + "1.0rc1.dev456", + "1.0rc1", + "1.0", + "1.0+abc.5", + "1.0+abc.7", + "1.0+5", + "1.0.post456.dev34", + "1.0.post456", + "1.0.15", + "1.1.dev1", ] for lower, higher in zip(want[:-1], want[1:]): From 67c6e49eb85d9a3d4d53fee17f9fc31331b5c80e Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 14:02:54 +0900 Subject: [PATCH 20/57] handle post.dev versions --- python/private/py_wheel_normalize_pep440.bzl | 44 +++++++++++++------- tests/pypi/pep508/evaluate_tests.bzl | 34 ++++++++------- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 626d8dc834..ef8a079bfc 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -568,6 +568,15 @@ def _version_lt(left, right): elif left_release < right_release: return True + if left.post == None and right.post == None: + pass + elif left.post == None: + return True + elif right.post == None: + return False + elif left.post > right.post: + return False + # the release is equal, check for pre version if right.pre != None: if left.pre == None: @@ -612,19 +621,23 @@ def _version_gt(left, right): elif left_release < right_release: return False - # the release is equal, check for post version - if right.post: - if not left.post: - # PEP440: The exclusive ordered comparison >V MUST NOT allow a post-release of the - # given version unless V itself is a post release. - return False - elif left.post > right.post: - return True - elif left.post < right.post: - return False + if left.post == None and right.post == None: + pass + elif left.post == None: + return False + elif right.post == None: + return True + elif left.post < right.post: + return False - if right.pre: + if left.dev == None and right.dev == None: + pass + elif left.dev == None: return True + elif left.dev == None: + return False + elif left.dev < right.dev: + return False return False @@ -678,10 +691,11 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " key = lambda: ( epoch, release, - pre or ("release",), - post if post != None else -1, - dev if dev != None else -1, - local, + ( + -1 if post == None else post, + dev == None, + -1 if dev == None else dev, + ), ), ) diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index be9244f063..bfab2be7ad 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -303,22 +303,22 @@ _tests.append(_misc_expressions) def _test_ordering(env): want = [ # Taken from https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering - "1.dev0", - "1.0.dev456", - "1.0a1", - "1.0a2.dev456", - "1.0a12.dev456", - "1.0a12", - "1.0b1.dev456", - "1.0b2", - "1.0b2.post345.dev456", - "1.0b2.post345", - "1.0rc1.dev456", - "1.0rc1", + # "1.dev0", + # "1.0.dev456", + # "1.0a1", + # "1.0a2.dev456", + # "1.0a12.dev456", + # "1.0a12", + # "1.0b1.dev456", + # "1.0b2", + # "1.0b2.post345.dev456", + # "1.0b2.post345", + # "1.0rc1.dev456", + # "1.0rc1", "1.0", - "1.0+abc.5", - "1.0+abc.7", - "1.0+5", + #"1.0+abc.5", + #"1.0+abc.7", + #"1.0+5", "1.0.post456.dev34", "1.0.post456", "1.0.15", @@ -330,9 +330,11 @@ def _test_ordering(env): higher = version(higher, strict = True) if not lower.key() < higher.key(): - env.fail("Expected '{}'.key() to be smaller than '{}'.key(), but got otherwise".format( + env.fail("Expected '{}'.key() to be smaller than '{}'.key(), but got otherwise: {} > {}".format( lower.str(), higher.str(), + lower.key(), + higher.key(), )) if not lower.lt(higher): From 4db2ff709c2e8e45fc19b25f6cc4a2fa5bb42f70 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 14:44:47 +0900 Subject: [PATCH 21/57] handle pre-release ordering --- python/private/py_wheel_normalize_pep440.bzl | 26 +++++++++++++++++--- tests/pypi/pep508/evaluate_tests.bzl | 16 ++++++------ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index ef8a079bfc..929a7a478a 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -621,6 +621,20 @@ def _version_gt(left, right): elif left_release < right_release: return False + if left.pre == None and right.pre == None: + # both are not pre-releases + pass + elif left.pre == None: + # only right pre-release + return True + elif right.pre == None: + # only right pre-release + return False + elif left.pre < right.pre: + return False + else: + return True + if left.post == None and right.post == None: pass elif left.post == None: @@ -669,7 +683,11 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " else: dev = None - local = local or "" + if local: + local = local.lstrip("+") + local = tuple([int(part) if part.isdigit() else part for part in local.split(".")]) + else: + local = None self = struct( epoch = epoch, @@ -690,11 +708,13 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " str = lambda: norm, key = lambda: ( epoch, - release, + _release, ( -1 if post == None else post, + pre == None, + pre, dev == None, - -1 if dev == None else dev, + dev, ), ), ) diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index bfab2be7ad..af9d189651 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -305,16 +305,16 @@ def _test_ordering(env): # Taken from https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering # "1.dev0", # "1.0.dev456", - # "1.0a1", - # "1.0a2.dev456", - # "1.0a12.dev456", - # "1.0a12", - # "1.0b1.dev456", - # "1.0b2", + "1.0a1", + "1.0a2.dev456", + "1.0a12.dev456", + "1.0a12", + "1.0b1.dev456", + "1.0b2", # "1.0b2.post345.dev456", # "1.0b2.post345", - # "1.0rc1.dev456", - # "1.0rc1", + "1.0rc1.dev456", + "1.0rc1", "1.0", #"1.0+abc.5", #"1.0+abc.7", From fef7a3c4e7a3aca2bfc0aba40ef7eb04c51c44a7 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 14:49:08 +0900 Subject: [PATCH 22/57] handle dev --- python/private/py_wheel_normalize_pep440.bzl | 72 +------------------- tests/pypi/pep508/evaluate_tests.bzl | 2 +- 2 files changed, 3 insertions(+), 71 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 929a7a478a..d8f9cb7428 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -568,43 +568,7 @@ def _version_lt(left, right): elif left_release < right_release: return True - if left.post == None and right.post == None: - pass - elif left.post == None: - return True - elif right.post == None: - return False - elif left.post > right.post: - return False - - # the release is equal, check for pre version - if right.pre != None: - if left.pre == None: - # PEP440: The exclusive ordered comparison right.pre: - return False - elif left.pre < right.pre: - return True - elif left.pre != None: - return True - - if right.dev != None: - if left.dev == None: - # PEP440: The exclusive ordered comparison right.dev: - return False - elif left.dev < right.dev: - return True - elif left.dev != None: - return True - - return False + return left.key() < right.key() def _version_gt(left, right): if left.epoch > right.epoch: @@ -621,39 +585,7 @@ def _version_gt(left, right): elif left_release < right_release: return False - if left.pre == None and right.pre == None: - # both are not pre-releases - pass - elif left.pre == None: - # only right pre-release - return True - elif right.pre == None: - # only right pre-release - return False - elif left.pre < right.pre: - return False - else: - return True - - if left.post == None and right.post == None: - pass - elif left.post == None: - return False - elif right.post == None: - return True - elif left.post < right.post: - return False - - if left.dev == None and right.dev == None: - pass - elif left.dev == None: - return True - elif left.dev == None: - return False - elif left.dev < right.dev: - return False - - return False + return left.key() > right.key() def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): epoch = epoch or 0 diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index af9d189651..949ae3242a 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -303,7 +303,7 @@ _tests.append(_misc_expressions) def _test_ordering(env): want = [ # Taken from https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering - # "1.dev0", + "1.dev0", # "1.0.dev456", "1.0a1", "1.0a2.dev456", From 1872cbc1c912b755cdd9c4eae8088150845a0ca5 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 16:28:54 +0900 Subject: [PATCH 23/57] finish implementing the correct ordering for most versions --- python/private/py_wheel_normalize_pep440.bzl | 34 +++++++++++++++++--- tests/pypi/pep508/evaluate_tests.bzl | 8 +++-- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index d8f9cb7428..214e3d2db6 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -587,6 +587,13 @@ def _version_gt(left, right): return left.key() > right.key() +def _first_non_none(*args): + for arg in args: + if arg != None: + return arg + + return None + def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): epoch = epoch or 0 _release = tuple([int(d) for d in release.split(".")]) @@ -617,6 +624,8 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " if local: local = local.lstrip("+") + + # If the part is numerical, handle it as a number local = tuple([int(part) if part.isdigit() else part for part in local.split(".")]) else: local = None @@ -641,13 +650,28 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " key = lambda: ( epoch, _release, - ( - -1 if post == None else post, - pre == None, + # PEP440 Within a pre-release, post-release or development release segment with + # a shared prefix, ordering MUST be by the value of the numeric component. + # PEP440 release ordering: .devN, aN, bN, rcN, , .postN + # We choose to first match the pre-release, then post release, then dev and + # then stable + _first_non_none( pre, - dev == None, - dev, + # We choose `~` since almost all of the ASCII characters will be before + # it. Use `ord` and `chr` functions to find a good value. + ("~", post) if post != None else None, + ("", dev) if dev != None else None, + # 'z' is just a character that goes after "rc", + ("z", 0), + ), + # PEP440 - pre-release ordering: .devN, , .postN + _first_non_none( + ("~", post) if post != None else None, + ("", dev) if dev != None else None, + ("z", 0), ), + # PEP440 - post release ordering: .devN, + ("", dev) if dev != None else ("~",), ), ) diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 949ae3242a..67bcfcd906 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -304,15 +304,17 @@ def _test_ordering(env): want = [ # Taken from https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering "1.dev0", - # "1.0.dev456", + "1.0.dev456", "1.0a1", "1.0a2.dev456", "1.0a12.dev456", "1.0a12", "1.0b1.dev456", + "1.0b1.dev457", "1.0b2", - # "1.0b2.post345.dev456", - # "1.0b2.post345", + "1.0b2.post345.dev456", + "1.0b2.post345.dev457", + "1.0b2.post345", "1.0rc1.dev456", "1.0rc1", "1.0", From 5d7fc1a8bc6b3aabcab74139a0fbd0075cbad437 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 16:51:44 +0900 Subject: [PATCH 24/57] Implement local version matching --- python/private/py_wheel_normalize_pep440.bzl | 2 ++ tests/pypi/pep508/evaluate_tests.bzl | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 214e3d2db6..fe7b661363 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -664,6 +664,8 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " # 'z' is just a character that goes after "rc", ("z", 0), ), + # PEP440 local versions go before post versions + tuple([(type(item) == "int", item) for item in local or []]), # PEP440 - pre-release ordering: .devN, , .postN _first_non_none( ("~", post) if post != None else None, diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 67bcfcd906..2b562e2cd2 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -318,9 +318,9 @@ def _test_ordering(env): "1.0rc1.dev456", "1.0rc1", "1.0", - #"1.0+abc.5", - #"1.0+abc.7", - #"1.0+5", + "1.0+abc.5", + "1.0+abc.7", + "1.0+5", "1.0.post456.dev34", "1.0.post456", "1.0.15", From 3267dad652622689e28c88c826c85fdcfab58e93 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 17:06:19 +0900 Subject: [PATCH 25/57] simplify the key test --- python/private/py_wheel_normalize_pep440.bzl | 62 +++++++++++--------- tests/pypi/pep508/evaluate_tests.bzl | 18 ------ 2 files changed, 34 insertions(+), 46 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index fe7b661363..807d7ad34f 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -594,6 +594,32 @@ def _first_non_none(*args): return None +def _key(self, release_key = ("z",)): + """This function returns a tuple that can be used in 'sorted' calls. + + This implements the PEP440 version sorting. + """ + return ( + self.epoch, + self.release, + # PEP440 Within a pre-release, post-release or development release segment with + # a shared prefix, ordering MUST be by the value of the numeric component. + # PEP440 release ordering: .devN, aN, bN, rcN, , .postN + # We choose to first match the pre-release, then post release, then dev and + # then stable + _first_non_none(self.pre, self.post, self.dev, release_key), + # PEP440 local versions go before post versions + tuple([(type(item) == "int", item) for item in self.local or []]), + # PEP440 - pre-release ordering: .devN, , .postN + _first_non_none( + self.post, + self.dev, + release_key, + ), + # PEP440 - post release ordering: .devN, + self.dev or release_key, + ) + def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): epoch = epoch or 0 _release = tuple([int(d) for d in release.split(".")]) @@ -612,6 +638,10 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " if not post.startswith(".post"): fail("post release identifier must start with '.post', got: {}".format(post)) post = int(post[len(".post"):]) + + # We choose `~` since almost all of the ASCII characters will be before + # it. Use `ord` and `chr` functions to find a good value. + post = ("~", post) else: post = None @@ -619,6 +649,9 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " if not dev.startswith(".dev"): fail("dev release identifier must start with '.dev', got: {}".format(dev)) dev = int(dev[len(".dev"):]) + + # Empty string goes first when comparing + dev = ("", dev) else: dev = None @@ -647,34 +680,7 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " le = lambda x: not _version_gt(self, x), # buildifier: disable=uninitialized ge = lambda x: not _version_lt(self, x), # buildifier: disable=uninitialized str = lambda: norm, - key = lambda: ( - epoch, - _release, - # PEP440 Within a pre-release, post-release or development release segment with - # a shared prefix, ordering MUST be by the value of the numeric component. - # PEP440 release ordering: .devN, aN, bN, rcN, , .postN - # We choose to first match the pre-release, then post release, then dev and - # then stable - _first_non_none( - pre, - # We choose `~` since almost all of the ASCII characters will be before - # it. Use `ord` and `chr` functions to find a good value. - ("~", post) if post != None else None, - ("", dev) if dev != None else None, - # 'z' is just a character that goes after "rc", - ("z", 0), - ), - # PEP440 local versions go before post versions - tuple([(type(item) == "int", item) for item in local or []]), - # PEP440 - pre-release ordering: .devN, , .postN - _first_non_none( - ("~", post) if post != None else None, - ("", dev) if dev != None else None, - ("z", 0), - ), - # PEP440 - post release ordering: .devN, - ("", dev) if dev != None else ("~",), - ), + key = lambda: _key(self), # buildifier: disable=uninitialized ) return self diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 2b562e2cd2..ced9007c22 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -339,24 +339,6 @@ def _test_ordering(env): higher.key(), )) - if not lower.lt(higher): - env.fail("Expected '{}' to be smaller than '{}', but got otherwise".format( - lower.str(), - higher.str(), - )) - - if not (higher.key() > lower.key()): - env.fail("Expected '{}'.key() to be greater than '{}'.key(), but got otherwise".format( - higher.str(), - lower.str(), - )) - - if not (higher.gt(lower)): - env.fail("Expected '{}' to be greater than '{}', but got otherwise".format( - higher.str(), - lower.str(), - )) - _tests.append(_test_ordering) def evaluate_test_suite(name): # buildifier: disable=function-docstring From 704fa64a088926d7535ad0be8032fea38750a14e Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 17:26:27 +0900 Subject: [PATCH 26/57] add notes --- python/private/py_wheel_normalize_pep440.bzl | 54 ++++++++++++++++---- tests/pypi/pep508/evaluate_tests.bzl | 10 ++-- 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 807d7ad34f..e3e046d79c 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -526,7 +526,6 @@ def _pad_zeros(release, n): release = list(release) + [0] * padding return tuple(release) -# TODO @aignas 2025-05-04: add tests for the comparison def _version_eq(left, right): if left.is_prefix and right.is_prefix: fail("Invalid comparison: both versions cannot be prefix matching") @@ -568,7 +567,14 @@ def _version_lt(left, right): elif left_release < right_release: return True - return left.key() < right.key() + # From PEP440, this is not a simple ordering check and we need to check the version + # semantically: + # * The exclusive ordered comparison right.epoch: @@ -585,7 +591,30 @@ def _version_gt(left, right): elif left_release < right_release: return False - return left.key() > right.key() + # From PEP440, this is not a simple ordering check and we need to check the version + # semantically: + # * The exclusive ordered comparison >V MUST NOT allow a post-release of the given version + # unless V itself is a post release. + # + # * The exclusive ordered comparison >V MUST NOT match a local version of the specified + # version. + + if left.post and right.post: + return left.post > right.post + else: + # ignore the left.post if right is not a post if right is a post, then this evaluates to + # False anyway. + return False + +def _version_ge(left, right): + # PEP440: simple order check + # https://peps.python.org/pep-0440/#inclusive-ordered-comparison + return left.key(local = False) >= right.key(local = False) + +def _version_le(left, right): + # PEP440: simple order check + # https://peps.python.org/pep-0440/#inclusive-ordered-comparison + return left.key(local = False) <= right.key(local = False) def _first_non_none(*args): for arg in args: @@ -594,11 +623,14 @@ def _first_non_none(*args): return None -def _key(self, release_key = ("z",)): +def _key(self, *, local, release_key = ("z",)): """This function returns a tuple that can be used in 'sorted' calls. This implements the PEP440 version sorting. """ + local = self.local if local else [] + local = local or [] + return ( self.epoch, self.release, @@ -609,7 +641,7 @@ def _key(self, release_key = ("z",)): # then stable _first_non_none(self.pre, self.post, self.dev, release_key), # PEP440 local versions go before post versions - tuple([(type(item) == "int", item) for item in self.local or []]), + tuple([(type(item) == "int", item) for item in local]), # PEP440 - pre-release ordering: .devN, , .postN _first_non_none( self.post, @@ -672,15 +704,14 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " local = local, is_prefix = is_prefix, norm = norm, - # TODO @aignas 2025-05-04: add tests for the comparison eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized ne = lambda x: not _version_eq(self, x), # buildifier: disable=uninitialized lt = lambda x: _version_lt(self, x), # buildifier: disable=uninitialized gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized - le = lambda x: not _version_gt(self, x), # buildifier: disable=uninitialized - ge = lambda x: not _version_lt(self, x), # buildifier: disable=uninitialized + le = lambda x: _version_le(self, x), # buildifier: disable=uninitialized + ge = lambda x: _version_ge(self, x), # buildifier: disable=uninitialized str = lambda: norm, - key = lambda: _key(self), # buildifier: disable=uninitialized + key = lambda *, local = True: _key(self, local = local), # buildifier: disable=uninitialized ) return self @@ -688,7 +719,7 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " def parse_version(version, strict = False): """Parse a PEP4408 compliant version - TODO: finish + TODO @aignas 2025-05-06: where should this go? See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode and https://peps.python.org/pep-0440/ @@ -702,6 +733,9 @@ def parse_version(version, strict = False): """ parser = _new(version.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* + + # TODO @aignas 2025-05-06: Remove this usage of a second parser just to get the normalized + # version. parser_2 = _new(version.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* accept(parser, _is("v"), "") # PEP 440: Preceding v character accept(parser_2, _is("v"), "") # PEP 440: Preceding v character diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index ced9007c22..6608a6a986 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -275,6 +275,7 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version != "3.11.*"', True, {"python_version": "3.10.1"}), _expr_case('python_version != "3.10"', False, {"python_version": "3.10.0"}), _expr_case('python_version == "3.10"', True, {"python_version": "3.10.0"}), + # Cases for the '>' operator # Taken from spec: https://peps.python.org/pep-0440/#exclusive-ordered-comparison _expr_case('python_version > "1.7"', True, {"python_version": "1.7.1"}), _expr_case('python_version > "1.7"', False, {"python_version": "1.7.0.post0"}), @@ -284,14 +285,13 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version > "1.7.post2"', False, {"python_version": "1.7.0"}), _expr_case('python_version > "1.7.1+local"', False, {"python_version": "1.7.1"}), _expr_case('python_version > "1.7.1+local"', True, {"python_version": "1.7.2"}), + # Extra cases for the '<' operator _expr_case('python_version < "1.7.1"', False, {"python_version": "1.7.2"}), _expr_case('python_version < "1.7.3"', True, {"python_version": "1.7.2"}), _expr_case('python_version < "1.7.1"', True, {"python_version": "1.7"}), - _expr_case('python_version < "1.7.1-rc2"', True, {"python_version": "1.7"}), - _expr_case('python_version < "1.7-rc2"', False, {"python_version": "1.7"}), - _expr_case('python_version < "1.7-rc2"', True, {"python_version": "1.7-rc1"}), - _expr_case('python_version < "1.7-rc2"', False, {"python_version": "1.7-rc3"}), - _expr_case('python_version < "1.7-rc12"', True, {"python_version": "1.7-rc3"}), + _expr_case('python_version < "1.7.1"', False, {"python_version": "1.7.1-rc2"}), + _expr_case('python_version < "1.7.1-rc3"', True, {"python_version": "1.7.1-rc2"}), + _expr_case('python_version < "1.7.1-rc1"', False, {"python_version": "1.7.1-rc2"}), ] def _misc_expressions(env): From 8f2c0c5ea462af927bd525196fc252541cd6b387 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 17:49:38 +0900 Subject: [PATCH 27/57] add tests for inclusive matches --- python/private/py_wheel_normalize_pep440.bzl | 8 ++++++-- tests/pypi/pep508/evaluate_tests.bzl | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index e3e046d79c..17abd037ee 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -609,12 +609,16 @@ def _version_gt(left, right): def _version_ge(left, right): # PEP440: simple order check # https://peps.python.org/pep-0440/#inclusive-ordered-comparison - return left.key(local = False) >= right.key(local = False) + _left = left.key(local = False) + _right = right.key(local = False) + return _left >= _right def _version_le(left, right): # PEP440: simple order check # https://peps.python.org/pep-0440/#inclusive-ordered-comparison - return left.key(local = False) <= right.key(local = False) + _left = left.key(local = False) + _right = right.key(local = False) + return _left <= _right def _first_non_none(*args): for arg in args: diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 6608a6a986..91945112f5 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -292,6 +292,11 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version < "1.7.1"', False, {"python_version": "1.7.1-rc2"}), _expr_case('python_version < "1.7.1-rc3"', True, {"python_version": "1.7.1-rc2"}), _expr_case('python_version < "1.7.1-rc1"', False, {"python_version": "1.7.1-rc2"}), + # Extra tests + _expr_case('python_version <= "1.7.1"', True, {"python_version": "1.7.1"}), + _expr_case('python_version <= "1.7.2"', True, {"python_version": "1.7.1"}), + _expr_case('python_version >= "1.7.1"', True, {"python_version": "1.7.1"}), + _expr_case('python_version >= "1.7.0"', True, {"python_version": "1.7.1"}), ] def _misc_expressions(env): From e545c7d1694ecfcfefcdd33d945a9df9e9e45b24 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 17:52:38 +0900 Subject: [PATCH 28/57] for now fix the test --- .../env_marker_setting/env_marker_setting_tests.bzl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl b/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl index 549c15c20b..920a31aff8 100644 --- a/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl +++ b/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl @@ -19,19 +19,19 @@ def _test_expr(name): ) cases = { - "python_full_version_lt_negative": { + "python_full_version_gte": { "config_settings": { PYTHON_VERSION: "3.12.0", }, - "expected": "FALSE", - "expression": "python_full_version < '3.8'", + "expected": "TRUE", + "expression": "python_full_version >= '3.12.0'", }, - "python_version_gte": { + "python_full_version_lt_negative": { "config_settings": { PYTHON_VERSION: "3.12.0", }, - "expected": "TRUE", - "expression": "python_version >= '3.12.0'", + "expected": "FALSE", + "expression": "python_full_version < '3.8'", }, } From 436ef9d3d9496b1427334debde2d438b0180b3c8 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 17:55:43 +0900 Subject: [PATCH 29/57] instead of fixing the test, do something more clever - check for equality as well --- python/private/py_wheel_normalize_pep440.bzl | 4 ++-- .../env_marker_setting/env_marker_setting_tests.bzl | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 17abd037ee..4a08366a8c 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -611,14 +611,14 @@ def _version_ge(left, right): # https://peps.python.org/pep-0440/#inclusive-ordered-comparison _left = left.key(local = False) _right = right.key(local = False) - return _left >= _right + return _left > _right or _version_eq(left, right) def _version_le(left, right): # PEP440: simple order check # https://peps.python.org/pep-0440/#inclusive-ordered-comparison _left = left.key(local = False) _right = right.key(local = False) - return _left <= _right + return _left < _right or _version_eq(left, right) def _first_non_none(*args): for arg in args: diff --git a/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl b/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl index 920a31aff8..549c15c20b 100644 --- a/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl +++ b/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl @@ -19,19 +19,19 @@ def _test_expr(name): ) cases = { - "python_full_version_gte": { + "python_full_version_lt_negative": { "config_settings": { PYTHON_VERSION: "3.12.0", }, - "expected": "TRUE", - "expression": "python_full_version >= '3.12.0'", + "expected": "FALSE", + "expression": "python_full_version < '3.8'", }, - "python_full_version_lt_negative": { + "python_version_gte": { "config_settings": { PYTHON_VERSION: "3.12.0", }, - "expected": "FALSE", - "expression": "python_full_version < '3.8'", + "expected": "TRUE", + "expression": "python_version >= '3.12.0'", }, } From b84d1095594c2bea43d679ee6a4d1e79215f0481 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 6 May 2025 22:19:51 +0900 Subject: [PATCH 30/57] get rid of a parser --- python/private/py_wheel_normalize_pep440.bzl | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 4a08366a8c..dfd980f064 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -738,11 +738,7 @@ def parse_version(version, strict = False): parser = _new(version.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* - # TODO @aignas 2025-05-06: Remove this usage of a second parser just to get the normalized - # version. - parser_2 = _new(version.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* accept(parser, _is("v"), "") # PEP 440: Preceding v character - accept(parser_2, _is("v"), "") # PEP 440: Preceding v character parts = {} fns = [ @@ -756,7 +752,6 @@ def parse_version(version, strict = False): for p, fn in fns: fn(parser) - fn(parser_2) parts[p] = parser.context()["norm"] parser.context()["norm"] = "" # Clear out the buffer so that it is easy to separate the fields @@ -769,7 +764,7 @@ def parse_version(version, strict = False): # https://peps.python.org/pep-0440/#public-version-identifiers return None - if parser_2.input[parser.context()["start"]:]: + if parser.input[parser.context()["start"]:]: if strict: fail( "Failed to parse PEP 440 version identifier '%s'." % parser.input, @@ -779,5 +774,5 @@ def parse_version(version, strict = False): # If we fail to parse the version return None return None - parts["norm"] = parser_2.context()["norm"] + parts["norm"] = "".join([parts[p] for p, _ in fns]) return _new_version(**parts) From a5f45f41c241c0cdf08f6fb137e3653bf751e38d Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Thu, 8 May 2025 09:08:09 +0900 Subject: [PATCH 31/57] move the implementation for the compatible version matcher --- python/private/py_wheel_normalize_pep440.bzl | 9 +++++++++ python/private/pypi/pep508_evaluate.bzl | 7 +------ tests/pypi/pep508/evaluate_tests.bzl | 7 +++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index dfd980f064..89cb272114 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -620,6 +620,14 @@ def _version_le(left, right): _right = right.key(local = False) return _left < _right or _version_eq(left, right) +def _version_compatible(left, right): + # https://peps.python.org/pep-0440/#compatible-release + # Note, the ~= operator can be also expressed as: + # >= V.N, == V.* + head, _, _ = right.norm.partition(".") + right_star = parse_version("{}.*".format(head)) + return left.ge(right) and left.eq(right_star) + def _first_non_none(*args): for arg in args: if arg != None: @@ -714,6 +722,7 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized le = lambda x: _version_le(self, x), # buildifier: disable=uninitialized ge = lambda x: _version_ge(self, x), # buildifier: disable=uninitialized + compatible = lambda x: _version_compatible(self, x), # buildifier: disable=uninitialized str = lambda: norm, key = lambda *, local = True: _key(self, local = local), # buildifier: disable=uninitialized ) diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index 304419435b..f0efa72549 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -388,12 +388,7 @@ def _version_expr(left, op, right): elif op == ">=": return _left.ge(_right) elif op == "~=": - # https://peps.python.org/pep-0440/#compatible-release - # Note, the ~= operator can be also expressed as: - # >= V.N, == V.* - head, _, _ = right.partition(".") - _right_star = parse_version("{}.*".format(head)) - return _left.ge(_right) and _left.eq(_right_star) + return _left.compatible(_right) else: return False # Let's just ignore the invalid ops diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 91945112f5..af2a98af41 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -297,6 +297,13 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version <= "1.7.2"', True, {"python_version": "1.7.1"}), _expr_case('python_version >= "1.7.1"', True, {"python_version": "1.7.1"}), _expr_case('python_version >= "1.7.0"', True, {"python_version": "1.7.1"}), + # Compatible version tests: + # https://packaging.python.org/en/latest/specifications/version-specifiers/#compatible-release + _expr_case('python_version ~= "2.2"', True, {"python_version": "2.3"}), + _expr_case('python_version ~= "2.2"', False, {"python_version": "2.1"}), + _expr_case('python_version ~= "2.2.post3"', False, {"python_version": "2.2"}), + _expr_case('python_version ~= "2.2.post3"', True, {"python_version": "2.3"}), + _expr_case('python_version ~= "2.2.post3"', False, {"python_version": "3.0"}), ] def _misc_expressions(env): From 3106315815a51ed00a6b2200c57693129d364593 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 16:14:39 +0900 Subject: [PATCH 32/57] create an eqq operator and simplify --- python/private/py_wheel_normalize_pep440.bzl | 41 ++++++++++++++++++-- python/private/pypi/pep508_evaluate.bzl | 10 ++--- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/py_wheel_normalize_pep440.bzl index 89cb272114..01c73a3202 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/py_wheel_normalize_pep440.bzl @@ -526,6 +526,14 @@ def _pad_zeros(release, n): release = list(release) + [0] * padding return tuple(release) +def _version_eqq(left, right): + if left.is_prefix or right.is_prefix: + fail(_prefix_err(left, "<", right)) + + # https://peps.python.org/pep-0440/#arbitrary-equality + # > simple string equality operations + return left.norm == right.norm + def _version_eq(left, right): if left.is_prefix and right.is_prefix: fail("Invalid comparison: both versions cannot be prefix matching") @@ -552,7 +560,21 @@ def _version_eq(left, right): ##and left.local == right.local ) +def _version_ne(left, right): + return not _version_eq(left, right) + +def _prefix_err(left, op, right): + if left.is_prefix or right.is_prefix: + fail("PEP440: only '==' and '!=' operators can use prefix matching: {} {} {}".format( + left, + op, + right, + )) + def _version_lt(left, right): + if left.is_prefix or right.is_prefix: + fail(_prefix_err(left, "<", right)) + if left.epoch > right.epoch: return False elif left.epoch < right.epoch: @@ -577,6 +599,9 @@ def _version_lt(left, right): return False def _version_gt(left, right): + if left.is_prefix or right.is_prefix: + fail(_prefix_err(left, ">", right)) + if left.epoch > right.epoch: return True elif left.epoch < right.epoch: @@ -607,6 +632,9 @@ def _version_gt(left, right): return False def _version_ge(left, right): + if left.is_prefix or right.is_prefix: + fail(_prefix_err(left, ">=", right)) + # PEP440: simple order check # https://peps.python.org/pep-0440/#inclusive-ordered-comparison _left = left.key(local = False) @@ -614,6 +642,9 @@ def _version_ge(left, right): return _left > _right or _version_eq(left, right) def _version_le(left, right): + if left.is_prefix or right.is_prefix: + fail(_prefix_err(left, "<=", right)) + # PEP440: simple order check # https://peps.python.org/pep-0440/#inclusive-ordered-comparison _left = left.key(local = False) @@ -621,6 +652,9 @@ def _version_le(left, right): return _left < _right or _version_eq(left, right) def _version_compatible(left, right): + if left.is_prefix or right.is_prefix: + fail(_prefix_err(left, "~=", right)) + # https://peps.python.org/pep-0440/#compatible-release # Note, the ~= operator can be also expressed as: # >= V.N, == V.* @@ -717,11 +751,12 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " is_prefix = is_prefix, norm = norm, eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized - ne = lambda x: not _version_eq(self, x), # buildifier: disable=uninitialized - lt = lambda x: _version_lt(self, x), # buildifier: disable=uninitialized + eqq = lambda x: _version_eqq(self, x), # buildifier: disable=uninitialized + ge = lambda x: _version_ge(self, x), # buildifier: disable=uninitialized gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized le = lambda x: _version_le(self, x), # buildifier: disable=uninitialized - ge = lambda x: _version_ge(self, x), # buildifier: disable=uninitialized + lt = lambda x: _version_lt(self, x), # buildifier: disable=uninitialized + ne = lambda x: _version_ne(self, x), # buildifier: disable=uninitialized compatible = lambda x: _version_compatible(self, x), # buildifier: disable=uninitialized str = lambda: norm, key = lambda *, local = True: _key(self, local = local), # buildifier: disable=uninitialized diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index f0efa72549..5ad6dcb786 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -368,17 +368,13 @@ def _version_expr(left, op, right): # or `platform_release`, which vary depending on platform. return _env_expr(left, op, right) - if op == "!=": + if op == "===": + return _left.eqq(_right) + elif op == "!=": return _left.ne(_right) elif op == "==": # Matching of major, minor, patch only return _left.eq(_right) - elif left.endswith(".*") or right.endswith(".*"): - fail("PEP440: only '==' and '!=' operators can use prefix matching: {} {} {}".format( - left, - op, - right, - )) elif op == "<": return _left.lt(_right) elif op == ">": From 22da4801f86c7828bc08257f205fe6e3aedeb87b Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 16:29:15 +0900 Subject: [PATCH 33/57] Add a new file for parsing versions --- python/BUILD.bazel | 2 +- python/private/BUILD.bazel | 12 +++++------ python/private/py_wheel.bzl | 2 +- python/private/pypi/BUILD.bazel | 2 +- python/private/pypi/pep508_evaluate.bzl | 21 +++---------------- ...wheel_normalize_pep440.bzl => version.bzl} | 12 +++++------ tests/py_wheel/py_wheel_tests.bzl | 2 +- tests/pypi/pep508/evaluate_tests.bzl | 3 ++- 8 files changed, 20 insertions(+), 36 deletions(-) rename python/private/{py_wheel_normalize_pep440.bzl => version.bzl} (98%) diff --git a/python/BUILD.bazel b/python/BUILD.bazel index 3389a0dacc..867c43478a 100644 --- a/python/BUILD.bazel +++ b/python/BUILD.bazel @@ -93,9 +93,9 @@ bzl_library( "//python/private:bzlmod_enabled_bzl", "//python/private:py_package.bzl", "//python/private:py_wheel_bzl", - "//python/private:py_wheel_normalize_pep440.bzl", "//python/private:stamp_bzl", "//python/private:util_bzl", + "//python/private:version.bzl", "@bazel_skylib//rules:native_binary", ], ) diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index 208916b200..e72a8fcaa7 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -578,11 +578,6 @@ bzl_library( ], ) -bzl_library( - name = "py_wheel_normalize_pep440_bzl", - srcs = ["py_wheel_normalize_pep440.bzl"], -) - bzl_library( name = "reexports_bzl", srcs = ["reexports.bzl"], @@ -663,6 +658,11 @@ bzl_library( ], ) +bzl_library( + name = "version_bzl", + srcs = ["version.bzl"], +) + bzl_library( name = "version_label_bzl", srcs = ["version_label.bzl"], @@ -706,7 +706,7 @@ exports_files( "repack_whl.py", "py_package.bzl", "py_wheel.bzl", - "py_wheel_normalize_pep440.bzl", + "version.bzl", "reexports.bzl", "stamp.bzl", "util.bzl", diff --git a/python/private/py_wheel.bzl b/python/private/py_wheel.bzl index c196ca6ad0..4c8ea22bfa 100644 --- a/python/private/py_wheel.bzl +++ b/python/private/py_wheel.bzl @@ -16,8 +16,8 @@ load(":py_info.bzl", "PyInfo") load(":py_package.bzl", "py_package_lib") -load(":py_wheel_normalize_pep440.bzl", "normalize_pep440") load(":stamp.bzl", "is_stamping_enabled") +load(":version.bzl", "normalize_pep440") PyWheelInfo = provider( doc = "Information about a wheel produced by `py_wheel`", diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel index aacbc99f70..31e7aaab04 100644 --- a/python/private/pypi/BUILD.bazel +++ b/python/private/pypi/BUILD.bazel @@ -232,7 +232,7 @@ bzl_library( srcs = ["pep508_env.bzl"], deps = [ ":pep508_platform_bzl", - "//python/private:py_wheel_normalize_pep440_bzl", + "//python/private:version_bzl", ], ) diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index 5ad6dcb786..a1e3266571 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -16,10 +16,7 @@ """ load("//python/private:enum.bzl", "enum") -load("//python/private:py_wheel_normalize_pep440.bzl", "parse_version") - -# TODO @aignas 2025-05-06: this is exposed for tests only -version = parse_version +load("//python/private:version.bzl", "version") # The expression parsing and resolution for the PEP508 is below # @@ -348,20 +345,8 @@ def _env_expr(left, op, right): def _version_expr(left, op, right): """Evaluate a version comparison expression""" - if op == "===": - # https://peps.python.org/pep-0440/#arbitrary-equality - # > simple string equality operations - return _env_expr(left, "==", right) - - if left.endswith(".*") and right.endswith(".*"): - fail("PEP440: only one of the sides can have '.*' suffix: {} {} {}".format( - left, - op, - right, - )) - - _left = parse_version(left) - _right = parse_version(right) + _left = version(left) + _right = version(right) if _left == None or _right == None: # Per spec, if either can't be normalized to a version, then # fallback to simple string comparison. Usually this is `platform_version` diff --git a/python/private/py_wheel_normalize_pep440.bzl b/python/private/version.bzl similarity index 98% rename from python/private/py_wheel_normalize_pep440.bzl rename to python/private/version.bzl index 01c73a3202..ff0ca24914 100644 --- a/python/private/py_wheel_normalize_pep440.bzl +++ b/python/private/version.bzl @@ -659,7 +659,7 @@ def _version_compatible(left, right): # Note, the ~= operator can be also expressed as: # >= V.N, == V.* head, _, _ = right.norm.partition(".") - right_star = parse_version("{}.*".format(head)) + right_star = version("{}.*".format(head)) return left.ge(right) and left.eq(right_star) def _first_non_none(*args): @@ -764,23 +764,21 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " return self -def parse_version(version, strict = False): +def version(version_str, strict = False): """Parse a PEP4408 compliant version - TODO @aignas 2025-05-06: where should this go? - See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode and https://peps.python.org/pep-0440/ Args: - version: version string to be normalized according to PEP 440. + version_str: version string to be normalized according to PEP 440. strict: fail if the version is invalid. Returns: string containing the normalized version. """ - parser = _new(version.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* + parser = _new(version_str.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* accept(parser, _is("v"), "") # PEP 440: Preceding v character @@ -799,7 +797,7 @@ def parse_version(version, strict = False): parts[p] = parser.context()["norm"] parser.context()["norm"] = "" # Clear out the buffer so that it is easy to separate the fields - is_prefix = version.endswith(".*") + is_prefix = version_str.endswith(".*") parts["is_prefix"] = is_prefix if is_prefix and (parts["local"] or parts["post"] or parts["dev"] or parts["pre"]): if strict: diff --git a/tests/py_wheel/py_wheel_tests.bzl b/tests/py_wheel/py_wheel_tests.bzl index 091e01c37d..b18a5a6f09 100644 --- a/tests/py_wheel/py_wheel_tests.bzl +++ b/tests/py_wheel/py_wheel_tests.bzl @@ -17,7 +17,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") load("@rules_testing//lib:truth.bzl", "matching") load("@rules_testing//lib:util.bzl", rt_util = "util") load("//python:packaging.bzl", "py_wheel") -load("//python/private:py_wheel_normalize_pep440.bzl", "normalize_pep440") # buildifier: disable=bzl-visibility +load("//python/private:version.bzl", "normalize_pep440") # buildifier: disable=bzl-visibility _basic_tests = [] _tests = [] diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index af2a98af41..67c3d3596c 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -14,8 +14,9 @@ """Tests for construction of Python version matching config settings.""" load("@rules_testing//lib:test_suite.bzl", "test_suite") +load("//python/private:version.bzl", "version") # buildifier: disable=bzl-visibility load("//python/private/pypi:pep508_env.bzl", pep508_env = "env") # buildifier: disable=bzl-visibility -load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize", "version") # buildifier: disable=bzl-visibility +load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize") # buildifier: disable=bzl-visibility _tests = [] From 4469beeec0dbf50b86cd833eb4fed50dfae3fba5 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 16:43:26 +0900 Subject: [PATCH 34/57] simplify the implementation --- python/private/version.bzl | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index ff0ca24914..f44ad0ff4b 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -662,13 +662,6 @@ def _version_compatible(left, right): right_star = version("{}.*".format(head)) return left.ge(right) and left.eq(right_star) -def _first_non_none(*args): - for arg in args: - if arg != None: - return arg - - return None - def _key(self, *, local, release_key = ("z",)): """This function returns a tuple that can be used in 'sorted' calls. @@ -685,15 +678,11 @@ def _key(self, *, local, release_key = ("z",)): # PEP440 release ordering: .devN, aN, bN, rcN, , .postN # We choose to first match the pre-release, then post release, then dev and # then stable - _first_non_none(self.pre, self.post, self.dev, release_key), + self.pre or self.post or self.dev or release_key, # PEP440 local versions go before post versions tuple([(type(item) == "int", item) for item in local]), # PEP440 - pre-release ordering: .devN, , .postN - _first_non_none( - self.post, - self.dev, - release_key, - ), + self.post or self.dev or release_key, # PEP440 - post release ordering: .devN, self.dev or release_key, ) From f3a996944c57810a38d236009f556fc0e96313f3 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 16:58:52 +0900 Subject: [PATCH 35/57] simplify --- python/private/version.bzl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index f44ad0ff4b..7df0b460b7 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -782,9 +782,11 @@ def version(version_str, strict = False): ] for p, fn in fns: + start = len(parser.context()["norm"]) fn(parser) - parts[p] = parser.context()["norm"] - parser.context()["norm"] = "" # Clear out the buffer so that it is easy to separate the fields + parts[p] = parser.context()["norm"][start:] + + parts["norm"] = parser.context()["norm"] is_prefix = version_str.endswith(".*") parts["is_prefix"] = is_prefix @@ -805,5 +807,4 @@ def version(version_str, strict = False): # If we fail to parse the version return None return None - parts["norm"] = "".join([parts[p] for p, _ in fns]) return _new_version(**parts) From 6047094fb843f5a001788d7366de1a51ee4f9701 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 17:20:30 +0900 Subject: [PATCH 36/57] reorder methods --- python/private/version.bzl | 112 ++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 7df0b460b7..46b35869e8 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -518,6 +518,62 @@ def normalize_pep440(version): ) return parser.context()["norm"] +def version(version_str, strict = False): + """Parse a PEP4408 compliant version + + See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode + and https://peps.python.org/pep-0440/ + + Args: + version_str: version string to be normalized according to PEP 440. + strict: fail if the version is invalid. + + Returns: + string containing the normalized version. + """ + + parser = _new(version_str.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* + + accept(parser, _is("v"), "") # PEP 440: Preceding v character + + parts = {} + fns = [ + ("epoch", accept_epoch), + ("release", accept_release), + ("pre", accept_prerelease), + ("post", accept_postrelease), + ("dev", accept_devrelease), + ("local", accept_local), + ] + + for p, fn in fns: + start = len(parser.context()["norm"]) + fn(parser) + parts[p] = parser.context()["norm"][start:] + + parts["norm"] = parser.context()["norm"] + + is_prefix = version_str.endswith(".*") + parts["is_prefix"] = is_prefix + if 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") + + # https://peps.python.org/pep-0440/#public-version-identifiers + return None + + if parser.input[parser.context()["start"]:]: + if strict: + fail( + "Failed to parse PEP 440 version identifier '%s'." % parser.input, + "Parse error at '%s'" % parser.input[parser.context()["start"]:], + ) + + # If we fail to parse the version return None + return None + + return _new_version(**parts) + def _pad_zeros(release, n): padding = n - len(release) if padding <= 0: @@ -752,59 +808,3 @@ def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = " ) return self - -def version(version_str, strict = False): - """Parse a PEP4408 compliant version - - See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode - and https://peps.python.org/pep-0440/ - - Args: - version_str: version string to be normalized according to PEP 440. - strict: fail if the version is invalid. - - Returns: - string containing the normalized version. - """ - - parser = _new(version_str.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* - - accept(parser, _is("v"), "") # PEP 440: Preceding v character - - parts = {} - fns = [ - ("epoch", accept_epoch), - ("release", accept_release), - ("pre", accept_prerelease), - ("post", accept_postrelease), - ("dev", accept_devrelease), - ("local", accept_local), - ] - - for p, fn in fns: - start = len(parser.context()["norm"]) - fn(parser) - parts[p] = parser.context()["norm"][start:] - - parts["norm"] = parser.context()["norm"] - - is_prefix = version_str.endswith(".*") - parts["is_prefix"] = is_prefix - if 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") - - # https://peps.python.org/pep-0440/#public-version-identifiers - return None - - if parser.input[parser.context()["start"]:]: - if strict: - fail( - "Failed to parse PEP 440 version identifier '%s'." % parser.input, - "Parse error at '%s'" % parser.input[parser.context()["start"]:], - ) - - # If we fail to parse the version return None - return None - - return _new_version(**parts) From af8eee908866885023cbd7e293c6c595c8733199 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 17:21:47 +0900 Subject: [PATCH 37/57] reorder methods more --- python/private/version.bzl | 132 ++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 46b35869e8..edcf3c3200 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -574,6 +574,72 @@ def version(version_str, strict = False): return _new_version(**parts) +def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): + epoch = epoch or 0 + _release = tuple([int(d) for d in release.split(".")]) + + if pre: + if pre.startswith("rc"): + prefix = "rc" + else: + prefix = pre[0] + + pre = (prefix, int(pre[len(prefix):])) + else: + pre = None + + if post: + if not post.startswith(".post"): + fail("post release identifier must start with '.post', got: {}".format(post)) + post = int(post[len(".post"):]) + + # We choose `~` since almost all of the ASCII characters will be before + # it. Use `ord` and `chr` functions to find a good value. + post = ("~", post) + else: + post = None + + if dev: + if not dev.startswith(".dev"): + fail("dev release identifier must start with '.dev', got: {}".format(dev)) + dev = int(dev[len(".dev"):]) + + # Empty string goes first when comparing + dev = ("", dev) + else: + dev = None + + if local: + local = local.lstrip("+") + + # If the part is numerical, handle it as a number + local = tuple([int(part) if part.isdigit() else part for part in local.split(".")]) + else: + local = None + + self = struct( + epoch = epoch, + release = _release, + pre = pre, + post = post, + dev = dev, + local = local, + is_prefix = is_prefix, + norm = norm, + eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized + eqq = lambda x: _version_eqq(self, x), # buildifier: disable=uninitialized + ge = lambda x: _version_ge(self, x), # buildifier: disable=uninitialized + gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized + le = lambda x: _version_le(self, x), # buildifier: disable=uninitialized + lt = lambda x: _version_lt(self, x), # buildifier: disable=uninitialized + ne = lambda x: _version_ne(self, x), # buildifier: disable=uninitialized + compatible = lambda x: _version_compatible(self, x), # buildifier: disable=uninitialized + str = lambda: norm, + key = lambda *, local = True: _key(self, local = local), # buildifier: disable=uninitialized + ) + + return self + def _pad_zeros(release, n): padding = n - len(release) if padding <= 0: @@ -742,69 +808,3 @@ def _key(self, *, local, release_key = ("z",)): # PEP440 - post release ordering: .devN, self.dev or release_key, ) - -def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): - epoch = epoch or 0 - _release = tuple([int(d) for d in release.split(".")]) - - if pre: - if pre.startswith("rc"): - prefix = "rc" - else: - prefix = pre[0] - - pre = (prefix, int(pre[len(prefix):])) - else: - pre = None - - if post: - if not post.startswith(".post"): - fail("post release identifier must start with '.post', got: {}".format(post)) - post = int(post[len(".post"):]) - - # We choose `~` since almost all of the ASCII characters will be before - # it. Use `ord` and `chr` functions to find a good value. - post = ("~", post) - else: - post = None - - if dev: - if not dev.startswith(".dev"): - fail("dev release identifier must start with '.dev', got: {}".format(dev)) - dev = int(dev[len(".dev"):]) - - # Empty string goes first when comparing - dev = ("", dev) - else: - dev = None - - if local: - local = local.lstrip("+") - - # If the part is numerical, handle it as a number - local = tuple([int(part) if part.isdigit() else part for part in local.split(".")]) - else: - local = None - - self = struct( - epoch = epoch, - release = _release, - pre = pre, - post = post, - dev = dev, - local = local, - is_prefix = is_prefix, - norm = norm, - eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized - eqq = lambda x: _version_eqq(self, x), # buildifier: disable=uninitialized - ge = lambda x: _version_ge(self, x), # buildifier: disable=uninitialized - gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized - le = lambda x: _version_le(self, x), # buildifier: disable=uninitialized - lt = lambda x: _version_lt(self, x), # buildifier: disable=uninitialized - ne = lambda x: _version_ne(self, x), # buildifier: disable=uninitialized - compatible = lambda x: _version_compatible(self, x), # buildifier: disable=uninitialized - str = lambda: norm, - key = lambda *, local = True: _key(self, local = local), # buildifier: disable=uninitialized - ) - - return self From e64c199fcd3b7597f953ad42df6d9f91b396a8d8 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 17:30:19 +0900 Subject: [PATCH 38/57] dedupe code for normalization and the actual version parsing --- python/private/version.bzl | 68 ++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index edcf3c3200..7dff96a4cc 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -503,40 +503,31 @@ def normalize_pep440(version): Returns: string containing the normalized version. """ - parser = _new(version.strip()) # PEP 440: Leading and Trailing Whitespace - accept(parser, _is("v"), "") # PEP 440: Preceding v character - accept_epoch(parser) - accept_release(parser) - accept_prerelease(parser) - accept_postrelease(parser) - accept_devrelease(parser) - accept_local(parser) - if parser.input[parser.context()["start"]:]: - fail( - "Failed to parse PEP 440 version identifier '%s'." % parser.input, - "Parse error at '%s'" % parser.input[parser.context()["start"]:], - ) - return parser.context()["norm"] + return _parse(version, strict = True)["norm"] -def version(version_str, strict = False): - """Parse a PEP4408 compliant version +def _parse(version_str, strict = True): + """Escape the version component of a filename. See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode and https://peps.python.org/pep-0440/ Args: version_str: version string to be normalized according to PEP 440. - strict: fail if the version is invalid. + strict: fail if the version is invalid, defaults to True. Returns: string containing the normalized version. """ + if strict: + version_str = version_str.strip() # PEP 440: Leading and Trailing Whitespace + is_prefix = False + else: + version_str = version_str.strip() # PEP 440: Leading and Trailing Whitespace + is_prefix = version_str.endswith(".*") + version_str = version_str.strip(" .*") # PEP 440: Leading and Trailing Whitespace and ".*" - parser = _new(version_str.strip(" " if strict else " .*")) # PEP 440: Leading and Trailing Whitespace and .* - + parser = _new(version_str) accept(parser, _is("v"), "") # PEP 440: Preceding v character - - parts = {} fns = [ ("epoch", accept_epoch), ("release", accept_release), @@ -546,15 +537,15 @@ def version(version_str, strict = False): ("local", accept_local), ] - for p, fn in fns: + parts = { + "is_prefix": is_prefix, + } + for key, fn in fns: start = len(parser.context()["norm"]) fn(parser) - parts[p] = parser.context()["norm"][start:] - + parts[key] = parser.context()["norm"][start:] parts["norm"] = parser.context()["norm"] - is_prefix = version_str.endswith(".*") - parts["is_prefix"] = is_prefix if 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") @@ -569,7 +560,26 @@ def version(version_str, strict = False): "Parse error at '%s'" % parser.input[parser.context()["start"]:], ) - # If we fail to parse the version return None + return None + + return parts + +def version(version_str, strict = False): + """Parse a PEP4408 compliant version + + See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode + and https://peps.python.org/pep-0440/ + + Args: + version_str: version string to be normalized according to PEP 440. + strict: fail if the version is invalid. + + Returns: + string containing the normalized version. + """ + + parts = _parse(version_str, strict = strict) + if not parts: return None return _new_version(**parts) @@ -688,9 +698,9 @@ def _version_ne(left, right): def _prefix_err(left, op, right): if left.is_prefix or right.is_prefix: fail("PEP440: only '==' and '!=' operators can use prefix matching: {} {} {}".format( - left, + left.str(), op, - right, + right.str(), )) def _version_lt(left, right): From 74b029d79a1723cc80969f4a591600de31a44946 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 17:30:40 +0900 Subject: [PATCH 39/57] dedupe code for normalization and the actual version parsing --- python/private/version.bzl | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 7dff96a4cc..be06200de2 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -518,11 +518,10 @@ def _parse(version_str, strict = True): Returns: string containing the normalized version. """ - if strict: - version_str = version_str.strip() # PEP 440: Leading and Trailing Whitespace - is_prefix = False - else: - version_str = version_str.strip() # PEP 440: Leading and Trailing Whitespace + version_str = version_str.strip() # PEP 440: Leading and Trailing Whitespace + is_prefix = False + + if not strict: is_prefix = version_str.endswith(".*") version_str = version_str.strip(" .*") # PEP 440: Leading and Trailing Whitespace and ".*" From a3820a8c88174fc9b4c465aa1e1c1dd5ee033093 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 17:32:48 +0900 Subject: [PATCH 40/57] reformat --- python/private/version.bzl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index be06200de2..e09b988bd3 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -535,7 +535,6 @@ def _parse(version_str, strict = True): ("dev", accept_devrelease), ("local", accept_local), ] - parts = { "is_prefix": is_prefix, } @@ -583,7 +582,16 @@ def version(version_str, strict = False): return _new_version(**parts) -def _new_version(*, epoch = 0, release, pre = "", post = "", dev = "", local = "", is_prefix = False, norm): +def _new_version( + *, + epoch = 0, + release, + pre = "", + post = "", + dev = "", + local = "", + is_prefix = False, + norm): epoch = epoch or 0 _release = tuple([int(d) for d in release.split(".")]) From 42c866f522e7180d85d1c23f6d519ef9e52ee970 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 17:36:50 +0900 Subject: [PATCH 41/57] reformat --- python/private/version.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index e09b988bd3..32e71295ef 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -584,14 +584,14 @@ def version(version_str, strict = False): def _new_version( *, + norm, epoch = 0, release, pre = "", post = "", dev = "", local = "", - is_prefix = False, - norm): + is_prefix = False): epoch = epoch or 0 _release = tuple([int(d) for d in release.split(".")]) From 2d7d9e52317abd22701b47cc2187fc76794793d0 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 17:52:06 +0900 Subject: [PATCH 42/57] rework the code structure a little --- python/private/version.bzl | 177 ++++++++++++++++++++----------------- 1 file changed, 98 insertions(+), 79 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 32e71295ef..14f1091518 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -544,6 +544,7 @@ def _parse(version_str, strict = True): parts[key] = parser.context()["norm"][start:] parts["norm"] = parser.context()["norm"] + # TODO @aignas 2025-05-09: move the `is_prefix` handling to `accept_release` if 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") @@ -582,65 +583,75 @@ def version(version_str, strict = False): return _new_version(**parts) -def _new_version( - *, - norm, - epoch = 0, - release, - pre = "", - post = "", - dev = "", - local = "", - is_prefix = False): - epoch = epoch or 0 - _release = tuple([int(d) for d in release.split(".")]) +def _parse_epoch(value): + if not value: + return 0 - if pre: - if pre.startswith("rc"): - prefix = "rc" - else: - prefix = pre[0] + return int(value) - pre = (prefix, int(pre[len(prefix):])) - else: - pre = None +def _parse_release(value): + return tuple([int(d) for d in value.split(".")]) - if post: - if not post.startswith(".post"): - fail("post release identifier must start with '.post', got: {}".format(post)) - post = int(post[len(".post"):]) +def _parse_local(value): + if not value: + return None - # We choose `~` since almost all of the ASCII characters will be before - # it. Use `ord` and `chr` functions to find a good value. - post = ("~", post) - else: - post = None + local = value.lstrip("+") - if dev: - if not dev.startswith(".dev"): - fail("dev release identifier must start with '.dev', got: {}".format(dev)) - dev = int(dev[len(".dev"):]) + # If the part is numerical, handle it as a number + return tuple([int(part) if part.isdigit() else part for part in local.split(".")]) - # Empty string goes first when comparing - dev = ("", dev) - else: - dev = None +def _parse_dev(value): + if not value: + return None - if local: - local = local.lstrip("+") + if not value.startswith(".dev"): + fail("dev release identifier must start with '.dev', got: {}".format(value)) + dev = int(value[len(".dev"):]) - # If the part is numerical, handle it as a number - local = tuple([int(part) if part.isdigit() else part for part in local.split(".")]) + # Empty string goes first when comparing + return ("", dev) + +def _parse_pre(value): + if not value: + return None + + if value.startswith("rc"): + prefix = "rc" else: - local = None + prefix = value[0] + + return (prefix, int(value[len(prefix):])) +def _parse_post(value): + if not value: + return None + + if not value.startswith(".post"): + fail("post release identifier must start with '.post', got: {}".format(value)) + post = int(value[len(".post"):]) + + # We choose `~` since almost all of the ASCII characters will be before + # it. Use `ord` and `chr` functions to find a good value. + return ("~", post) + +def _new_version( + *, + norm, + epoch = 0, + release, + pre = "", + post = "", + dev = "", + local = "", + is_prefix = False): self = struct( - epoch = epoch, - release = _release, - pre = pre, - post = post, - dev = dev, - local = local, + epoch = _parse_epoch(epoch), + release = _parse_release(release), + pre = _parse_pre(pre), + post = _parse_post(post), + dev = _parse_dev(dev), + local = _parse_local(local), is_prefix = is_prefix, norm = norm, eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized @@ -652,7 +663,7 @@ def _new_version( ne = lambda x: _version_ne(self, x), # buildifier: disable=uninitialized compatible = lambda x: _version_compatible(self, x), # buildifier: disable=uninitialized str = lambda: norm, - key = lambda *, local = True: _key(self, local = local), # buildifier: disable=uninitialized + key = lambda *, local = True: _version_key(self, local = local), # buildifier: disable=uninitialized ) return self @@ -665,15 +676,25 @@ def _pad_zeros(release, n): release = list(release) + [0] * padding return tuple(release) +def _prefix_err(left, op, right): + if left.is_prefix or right.is_prefix: + fail("PEP440: only '==' and '!=' operators can use prefix matching: {} {} {}".format( + left.norm, + op, + right.norm, + )) + def _version_eqq(left, right): + """=== operator""" if left.is_prefix or right.is_prefix: - fail(_prefix_err(left, "<", right)) + fail(_prefix_err(left, "===", right)) # https://peps.python.org/pep-0440/#arbitrary-equality # > simple string equality operations return left.norm == right.norm def _version_eq(left, right): + """== operator""" if left.is_prefix and right.is_prefix: fail("Invalid comparison: both versions cannot be prefix matching") if left.is_prefix: @@ -699,18 +720,24 @@ def _version_eq(left, right): ##and left.local == right.local ) +def _version_compatible(left, right): + """~= operator""" + if left.is_prefix or right.is_prefix: + fail(_prefix_err(left, "~=", right)) + + # https://peps.python.org/pep-0440/#compatible-release + # Note, the ~= operator can be also expressed as: + # >= V.N, == V.* + head, _, _ = right.norm.partition(".") + right_star = version("{}.*".format(head)) + return _version_ge(left, right) and _version_eq(left, right_star) + def _version_ne(left, right): + """!= operator""" return not _version_eq(left, right) -def _prefix_err(left, op, right): - if left.is_prefix or right.is_prefix: - fail("PEP440: only '==' and '!=' operators can use prefix matching: {} {} {}".format( - left.str(), - op, - right.str(), - )) - def _version_lt(left, right): + """< operator""" if left.is_prefix or right.is_prefix: fail(_prefix_err(left, "<", right)) @@ -738,6 +765,7 @@ def _version_lt(left, right): return False def _version_gt(left, right): + """> operator""" if left.is_prefix or right.is_prefix: fail(_prefix_err(left, ">", right)) @@ -770,38 +798,29 @@ def _version_gt(left, right): # False anyway. return False -def _version_ge(left, right): - if left.is_prefix or right.is_prefix: - fail(_prefix_err(left, ">=", right)) - - # PEP440: simple order check - # https://peps.python.org/pep-0440/#inclusive-ordered-comparison - _left = left.key(local = False) - _right = right.key(local = False) - return _left > _right or _version_eq(left, right) - def _version_le(left, right): + """<= operator""" if left.is_prefix or right.is_prefix: fail(_prefix_err(left, "<=", right)) # PEP440: simple order check # https://peps.python.org/pep-0440/#inclusive-ordered-comparison - _left = left.key(local = False) - _right = right.key(local = False) + _left = _version_key(left, local = False) + _right = _version_key(right, local = False) return _left < _right or _version_eq(left, right) -def _version_compatible(left, right): +def _version_ge(left, right): + """>= operator""" if left.is_prefix or right.is_prefix: - fail(_prefix_err(left, "~=", right)) + fail(_prefix_err(left, ">=", right)) - # https://peps.python.org/pep-0440/#compatible-release - # Note, the ~= operator can be also expressed as: - # >= V.N, == V.* - head, _, _ = right.norm.partition(".") - right_star = version("{}.*".format(head)) - return left.ge(right) and left.eq(right_star) + # PEP440: simple order check + # https://peps.python.org/pep-0440/#inclusive-ordered-comparison + _left = _version_key(left, local = False) + _right = _version_key(right, local = False) + return _left > _right or _version_eq(left, right) -def _key(self, *, local, release_key = ("z",)): +def _version_key(self, *, local, release_key = ("z",)): """This function returns a tuple that can be used in 'sorted' calls. This implements the PEP440 version sorting. From 743fd38f2533e282f0cdd01ff2ad647db3e62a40 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 18:10:04 +0900 Subject: [PATCH 43/57] refactor recursiveness --- python/private/version.bzl | 59 ++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 14f1091518..227f32124d 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -653,20 +653,30 @@ def _new_version( dev = _parse_dev(dev), local = _parse_local(local), is_prefix = is_prefix, - norm = norm, - eq = lambda x: _version_eq(self, x), # buildifier: disable=uninitialized - eqq = lambda x: _version_eqq(self, x), # buildifier: disable=uninitialized - ge = lambda x: _version_ge(self, x), # buildifier: disable=uninitialized - gt = lambda x: _version_gt(self, x), # buildifier: disable=uninitialized - le = lambda x: _version_le(self, x), # buildifier: disable=uninitialized - lt = lambda x: _version_lt(self, x), # buildifier: disable=uninitialized - ne = lambda x: _version_ne(self, x), # buildifier: disable=uninitialized - compatible = lambda x: _version_compatible(self, x), # buildifier: disable=uninitialized - str = lambda: norm, - key = lambda *, local = True: _version_key(self, local = local), # buildifier: disable=uninitialized + string = norm, ) - return self + public = struct( + eq = mkmethod(self, _version_eq), + eqq = mkmethod(self, _version_eqq), + ge = mkmethod(self, _version_ge), + gt = mkmethod(self, _version_gt), + le = mkmethod(self, _version_le), + lt = mkmethod(self, _version_lt), + ne = mkmethod(self, _version_ne), + compatible = mkmethod(self, _version_compatible), + key = mkmethod(self, _version_key), + epoch = self.epoch, + release = self.release, + pre = self.pre, + post = self.post, + dev = self.dev, + local = self.local, + is_prefix = self.is_prefix, + string = norm, + ) + + return public def _pad_zeros(release, n): padding = n - len(release) @@ -679,9 +689,9 @@ def _pad_zeros(release, n): def _prefix_err(left, op, right): if left.is_prefix or right.is_prefix: fail("PEP440: only '==' and '!=' operators can use prefix matching: {} {} {}".format( - left.norm, + left.string, op, - right.norm, + right.string, )) def _version_eqq(left, right): @@ -691,16 +701,16 @@ def _version_eqq(left, right): # https://peps.python.org/pep-0440/#arbitrary-equality # > simple string equality operations - return left.norm == right.norm + return left.string == right.string def _version_eq(left, right): """== operator""" if left.is_prefix and right.is_prefix: fail("Invalid comparison: both versions cannot be prefix matching") if left.is_prefix: - return right.norm.startswith("{}.".format(left.norm)) + return right.string.startswith("{}.".format(left.string)) if right.is_prefix: - return left.norm.startswith("{}.".format(right.norm)) + return left.string.startswith("{}.".format(right.string)) if left.epoch != right.epoch: return False @@ -728,9 +738,15 @@ def _version_compatible(left, right): # https://peps.python.org/pep-0440/#compatible-release # Note, the ~= operator can be also expressed as: # >= V.N, == V.* - head, _, _ = right.norm.partition(".") - right_star = version("{}.*".format(head)) - return _version_ge(left, right) and _version_eq(left, right_star) + + right_star = ".".join([str(d) for d in right.release[:-1]]) + if right_star: + right_star = "{}.".format(right_star) + + # TODO @aignas 2025-05-09: more tests: + # epoch + # negative tests + return _version_ge(left, right) and left.string.startswith(right_star) def _version_ne(left, right): """!= operator""" @@ -820,11 +836,12 @@ def _version_ge(left, right): _right = _version_key(right, local = False) return _left > _right or _version_eq(left, right) -def _version_key(self, *, local, release_key = ("z",)): +def _version_key(self, *, local = True): """This function returns a tuple that can be used in 'sorted' calls. This implements the PEP440 version sorting. """ + release_key = ("z",) local = self.local if local else [] local = local or [] From ea6c1272580e3f070b9fbfd544ad5e8aee31602b Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 9 May 2025 18:16:49 +0900 Subject: [PATCH 44/57] reorder the code --- python/private/version.bzl | 53 ++++++++++++++------------------------ 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 227f32124d..3d4e5e15ec 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -581,7 +581,16 @@ def version(version_str, strict = False): if not parts: return None - return _new_version(**parts) + return _new_version( + epoch = _parse_epoch(parts["epoch"]), + 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"]), + string = parts["norm"], + is_prefix = parts["is_prefix"], + ) def _parse_epoch(value): if not value: @@ -635,48 +644,24 @@ def _parse_post(value): # it. Use `ord` and `chr` functions to find a good value. return ("~", post) -def _new_version( - *, - norm, - epoch = 0, - release, - pre = "", - post = "", - dev = "", - local = "", - is_prefix = False): - self = struct( - epoch = _parse_epoch(epoch), - release = _parse_release(release), - pre = _parse_pre(pre), - post = _parse_post(post), - dev = _parse_dev(dev), - local = _parse_local(local), - is_prefix = is_prefix, - string = norm, - ) +def _new_version(**kwargs): + self = struct(**kwargs) - public = struct( + return struct( + # methods, keep sorted + compatible = mkmethod(self, _version_compatible), eq = mkmethod(self, _version_eq), eqq = mkmethod(self, _version_eqq), ge = mkmethod(self, _version_ge), gt = mkmethod(self, _version_gt), + key = mkmethod(self, _version_key), le = mkmethod(self, _version_le), lt = mkmethod(self, _version_lt), ne = mkmethod(self, _version_ne), - compatible = mkmethod(self, _version_compatible), - key = mkmethod(self, _version_key), - epoch = self.epoch, - release = self.release, - pre = self.pre, - post = self.post, - dev = self.dev, - local = self.local, - is_prefix = self.is_prefix, - string = norm, - ) - return public + # attrs are the same as self + **kwargs + ) def _pad_zeros(release, n): padding = n - len(release) From cbbb345896b10ae090966343d453db9f3dece57a Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 10 May 2025 09:35:23 +0900 Subject: [PATCH 45/57] refactor: do not use object oriented style --- python/private/py_wheel.bzl | 4 +-- python/private/pypi/pep508_evaluate.bzl | 21 +++++++------- python/private/version.bzl | 38 +++++++++++-------------- tests/py_wheel/py_wheel_tests.bzl | 4 +-- tests/pypi/pep508/evaluate_tests.bzl | 17 ++++++----- 5 files changed, 41 insertions(+), 43 deletions(-) diff --git a/python/private/py_wheel.bzl b/python/private/py_wheel.bzl index 4c8ea22bfa..5ce6088372 100644 --- a/python/private/py_wheel.bzl +++ b/python/private/py_wheel.bzl @@ -17,7 +17,7 @@ load(":py_info.bzl", "PyInfo") load(":py_package.bzl", "py_package_lib") load(":stamp.bzl", "is_stamping_enabled") -load(":version.bzl", "normalize_pep440") +load(":version.bzl", "version") PyWheelInfo = provider( doc = "Information about a wheel produced by `py_wheel`", @@ -310,7 +310,7 @@ def _py_wheel_impl(ctx): filename_segments = [ _escape_filename_distribution_name(ctx.attr.distribution), - normalize_pep440(version), + version.normalize(version), _escape_filename_segment(python_tag), _escape_filename_segment(abi), _escape_filename_segment(ctx.attr.platform), diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index a1e3266571..61a5b19999 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -345,8 +345,8 @@ def _env_expr(left, op, right): def _version_expr(left, op, right): """Evaluate a version comparison expression""" - _left = version(left) - _right = version(right) + _left = version.parse(left) + _right = version.parse(right) if _left == None or _right == None: # Per spec, if either can't be normalized to a version, then # fallback to simple string comparison. Usually this is `platform_version` @@ -354,22 +354,21 @@ def _version_expr(left, op, right): return _env_expr(left, op, right) if op == "===": - return _left.eqq(_right) + return version.is_eeq(_left, _right) elif op == "!=": - return _left.ne(_right) + return version.is_ne(_left, _right) elif op == "==": - # Matching of major, minor, patch only - return _left.eq(_right) + return version.is_eq(_left, _right) elif op == "<": - return _left.lt(_right) + return version.is_lt(_left, _right) elif op == ">": - return _left.gt(_right) + return version.is_gt(_left, _right) elif op == "<=": - return _left.le(_right) + return version.is_le(_left, _right) elif op == ">=": - return _left.ge(_right) + return version.is_ge(_left, _right) elif op == "~=": - return _left.compatible(_right) + return version.is_compatible(_left, _right) else: return False # Let's just ignore the invalid ops diff --git a/python/private/version.bzl b/python/private/version.bzl index 3d4e5e15ec..57562eb707 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -563,7 +563,7 @@ def _parse(version_str, strict = True): return parts -def version(version_str, strict = False): +def parse(version_str, strict = False): """Parse a PEP4408 compliant version See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode @@ -581,7 +581,7 @@ def version(version_str, strict = False): if not parts: return None - return _new_version( + return struct( epoch = _parse_epoch(parts["epoch"]), release = _parse_release(parts["release"]), pre = _parse_pre(parts["pre"]), @@ -644,25 +644,6 @@ def _parse_post(value): # it. Use `ord` and `chr` functions to find a good value. return ("~", post) -def _new_version(**kwargs): - self = struct(**kwargs) - - return struct( - # methods, keep sorted - compatible = mkmethod(self, _version_compatible), - eq = mkmethod(self, _version_eq), - eqq = mkmethod(self, _version_eqq), - ge = mkmethod(self, _version_ge), - gt = mkmethod(self, _version_gt), - key = mkmethod(self, _version_key), - le = mkmethod(self, _version_le), - lt = mkmethod(self, _version_lt), - ne = mkmethod(self, _version_ne), - - # attrs are the same as self - **kwargs - ) - def _pad_zeros(release, n): padding = n - len(release) if padding <= 0: @@ -846,3 +827,18 @@ def _version_key(self, *, local = True): # PEP440 - post release ordering: .devN, self.dev or release_key, ) + +version = struct( + normalize = normalize_pep440, + parse = parse, + # methods, keep sorted + key = _version_key, + is_compatible = _version_compatible, + is_eq = _version_eq, + is_eeq = _version_eqq, + is_ge = _version_ge, + is_gt = _version_gt, + is_le = _version_le, + is_lt = _version_lt, + is_ne = _version_ne, +) diff --git a/tests/py_wheel/py_wheel_tests.bzl b/tests/py_wheel/py_wheel_tests.bzl index b18a5a6f09..6d15947be7 100644 --- a/tests/py_wheel/py_wheel_tests.bzl +++ b/tests/py_wheel/py_wheel_tests.bzl @@ -17,7 +17,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") load("@rules_testing//lib:truth.bzl", "matching") load("@rules_testing//lib:util.bzl", rt_util = "util") load("//python:packaging.bzl", "py_wheel") -load("//python/private:version.bzl", "normalize_pep440") # buildifier: disable=bzl-visibility +load("//python/private:version.bzl", "version") # buildifier: disable=bzl-visibility _basic_tests = [] _tests = [] @@ -257,7 +257,7 @@ def _test_pep440_normalization(env): prefix = prefixes[i % len(prefixes)] postfix = postfixes[(i // len(prefixes)) % len(postfixes)] env.expect.that_str( - normalize_pep440( + version.normalize( prefix + iepoch + irelease + iprepost + idev + ilocal + postfix, ), diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 67c3d3596c..6c02fa9d49 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -341,15 +341,18 @@ def _test_ordering(env): ] for lower, higher in zip(want[:-1], want[1:]): - lower = version(lower, strict = True) - higher = version(higher, strict = True) + lower = version.parse(lower, strict = True) + higher = version.parse(higher, strict = True) - if not lower.key() < higher.key(): + lower_key = version.key(lower) + higher_key = version.key(higher) + + if not lower_key < higher_key: env.fail("Expected '{}'.key() to be smaller than '{}'.key(), but got otherwise: {} > {}".format( - lower.str(), - higher.str(), - lower.key(), - higher.key(), + lower.string, + higher.string, + lower_key, + higher_key, )) _tests.append(_test_ordering) From c56ac3229494ad295f34114de57806087606658b Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 10 May 2025 09:38:08 +0900 Subject: [PATCH 46/57] fix epoch parsing --- python/private/version.bzl | 5 ++++- tests/pypi/pep508/evaluate_tests.bzl | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 57562eb707..3f6f36f52d 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -596,7 +596,10 @@ def _parse_epoch(value): if not value: return 0 - return int(value) + if not value.endswith("!"): + fail("epoch string segment needs to end with '!', got: {}".format(value)) + + return int(value[:-1]) def _parse_release(value): return tuple([int(d) for d in value.split(".")]) diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 6c02fa9d49..4b66edf2cf 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -338,6 +338,7 @@ def _test_ordering(env): "1.0.post456", "1.0.15", "1.1.dev1", + "1!0.1", ] for lower, higher in zip(want[:-1], want[1:]): From f7eec915ec6055c74523e0d2ac057f886357d8e0 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 10 May 2025 09:45:14 +0900 Subject: [PATCH 47/57] finish adding tests --- python/private/version.bzl | 10 ++++++---- tests/pypi/pep508/evaluate_tests.bzl | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 3f6f36f52d..4effacdd0c 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -608,10 +608,11 @@ def _parse_local(value): if not value: return None - local = value.lstrip("+") + if not value.startswith("+"): + fail("local release identifier must start with '+', got: {}".format(value)) # If the part is numerical, handle it as a number - return tuple([int(part) if part.isdigit() else part for part in local.split(".")]) + return tuple([int(part) if part.isdigit() else part for part in value[1:].split(".")]) def _parse_dev(value): if not value: @@ -709,11 +710,12 @@ def _version_compatible(left, right): # >= V.N, == V.* right_star = ".".join([str(d) for d in right.release[:-1]]) - if right_star: + if right.epoch: + right_star = "{}!{}.".format(right.epoch, right_star) + else: right_star = "{}.".format(right_star) # TODO @aignas 2025-05-09: more tests: - # epoch # negative tests return _version_ge(left, right) and left.string.startswith(right_star) diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 4b66edf2cf..4af5f659bd 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -305,6 +305,11 @@ _MISC_EXPRESSIONS = [ _expr_case('python_version ~= "2.2.post3"', False, {"python_version": "2.2"}), _expr_case('python_version ~= "2.2.post3"', True, {"python_version": "2.3"}), _expr_case('python_version ~= "2.2.post3"', False, {"python_version": "3.0"}), + _expr_case('python_version ~= "1!2.2"', False, {"python_version": "2.7"}), + _expr_case('python_version ~= "0!2.2"', True, {"python_version": "2.7"}), + _expr_case('python_version ~= "1!2.2"', True, {"python_version": "1!2.7"}), + _expr_case('python_version ~= "1.2.3"', True, {"python_version": "1.2.4"}), + _expr_case('python_version ~= "1.2.3"', False, {"python_version": "1.3.2"}), ] def _misc_expressions(env): From ff23111abae0d51f335e397f3690ea74ba5a5278 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 10 May 2025 10:10:13 +0900 Subject: [PATCH 48/57] simplify the parsing function --- python/private/version.bzl | 98 ++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 46 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 4effacdd0c..14e2e12b7e 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -59,18 +59,23 @@ def _open_context(self): self.contexts.append(_ctx(_context(self)["start"])) return self.contexts[-1] -def _accept(self): +def _accept(self, key = None): """Close the current ctx successfully and merge the results.""" finished = self.contexts.pop() self.contexts[-1]["norm"] += finished["norm"] + if key: + self.contexts[-1][key] = finished["norm"] + self.contexts[-1]["start"] = finished["start"] return True def _context(self): return self.contexts[-1] -def _discard(self): +def _discard(self, key = None): self.contexts.pop() + if key: + self.contexts[-1][key] = "" return False def _new(input): @@ -313,9 +318,9 @@ def accept_epoch(parser): if accept_digits(parser) and accept(parser, _is("!"), "!"): if ctx["norm"] == "0!": ctx["norm"] = "" - return parser.accept() + return parser.accept("epoch") else: - return parser.discard() + return parser.discard("epoch") def accept_release(parser): """Accept the release segment, numbers separated by dots. @@ -329,10 +334,10 @@ def accept_release(parser): parser.open_context() if not accept_digits(parser): - return parser.discard() + return parser.discard("release") accept_dot_number_sequence(parser) - return parser.accept() + return parser.accept("release") def accept_pre_l(parser): """PEP 440: Pre-release spelling. @@ -374,7 +379,7 @@ def accept_prerelease(parser): accept(parser, _in(["-", "_", "."]), "") if not accept_pre_l(parser): - return parser.discard() + return parser.discard("pre") accept(parser, _in(["-", "_", "."]), "") @@ -382,7 +387,7 @@ def accept_prerelease(parser): # PEP 440: Implicit pre-release number ctx["norm"] += "0" - return parser.accept() + return parser.accept("pre") def accept_implicit_postrelease(parser): """PEP 440: Implicit post releases. @@ -444,9 +449,9 @@ def accept_postrelease(parser): parser.open_context() if accept_implicit_postrelease(parser) or accept_explicit_postrelease(parser): - return parser.accept() + return parser.accept("post") - return parser.discard() + return parser.discard("post") def accept_devrelease(parser): """PEP 440: Developmental releases. @@ -470,9 +475,9 @@ def accept_devrelease(parser): # PEP 440: Implicit development release number ctx["norm"] += "0" - return parser.accept() + return parser.accept("dev") - return parser.discard() + return parser.discard("dev") def accept_local(parser): """PEP 440: Local version identifiers. @@ -487,9 +492,9 @@ def accept_local(parser): if accept(parser, _is("+"), "+") and accept_alnum(parser): accept_separator_alnum_sequence(parser) - return parser.accept() + return parser.accept("local") - return parser.discard() + return parser.discard("local") def normalize_pep440(version): """Escape the version component of a filename. @@ -518,63 +523,66 @@ def _parse(version_str, strict = True): Returns: string containing the normalized version. """ - version_str = version_str.strip() # PEP 440: Leading and Trailing Whitespace + version = version_str.strip() # PEP 440: Leading and Trailing Whitespace is_prefix = False if not strict: - is_prefix = version_str.endswith(".*") - version_str = version_str.strip(" .*") # PEP 440: Leading and Trailing Whitespace and ".*" + is_prefix = version.endswith(".*") + version = version.strip(" .*") # PEP 440: Leading and Trailing Whitespace and ".*" - parser = _new(version_str) + parser = _new(version) accept(parser, _is("v"), "") # PEP 440: Preceding v character - fns = [ - ("epoch", accept_epoch), - ("release", accept_release), - ("pre", accept_prerelease), - ("post", accept_postrelease), - ("dev", accept_devrelease), - ("local", accept_local), - ] - parts = { - "is_prefix": is_prefix, - } - for key, fn in fns: - start = len(parser.context()["norm"]) - fn(parser) - parts[key] = parser.context()["norm"][start:] - parts["norm"] = parser.context()["norm"] - - # TODO @aignas 2025-05-09: move the `is_prefix` handling to `accept_release` - if is_prefix and (parts["local"] or parts["post"] or parts["dev"] or parts["pre"]): + accept_epoch(parser) + accept_release(parser) + accept_prerelease(parser) + accept_postrelease(parser) + accept_devrelease(parser) + accept_local(parser) + + parser_ctx = parser.context() + if is_prefix and (parser_ctx["local"] or parser_ctx["post"] or parser_ctx["dev"] or parser_ctx["pre"]): if strict: 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 - if parser.input[parser.context()["start"]:]: + if parser.input[parser_ctx["start"]:]: if strict: fail( "Failed to parse PEP 440 version identifier '%s'." % parser.input, - "Parse error at '%s'" % parser.input[parser.context()["start"]:], + "Parse error at '%s'" % parser.input[parser_ctx["start"]:], ) return None - return parts + parser_ctx["is_prefix"] = is_prefix + return parser_ctx def parse(version_str, strict = False): - """Parse a PEP4408 compliant version + """Parse a PEP4408 compliant version. - See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode - and https://peps.python.org/pep-0440/ + This is similar to `normalize_pep440`, but it parsers individual components to to + comparable types. Args: version_str: version string to be normalized according to PEP 440. strict: fail if the version is invalid. Returns: - string containing the normalized version. + a struct with individual components of a version: + * `epoch` {type}`int`, defaults to `0` + * `release` {type}`tuple[int]` an n-tuple of ints + * `pre` {type}`tuple[str, int] | None` a tuple of a string and an int, + e.g. ("a", 1) + * `post` {type}`tuple[str, int] | None` a tuple of a string and an int, + e.g. ("~", 1) + * `dev` {type}`tuple[str, int] | None` a tuple of a string and an int, + e.g. ("", 1) + * `local` {type}`tuple[str, int] | None` a tuple of components in the local + version, e.g. ("abc", 123). + * `is_prefix` {type}`bool` whether the version_str ends with `.*`. + * `string` {type}`str` normalized value of the input. """ parts = _parse(version_str, strict = strict) @@ -715,8 +723,6 @@ def _version_compatible(left, right): else: right_star = "{}.".format(right_star) - # TODO @aignas 2025-05-09: more tests: - # negative tests return _version_ge(left, right) and left.string.startswith(right_star) def _version_ne(left, right): From 90ce28c9978220a44b3303bc7adb97f363a791ff Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 10 May 2025 10:14:14 +0900 Subject: [PATCH 49/57] add changelog --- CHANGELOG.md | 2 ++ python/private/version.bzl | 14 +++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9cb14459d..48a75a85ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,8 @@ END_UNRELEASED_TEMPLATE multiple times. * (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file to specify the requirements. +* (pypi) Starlark implementation of the marker evaluation now supports the full PEP440 + spec for version comparison. {#v0-0-0-added} ### Added diff --git a/python/private/version.bzl b/python/private/version.bzl index 14e2e12b7e..eedbeb0b8a 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -540,13 +540,6 @@ def _parse(version_str, strict = True): accept_local(parser) parser_ctx = parser.context() - if is_prefix and (parser_ctx["local"] or parser_ctx["post"] or parser_ctx["dev"] or parser_ctx["pre"]): - if strict: - 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 - if parser.input[parser_ctx["start"]:]: if strict: fail( @@ -589,6 +582,13 @@ def parse(version_str, strict = False): 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") + + # https://peps.python.org/pep-0440/#public-version-identifiers + return None + return struct( epoch = _parse_epoch(parts["epoch"]), release = _parse_release(parts["release"]), From 9c8d123055196089c382d38671e14bcc497e2bf3 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 10 May 2025 10:24:39 +0900 Subject: [PATCH 50/57] fixup --- python/private/py_wheel.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/private/py_wheel.bzl b/python/private/py_wheel.bzl index 5ce6088372..ffc24f6846 100644 --- a/python/private/py_wheel.bzl +++ b/python/private/py_wheel.bzl @@ -306,11 +306,11 @@ def _input_file_to_arg(input_file): def _py_wheel_impl(ctx): abi = _replace_make_variables(ctx.attr.abi, ctx) python_tag = _replace_make_variables(ctx.attr.python_tag, ctx) - version = _replace_make_variables(ctx.attr.version, ctx) + version_str = _replace_make_variables(ctx.attr.version, ctx) filename_segments = [ _escape_filename_distribution_name(ctx.attr.distribution), - version.normalize(version), + version.normalize(version_str), _escape_filename_segment(python_tag), _escape_filename_segment(abi), _escape_filename_segment(ctx.attr.platform), @@ -343,7 +343,7 @@ def _py_wheel_impl(ctx): args = ctx.actions.args() args.add("--name", ctx.attr.distribution) - args.add("--version", version) + args.add("--version", version_str) args.add("--python_tag", python_tag) args.add("--abi", abi) args.add("--platform", ctx.attr.platform) From ab1972d0cabc9446fb077f21b66be04e00c7adbd Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 10 May 2025 10:34:47 +0900 Subject: [PATCH 51/57] move the pep440 version tests to its own file --- .bazelrc | 4 +- tests/py_wheel/py_wheel_tests.bzl | 101 ----------------- tests/pypi/pep508/evaluate_tests.bzl | 46 -------- tests/version/BUILD.bazel | 3 + tests/version/version_test.bzl | 157 +++++++++++++++++++++++++++ 5 files changed, 162 insertions(+), 149 deletions(-) create mode 100644 tests/version/BUILD.bazel create mode 100644 tests/version/version_test.bzl diff --git a/.bazelrc b/.bazelrc index d2e0721526..4e6f2fa187 100644 --- a/.bazelrc +++ b/.bazelrc @@ -4,8 +4,8 @@ # (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it) # To update these lines, execute # `bazel run @rules_bazel_integration_test//tools:update_deleted_packages` -build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/python/private,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma -query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/python/private,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma +build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,gazelle/python/private,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma +query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,gazelle/python/private,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma test --test_output=errors diff --git a/tests/py_wheel/py_wheel_tests.bzl b/tests/py_wheel/py_wheel_tests.bzl index 6d15947be7..43c068e597 100644 --- a/tests/py_wheel/py_wheel_tests.bzl +++ b/tests/py_wheel/py_wheel_tests.bzl @@ -17,7 +17,6 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") load("@rules_testing//lib:truth.bzl", "matching") load("@rules_testing//lib:util.bzl", rt_util = "util") load("//python:packaging.bzl", "py_wheel") -load("//python/private:version.bzl", "version") # buildifier: disable=bzl-visibility _basic_tests = [] _tests = [] @@ -168,106 +167,6 @@ def _test_content_type_from_description_impl(env, target): _tests.append(_test_content_type_from_description) -def _test_pep440_normalization(env): - prefixes = ["v", " v", " \t\r\nv"] - epochs = { - "": ["", "0!", "00!"], - "1!": ["1!", "001!"], - "200!": ["200!", "00200!"], - } - releases = { - "0.1": ["0.1", "0.01"], - "2023.7.19": ["2023.7.19", "2023.07.19"], - } - pres = { - "": [""], - "a0": ["a", ".a", "-ALPHA0", "_alpha0", ".a0"], - "a4": ["alpha4", ".a04"], - "b0": ["b", ".b", "-BETA0", "_beta0", ".b0"], - "b5": ["beta05", ".b5"], - "rc0": ["C", "_c0", "RC", "_rc0", "-preview_0"], - } - explicit_posts = { - "": [""], - ".post0": [], - ".post1": [".post1", "-r1", "_rev1"], - } - implicit_posts = [[".post1", "-1"], [".post2", "-2"]] - devs = { - "": [""], - ".dev0": ["dev", "-DEV", "_Dev-0"], - ".dev9": ["DEV9", ".dev09", ".dev9"], - ".dev{BUILD_TIMESTAMP}": [ - "-DEV{BUILD_TIMESTAMP}", - "_dev_{BUILD_TIMESTAMP}", - ], - } - locals = { - "": [""], - "+ubuntu.7": ["+Ubuntu_7", "+ubuntu-007"], - "+ubuntu.r007": ["+Ubuntu_R007"], - } - epochs = [ - [normalized_epoch, input_epoch] - for normalized_epoch, input_epochs in epochs.items() - for input_epoch in input_epochs - ] - releases = [ - [normalized_release, input_release] - for normalized_release, input_releases in releases.items() - for input_release in input_releases - ] - pres = [ - [normalized_pre, input_pre] - for normalized_pre, input_pres in pres.items() - for input_pre in input_pres - ] - explicit_posts = [ - [normalized_post, input_post] - for normalized_post, input_posts in explicit_posts.items() - for input_post in input_posts - ] - pres_and_posts = [ - [normalized_pre + normalized_post, input_pre + input_post] - for normalized_pre, input_pre in pres - for normalized_post, input_post in explicit_posts - ] + [ - [normalized_pre + normalized_post, input_pre + input_post] - for normalized_pre, input_pre in pres - for normalized_post, input_post in implicit_posts - if input_pre == "" or input_pre[-1].isdigit() - ] - devs = [ - [normalized_dev, input_dev] - for normalized_dev, input_devs in devs.items() - for input_dev in input_devs - ] - locals = [ - [normalized_local, input_local] - for normalized_local, input_locals in locals.items() - for input_local in input_locals - ] - postfixes = ["", " ", " \t\r\n"] - i = 0 - for nepoch, iepoch in epochs: - for nrelease, irelease in releases: - for nprepost, iprepost in pres_and_posts: - for ndev, idev in devs: - for nlocal, ilocal in locals: - prefix = prefixes[i % len(prefixes)] - postfix = postfixes[(i // len(prefixes)) % len(postfixes)] - env.expect.that_str( - version.normalize( - prefix + iepoch + irelease + iprepost + - idev + ilocal + postfix, - ), - ).equals( - nepoch + nrelease + nprepost + ndev + nlocal, - ) - i += 1 - -_basic_tests.append(_test_pep440_normalization) - def py_wheel_test_suite(name): test_suite( name = name, diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 4af5f659bd..7b6c064b94 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -14,7 +14,6 @@ """Tests for construction of Python version matching config settings.""" load("@rules_testing//lib:test_suite.bzl", "test_suite") -load("//python/private:version.bzl", "version") # buildifier: disable=bzl-visibility load("//python/private/pypi:pep508_env.bzl", pep508_env = "env") # buildifier: disable=bzl-visibility load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize") # buildifier: disable=bzl-visibility @@ -318,51 +317,6 @@ def _misc_expressions(env): _tests.append(_misc_expressions) -def _test_ordering(env): - want = [ - # Taken from https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering - "1.dev0", - "1.0.dev456", - "1.0a1", - "1.0a2.dev456", - "1.0a12.dev456", - "1.0a12", - "1.0b1.dev456", - "1.0b1.dev457", - "1.0b2", - "1.0b2.post345.dev456", - "1.0b2.post345.dev457", - "1.0b2.post345", - "1.0rc1.dev456", - "1.0rc1", - "1.0", - "1.0+abc.5", - "1.0+abc.7", - "1.0+5", - "1.0.post456.dev34", - "1.0.post456", - "1.0.15", - "1.1.dev1", - "1!0.1", - ] - - for lower, higher in zip(want[:-1], want[1:]): - lower = version.parse(lower, strict = True) - higher = version.parse(higher, strict = True) - - lower_key = version.key(lower) - higher_key = version.key(higher) - - if not lower_key < higher_key: - env.fail("Expected '{}'.key() to be smaller than '{}'.key(), but got otherwise: {} > {}".format( - lower.string, - higher.string, - lower_key, - higher_key, - )) - -_tests.append(_test_ordering) - def evaluate_test_suite(name): # buildifier: disable=function-docstring test_suite( name = name, diff --git a/tests/version/BUILD.bazel b/tests/version/BUILD.bazel new file mode 100644 index 0000000000..d6fdecd4cf --- /dev/null +++ b/tests/version/BUILD.bazel @@ -0,0 +1,3 @@ +load(":version_test.bzl", "version_test_suite") + +version_test_suite(name = "version_tests") diff --git a/tests/version/version_test.bzl b/tests/version/version_test.bzl new file mode 100644 index 0000000000..589f9ac05d --- /dev/null +++ b/tests/version/version_test.bzl @@ -0,0 +1,157 @@ +"" + +load("@rules_testing//lib:analysis_test.bzl", "test_suite") +load("//python/private:version.bzl", "version") # buildifier: disable=bzl-visibility + +_tests = [] + +def _test_normalization(env): + prefixes = ["v", " v", " \t\r\nv"] + epochs = { + "": ["", "0!", "00!"], + "1!": ["1!", "001!"], + "200!": ["200!", "00200!"], + } + releases = { + "0.1": ["0.1", "0.01"], + "2023.7.19": ["2023.7.19", "2023.07.19"], + } + pres = { + "": [""], + "a0": ["a", ".a", "-ALPHA0", "_alpha0", ".a0"], + "a4": ["alpha4", ".a04"], + "b0": ["b", ".b", "-BETA0", "_beta0", ".b0"], + "b5": ["beta05", ".b5"], + "rc0": ["C", "_c0", "RC", "_rc0", "-preview_0"], + } + explicit_posts = { + "": [""], + ".post0": [], + ".post1": [".post1", "-r1", "_rev1"], + } + implicit_posts = [[".post1", "-1"], [".post2", "-2"]] + devs = { + "": [""], + ".dev0": ["dev", "-DEV", "_Dev-0"], + ".dev9": ["DEV9", ".dev09", ".dev9"], + ".dev{BUILD_TIMESTAMP}": [ + "-DEV{BUILD_TIMESTAMP}", + "_dev_{BUILD_TIMESTAMP}", + ], + } + locals = { + "": [""], + "+ubuntu.7": ["+Ubuntu_7", "+ubuntu-007"], + "+ubuntu.r007": ["+Ubuntu_R007"], + } + epochs = [ + [normalized_epoch, input_epoch] + for normalized_epoch, input_epochs in epochs.items() + for input_epoch in input_epochs + ] + releases = [ + [normalized_release, input_release] + for normalized_release, input_releases in releases.items() + for input_release in input_releases + ] + pres = [ + [normalized_pre, input_pre] + for normalized_pre, input_pres in pres.items() + for input_pre in input_pres + ] + explicit_posts = [ + [normalized_post, input_post] + for normalized_post, input_posts in explicit_posts.items() + for input_post in input_posts + ] + pres_and_posts = [ + [normalized_pre + normalized_post, input_pre + input_post] + for normalized_pre, input_pre in pres + for normalized_post, input_post in explicit_posts + ] + [ + [normalized_pre + normalized_post, input_pre + input_post] + for normalized_pre, input_pre in pres + for normalized_post, input_post in implicit_posts + if input_pre == "" or input_pre[-1].isdigit() + ] + devs = [ + [normalized_dev, input_dev] + for normalized_dev, input_devs in devs.items() + for input_dev in input_devs + ] + locals = [ + [normalized_local, input_local] + for normalized_local, input_locals in locals.items() + for input_local in input_locals + ] + postfixes = ["", " ", " \t\r\n"] + i = 0 + for nepoch, iepoch in epochs: + for nrelease, irelease in releases: + for nprepost, iprepost in pres_and_posts: + for ndev, idev in devs: + for nlocal, ilocal in locals: + prefix = prefixes[i % len(prefixes)] + postfix = postfixes[(i // len(prefixes)) % len(postfixes)] + env.expect.that_str( + version.normalize( + prefix + iepoch + irelease + iprepost + + idev + ilocal + postfix, + ), + ).equals( + nepoch + nrelease + nprepost + ndev + nlocal, + ) + i += 1 + +_tests.append(_test_normalization) + +def _test_ordering(env): + want = [ + # Taken from https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering + "1.dev0", + "1.0.dev456", + "1.0a1", + "1.0a2.dev456", + "1.0a12.dev456", + "1.0a12", + "1.0b1.dev456", + "1.0b1.dev457", + "1.0b2", + "1.0b2.post345.dev456", + "1.0b2.post345.dev457", + "1.0b2.post345", + "1.0rc1.dev456", + "1.0rc1", + "1.0", + "1.0+abc.5", + "1.0+abc.7", + "1.0+5", + "1.0.post456.dev34", + "1.0.post456", + "1.0.15", + "1.1.dev1", + "1!0.1", + ] + + for lower, higher in zip(want[:-1], want[1:]): + lower = version.parse(lower, strict = True) + higher = version.parse(higher, strict = True) + + lower_key = version.key(lower) + higher_key = version.key(higher) + + if not lower_key < higher_key: + env.fail("Expected '{}'.key() to be smaller than '{}'.key(), but got otherwise: {} > {}".format( + lower.string, + higher.string, + lower_key, + higher_key, + )) + +_tests.append(_test_ordering) + +def version_test_suite(name): + test_suite( + name = name, + basic_tests = _tests, + ) From 9a22e1f083a6e365036b186051d8df29946816fb Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 10 May 2025 17:00:18 +0900 Subject: [PATCH 52/57] comment: rename a leftover eqq from previous times --- python/private/version.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index eedbeb0b8a..a70feea593 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -672,7 +672,7 @@ def _prefix_err(left, op, right): right.string, )) -def _version_eqq(left, right): +def _version_eeq(left, right): """=== operator""" if left.is_prefix or right.is_prefix: fail(_prefix_err(left, "===", right)) @@ -846,7 +846,7 @@ version = struct( key = _version_key, is_compatible = _version_compatible, is_eq = _version_eq, - is_eeq = _version_eqq, + is_eeq = _version_eeq, is_ge = _version_ge, is_gt = _version_gt, is_le = _version_le, From 8c90d9f4d51e0913440e525a177748b06fb48cfd Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 10 May 2025 14:47:14 -0700 Subject: [PATCH 53/57] consolidate changelog entry --- CHANGELOG.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab3a11d1fe..aa7fc9d415 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,8 +86,6 @@ END_UNRELEASED_TEMPLATE multiple times. * (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file to specify the requirements. -* (pypi) Starlark implementation of the marker evaluation now supports the full PEP440 - spec for version comparison. {#v0-0-0-added} ### Added @@ -96,9 +94,9 @@ END_UNRELEASED_TEMPLATE (the default), the subprocess's stdout/stderr will be logged. * (toolchains) Local toolchains can be activated with custom flags. See [Conditionally using local toolchains] docs for how to configure. -* (pypi) `RULES_PYTHON_ENABLE_PIPSTAR` environment variable: when `1`, the Starlark - implementation of wheel METADATA parsing is used (which has improved multi-platform - build support). +* (pypi) Starlark-based evaluation of environment markers (requirements.txt conditionals) + available (not enabled by default) for improved multi-platform build support. + Set the `RULES_PYTHON_ENABLE_PIPSTAR=1` environment variable to enable it. {#v0-0-0-removed} ### Removed From 655cd52e3fc508d6e54bd6ba8006bc413368f234 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 10 May 2025 14:48:15 -0700 Subject: [PATCH 54/57] Update python/private/version.bzl --- python/private/version.bzl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index a70feea593..61d5fe54c4 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -523,7 +523,8 @@ def _parse(version_str, strict = True): Returns: string containing the normalized version. """ - version = version_str.strip() # PEP 440: Leading and Trailing Whitespace + # https://packaging.python.org/en/latest/specifications/version-specifiers/#leading-and-trailing-whitespace + version = version_str.strip() is_prefix = False if not strict: From c837f10ab9370867c0af5dbb3ec45e20c122e237 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 10 May 2025 14:48:22 -0700 Subject: [PATCH 55/57] Update python/private/version.bzl --- python/private/version.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 61d5fe54c4..0fcd0fdcb9 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -556,7 +556,7 @@ def _parse(version_str, strict = True): def parse(version_str, strict = False): """Parse a PEP4408 compliant version. - This is similar to `normalize_pep440`, but it parsers individual components to to + This is similar to `normalize_pep440`, but it parses individual components to comparable types. Args: From 188a9e717068a781d38df5efb1bbfd5f2c7b5cd2 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 10 May 2025 14:48:30 -0700 Subject: [PATCH 56/57] Update python/private/version.bzl --- python/private/version.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/python/private/version.bzl b/python/private/version.bzl index 0fcd0fdcb9..09afcc77ef 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -706,7 +706,6 @@ def _version_eq(left, right): left.post == right.post and left.dev == right.dev # local is ignored for == checks - ##and left.local == right.local ) def _version_compatible(left, right): From 773b1a7626d8bf981b1247b38651dcdd681c6445 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 11 May 2025 11:18:46 +0900 Subject: [PATCH 57/57] run buildifier --- python/private/version.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/python/private/version.bzl b/python/private/version.bzl index 09afcc77ef..4425cc7661 100644 --- a/python/private/version.bzl +++ b/python/private/version.bzl @@ -523,6 +523,7 @@ def _parse(version_str, strict = True): Returns: string containing the normalized version. """ + # https://packaging.python.org/en/latest/specifications/version-specifiers/#leading-and-trailing-whitespace version = version_str.strip() is_prefix = False