From 2c75b5155f2e114108696066cba76c9fe9bef49a Mon Sep 17 00:00:00 2001
From: aignas <240938+aignas@users.noreply.github.com>
Date: Fri, 13 Sep 2024 10:47:37 +0900
Subject: [PATCH 1/4] refactor: add a semver parsing utility function

Work towards #2081.
---
 examples/bzlmod/MODULE.bazel.lock |  4 +-
 python/private/BUILD.bazel        |  5 ++
 python/private/pypi/BUILD.bazel   |  1 +
 python/private/pypi/extension.bzl | 17 +-----
 python/private/semver.bzl         | 61 ++++++++++++++++++++
 tests/semver/BUILD.bazel          | 17 ++++++
 tests/semver/semver_test.bzl      | 94 +++++++++++++++++++++++++++++++
 7 files changed, 182 insertions(+), 17 deletions(-)
 create mode 100644 python/private/semver.bzl
 create mode 100644 tests/semver/BUILD.bazel
 create mode 100644 tests/semver/semver_test.bzl

diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock
index d747ed35d7..05cf5305f6 100644
--- a/examples/bzlmod/MODULE.bazel.lock
+++ b/examples/bzlmod/MODULE.bazel.lock
@@ -1231,7 +1231,7 @@
     },
     "@@rules_python~//python/extensions:pip.bzl%pip": {
       "general": {
-        "bzlTransitiveDigest": "vzdh1M3LRVqyF10AVUO1+FOE7CZwlZaFT+7RgQ4OKXg=",
+        "bzlTransitiveDigest": "nhEjP1p9Faj4/hZ9PWLLgzxuDEZEPXXN/TixJkcqqD8=",
         "usagesDigest": "MChlcSw99EuW3K7OOoMcXQIdcJnEh6YmfyjJm+9mxIg=",
         "recordedFileInputs": {
           "@@other_module~//requirements_lock_3_11.txt": "a7d0061366569043d5efcf80e34a32c732679367cb3c831c4cdc606adc36d314",
@@ -6140,7 +6140,7 @@
     },
     "@@rules_python~//python/private/pypi:pip.bzl%pip_internal": {
       "general": {
-        "bzlTransitiveDigest": "TgRegkReKbGzK4VxYz9up697gcf5Q8NFuZYnZHryck8=",
+        "bzlTransitiveDigest": "pmzJqpzKkz0ZOFUH8h0tza6o/nTvdsrYHHYT80PCs+U=",
         "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=",
         "recordedFileInputs": {
           "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a",
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
index 7b913df2b3..b688b71bf0 100644
--- a/python/private/BUILD.bazel
+++ b/python/private/BUILD.bazel
@@ -296,6 +296,11 @@ bzl_library(
     srcs = ["repo_utils.bzl"],
 )
 
+bzl_library(
+    name = "semver_bzl",
+    srcs = ["semver.bzl"],
+)
+
 bzl_library(
     name = "sentinel_bzl",
     srcs = ["sentinel.bzl"],
diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel
index 3b11dbe7f8..f6add1355e 100644
--- a/python/private/pypi/BUILD.bazel
+++ b/python/private/pypi/BUILD.bazel
@@ -59,6 +59,7 @@ bzl_library(
     srcs = ["extension.bzl"],
     deps = [
         ":attrs_bzl",
+        "//python/private:semver_bzl",
         ":hub_repository_bzl",
         ":parse_requirements_bzl",
         ":evaluate_markers_bzl",
diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl
index 1bc8f15149..77a477899e 100644
--- a/python/private/pypi/extension.bzl
+++ b/python/private/pypi/extension.bzl
@@ -19,6 +19,7 @@ load("@pythons_hub//:interpreters.bzl", "DEFAULT_PYTHON_VERSION", "INTERPRETER_L
 load("//python/private:auth.bzl", "AUTH_ATTRS")
 load("//python/private:normalize_name.bzl", "normalize_name")
 load("//python/private:repo_utils.bzl", "repo_utils")
+load("//python/private:semver.bzl", "semver")
 load("//python/private:version_label.bzl", "version_label")
 load(":attrs.bzl", "use_isolated")
 load(":evaluate_markers.bzl", "evaluate_markers", EVALUATE_MARKERS_SRCS = "SRCS")
@@ -32,22 +33,8 @@ load(":simpleapi_download.bzl", "simpleapi_download")
 load(":whl_library.bzl", "whl_library")
 load(":whl_repo_name.bzl", "whl_repo_name")
 
-def _parse_version(version):
-    major, _, version = version.partition(".")
-    minor, _, version = version.partition(".")
-    patch, _, version = version.partition(".")
-    build, _, version = version.partition(".")
-
-    return struct(
-        # use semver vocabulary here
-        major = major,
-        minor = minor,
-        patch = patch,  # this is called `micro` in the Python interpreter versioning scheme
-        build = build,
-    )
-
 def _major_minor_version(version):
-    version = _parse_version(version)
+    version = semver(version)
     return "{}.{}".format(version.major, version.minor)
 
 def _whl_mods_impl(mctx):
diff --git a/python/private/semver.bzl b/python/private/semver.bzl
new file mode 100644
index 0000000000..f1920bf001
--- /dev/null
+++ b/python/private/semver.bzl
@@ -0,0 +1,61 @@
+# Copyright 2024 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"A semver version parser"
+
+def _key(version):
+    return (
+        version.pre_release == "",
+        tuple([
+            (
+                i if not i.isdigit() else "",
+                # digit values take precedence
+                int(i) if i.isdigit() else 0,
+            )
+            for i in version.pre_release.split(".")
+        ]) if version.pre_release else None,
+        version.patch,
+        version.minor,
+        version.major,
+    )
+
+def semver(version):
+    """Parse the semver version and return the values as a struct.
+
+    Args:
+        version: {type}`str` the version string
+
+    Returns:
+        A {type}`struct` with `major`, `minor`, `patch` and `build` attributes.
+    """
+
+    # Implement the https://semver.org/ spec
+    major, _, version = version.partition(".")
+    minor, _, version = version.partition(".")
+    patch, _, build = version.partition("+")
+    patch, _, pre_release = patch.partition("-")
+
+    public = struct(
+        # use semver vocabulary here
+        major = int(major),
+        minor = int(minor or "0"),
+        # NOTE: this is called `micro` in the Python interpreter versioning scheme
+        patch = int(patch or "0"),
+        pre_release = pre_release,
+        build = build,
+        # buildifier: disable=uninitialized
+        key = lambda: _key(self.actual),
+    )
+    self = struct(actual = public)
+    return public
diff --git a/tests/semver/BUILD.bazel b/tests/semver/BUILD.bazel
new file mode 100644
index 0000000000..e12b1e5300
--- /dev/null
+++ b/tests/semver/BUILD.bazel
@@ -0,0 +1,17 @@
+# Copyright 2024 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load(":semver_test.bzl", "semver_test_suite")
+
+semver_test_suite(name = "semver_tests")
diff --git a/tests/semver/semver_test.bzl b/tests/semver/semver_test.bzl
new file mode 100644
index 0000000000..c85a008b74
--- /dev/null
+++ b/tests/semver/semver_test.bzl
@@ -0,0 +1,94 @@
+# Copyright 2023 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+""
+
+load("@rules_testing//lib:test_suite.bzl", "test_suite")
+load("//python/private:semver.bzl", "semver")  # buildifier: disable=bzl-visibility
+
+_tests = []
+
+def _test_semver_from_major(env):
+    actual = semver("3")
+    env.expect.that_int(actual.major).equals(3)
+    env.expect.that_int(actual.minor).equals(0)
+    env.expect.that_int(actual.patch).equals(0)
+    env.expect.that_str(actual.build).equals("")
+
+_tests.append(_test_semver_from_major)
+
+def _test_semver_from_major_minor_version(env):
+    actual = semver("4.9")
+    env.expect.that_int(actual.major).equals(4)
+    env.expect.that_int(actual.minor).equals(9)
+    env.expect.that_int(actual.patch).equals(0)
+    env.expect.that_str(actual.build).equals("")
+
+_tests.append(_test_semver_from_major_minor_version)
+
+def _test_semver_with_build_info(env):
+    actual = semver("1.2.3+mybuild")
+    env.expect.that_int(actual.major).equals(1)
+    env.expect.that_int(actual.minor).equals(2)
+    env.expect.that_int(actual.patch).equals(3)
+    env.expect.that_str(actual.build).equals("mybuild")
+
+_tests.append(_test_semver_with_build_info)
+
+def _test_semver_with_build_info_multiple_pluses(env):
+    actual = semver("1.2.3-rc0+build+info")
+    env.expect.that_int(actual.major).equals(1)
+    env.expect.that_int(actual.minor).equals(2)
+    env.expect.that_int(actual.patch).equals(3)
+    env.expect.that_str(actual.pre_release).equals("rc0")
+    env.expect.that_str(actual.build).equals("build+info")
+
+_tests.append(_test_semver_with_build_info_multiple_pluses)
+
+def _test_semver_alpha_beta(env):
+    actual = semver("1.2.3-alpha.beta")
+    env.expect.that_int(actual.major).equals(1)
+    env.expect.that_int(actual.minor).equals(2)
+    env.expect.that_int(actual.patch).equals(3)
+    env.expect.that_str(actual.pre_release).equals("alpha.beta")
+
+_tests.append(_test_semver_alpha_beta)
+
+def _test_semver_sort(env):
+    want = [
+        semver(item)
+        for item in [
+            "1.0.0-alpha",
+            "1.0.0-alpha.1",
+            "1.0.0-alpha.beta",
+            "1.0.0-beta",
+            "1.0.0-beta.2",
+            "1.0.0-beta.11",
+            "1.0.0-rc.1",
+            "1.0.0-rc.2",
+            "1.0.0",
+        ]
+    ]
+    actual = sorted(want, key = lambda x: x.key())
+    env.expect.that_collection(actual).contains_exactly(want).in_order()
+
+_tests.append(_test_semver_sort)
+
+def semver_test_suite(name):
+    """Create the test suite.
+
+    Args:
+        name: the name of the test suite
+    """
+    test_suite(name = name, basic_tests = _tests)

From 38dfc451a224fc8ed05f6737dbe9f81bbd3899d4 Mon Sep 17 00:00:00 2001
From: aignas <240938+aignas@users.noreply.github.com>
Date: Fri, 13 Sep 2024 16:01:34 +0900
Subject: [PATCH 2/4] add extra tests and fix a bug

---
 examples/bzlmod/MODULE.bazel.lock | 4 ++--
 python/private/semver.bzl         | 7 +++----
 tests/semver/semver_test.bzl      | 8 ++++++++
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock
index 05cf5305f6..4aa27dd2ca 100644
--- a/examples/bzlmod/MODULE.bazel.lock
+++ b/examples/bzlmod/MODULE.bazel.lock
@@ -1231,7 +1231,7 @@
     },
     "@@rules_python~//python/extensions:pip.bzl%pip": {
       "general": {
-        "bzlTransitiveDigest": "nhEjP1p9Faj4/hZ9PWLLgzxuDEZEPXXN/TixJkcqqD8=",
+        "bzlTransitiveDigest": "iAOFsTP/Oc89H/U6f40zNuHOhOiJX0ZdvF0Vf45hac8=",
         "usagesDigest": "MChlcSw99EuW3K7OOoMcXQIdcJnEh6YmfyjJm+9mxIg=",
         "recordedFileInputs": {
           "@@other_module~//requirements_lock_3_11.txt": "a7d0061366569043d5efcf80e34a32c732679367cb3c831c4cdc606adc36d314",
@@ -6140,7 +6140,7 @@
     },
     "@@rules_python~//python/private/pypi:pip.bzl%pip_internal": {
       "general": {
-        "bzlTransitiveDigest": "pmzJqpzKkz0ZOFUH8h0tza6o/nTvdsrYHHYT80PCs+U=",
+        "bzlTransitiveDigest": "k84XIQ/l9GbyU5F5AH/kbf34MhqLbUqB/VvthuaXU8k=",
         "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=",
         "recordedFileInputs": {
           "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a",
diff --git a/python/private/semver.bzl b/python/private/semver.bzl
index f1920bf001..564a4c36be 100644
--- a/python/private/semver.bzl
+++ b/python/private/semver.bzl
@@ -16,6 +16,9 @@
 
 def _key(version):
     return (
+        version.major,
+        version.minor,
+        version.patch,
         version.pre_release == "",
         tuple([
             (
@@ -25,9 +28,6 @@ def _key(version):
             )
             for i in version.pre_release.split(".")
         ]) if version.pre_release else None,
-        version.patch,
-        version.minor,
-        version.major,
     )
 
 def semver(version):
@@ -47,7 +47,6 @@ def semver(version):
     patch, _, pre_release = patch.partition("-")
 
     public = struct(
-        # use semver vocabulary here
         major = int(major),
         minor = int(minor or "0"),
         # NOTE: this is called `micro` in the Python interpreter versioning scheme
diff --git a/tests/semver/semver_test.bzl b/tests/semver/semver_test.bzl
index c85a008b74..d2a18b81fc 100644
--- a/tests/semver/semver_test.bzl
+++ b/tests/semver/semver_test.bzl
@@ -69,6 +69,12 @@ def _test_semver_sort(env):
     want = [
         semver(item)
         for item in [
+            # The items are sorted from lowest to highest version
+            "0.0.1",
+            "0.1.0-rc",
+            "0.1.0",
+            "0.9.11",
+            "0.9.12",
             "1.0.0-alpha",
             "1.0.0-alpha.1",
             "1.0.0-alpha.beta",
@@ -78,6 +84,8 @@ def _test_semver_sort(env):
             "1.0.0-rc.1",
             "1.0.0-rc.2",
             "1.0.0",
+            "2.0",
+            "3",
         ]
     ]
     actual = sorted(want, key = lambda x: x.key())

From 4e7cd9bf229ba52efb71bc95eef96bfa20e4b4f8 Mon Sep 17 00:00:00 2001
From: aignas <240938+aignas@users.noreply.github.com>
Date: Fri, 13 Sep 2024 16:23:09 +0900
Subject: [PATCH 3/4] compare the build metadata and add comments

---
 examples/bzlmod/MODULE.bazel.lock |  4 ++--
 python/private/semver.bzl         | 11 ++++++++---
 tests/semver/semver_test.bzl      | 11 +++++++++++
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock
index 4aa27dd2ca..84caa75f0d 100644
--- a/examples/bzlmod/MODULE.bazel.lock
+++ b/examples/bzlmod/MODULE.bazel.lock
@@ -1231,7 +1231,7 @@
     },
     "@@rules_python~//python/extensions:pip.bzl%pip": {
       "general": {
-        "bzlTransitiveDigest": "iAOFsTP/Oc89H/U6f40zNuHOhOiJX0ZdvF0Vf45hac8=",
+        "bzlTransitiveDigest": "mXcDFDeZIEVcpBAiu4mYx6kFUg98Eq91msIJCai7Upo=",
         "usagesDigest": "MChlcSw99EuW3K7OOoMcXQIdcJnEh6YmfyjJm+9mxIg=",
         "recordedFileInputs": {
           "@@other_module~//requirements_lock_3_11.txt": "a7d0061366569043d5efcf80e34a32c732679367cb3c831c4cdc606adc36d314",
@@ -6140,7 +6140,7 @@
     },
     "@@rules_python~//python/private/pypi:pip.bzl%pip_internal": {
       "general": {
-        "bzlTransitiveDigest": "k84XIQ/l9GbyU5F5AH/kbf34MhqLbUqB/VvthuaXU8k=",
+        "bzlTransitiveDigest": "J3UHKsngonB+HLry4KarnLxYc65Nf/6OZ7/UhS4akpQ=",
         "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=",
         "recordedFileInputs": {
           "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a",
diff --git a/python/private/semver.bzl b/python/private/semver.bzl
index 564a4c36be..9a240d46b7 100644
--- a/python/private/semver.bzl
+++ b/python/private/semver.bzl
@@ -19,7 +19,9 @@ def _key(version):
         version.major,
         version.minor,
         version.patch,
+        # non pre-release versions are higher
         version.pre_release == "",
+        # then we compare each element of the pre_release tag separately
         tuple([
             (
                 i if not i.isdigit() else "",
@@ -28,6 +30,8 @@ def _key(version):
             )
             for i in version.pre_release.split(".")
         ]) if version.pre_release else None,
+        # And build info is just alphabetic
+        version.build,
     )
 
 def semver(version):
@@ -41,9 +45,9 @@ def semver(version):
     """
 
     # Implement the https://semver.org/ spec
-    major, _, version = version.partition(".")
-    minor, _, version = version.partition(".")
-    patch, _, build = version.partition("+")
+    major, _, tail = version.partition(".")
+    minor, _, tail = tail.partition(".")
+    patch, _, build = tail.partition("+")
     patch, _, pre_release = patch.partition("-")
 
     public = struct(
@@ -55,6 +59,7 @@ def semver(version):
         build = build,
         # buildifier: disable=uninitialized
         key = lambda: _key(self.actual),
+        str = lambda: version,
     )
     self = struct(actual = public)
     return public
diff --git a/tests/semver/semver_test.bzl b/tests/semver/semver_test.bzl
index d2a18b81fc..6395639810 100644
--- a/tests/semver/semver_test.bzl
+++ b/tests/semver/semver_test.bzl
@@ -84,12 +84,23 @@ def _test_semver_sort(env):
             "1.0.0-rc.1",
             "1.0.0-rc.2",
             "1.0.0",
+            # Also handle missing minor and patch version strings
             "2.0",
             "3",
+            # Alphabetic comparison for different builds
+            "3.0.0+build0",
+            "3.0.0+build1",
         ]
     ]
     actual = sorted(want, key = lambda x: x.key())
     env.expect.that_collection(actual).contains_exactly(want).in_order()
+    for i, greater in enumerate(want[1:]):
+        smaller = actual[i]
+        if greater.key() <= smaller.key():
+            env.fail("Expected '{}' to be smaller than '{}', but got otherwise".format(
+                smaller.str(),
+                greater.str(),
+            ))
 
 _tests.append(_test_semver_sort)
 

From 7d4caeb330e3ef9d33b789d543ddfe87c0862309 Mon Sep 17 00:00:00 2001
From: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
Date: Sun, 15 Sep 2024 13:10:07 +0900
Subject: [PATCH 4/4] chore: pre-commit run update-bzlmod-lockfiles -a

---
 examples/bzlmod/MODULE.bazel.lock | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock
index af31a123a8..0cfe49d5d8 100644
--- a/examples/bzlmod/MODULE.bazel.lock
+++ b/examples/bzlmod/MODULE.bazel.lock
@@ -1231,7 +1231,7 @@
     },
     "@@rules_python~//python/extensions:pip.bzl%pip": {
       "general": {
-        "bzlTransitiveDigest": "QxV2PiqVV2B5LpnSrlzLgYyKNbUEXyVc1u+ahMrefws=",
+        "bzlTransitiveDigest": "7vRndkQ5a5Q2gcPIP8Jd/AkNRuB4n7SofpNFmFvodG8=",
         "usagesDigest": "MChlcSw99EuW3K7OOoMcXQIdcJnEh6YmfyjJm+9mxIg=",
         "recordedFileInputs": {
           "@@other_module~//requirements_lock_3_11.txt": "a7d0061366569043d5efcf80e34a32c732679367cb3c831c4cdc606adc36d314",
@@ -6140,7 +6140,7 @@
     },
     "@@rules_python~//python/private/pypi:pip.bzl%pip_internal": {
       "general": {
-        "bzlTransitiveDigest": "P0W31OsSgVVNQ3oRHHFiRWK7NLBLyI+KbQQBCPhou7w=",
+        "bzlTransitiveDigest": "DQe4hZM+myEcJ/pVW54jl5vWJOw+oZNBZfE0WOX/S9g=",
         "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=",
         "recordedFileInputs": {
           "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a",