fix(pypi): fixes to the marker evaluation and utils (#2767)

These are just bugfixes to already merged code:
* Fix nested bracket parsing in PEP508 marker parser.
* Fix the sys_platform constants, which I noticed in #2629 but they got
  also pointed out in #2766.
* Port some of python tests for requirement parsing and improve the
  implementation. Those tests will be removed in #2629.
* Move the platform related code to a separate file.
* Rename `pep508_req.bzl` to `pep508_requirement.bzl` to follow the
  convention.


All of the bug fixes have added tests.

Work towards #2423.

Author: aignas

python/private/pypi/BUILD.bazel

@@ -77,7 +77,8 @@ bzl_library(
     deps = [
         ":pep508_env_bzl",
         ":pep508_evaluate_bzl",
-        ":pep508_req_bzl",
+        ":pep508_platform_bzl",
+        ":pep508_requirement_bzl",
     ],
 )
 
@@ -223,6 +224,9 @@ bzl_library(
 bzl_library(
     name = "pep508_env_bzl",
     srcs = ["pep508_env.bzl"],
+    deps = [
+        ":pep508_platform_bzl",
+    ],
 )
 
 bzl_library(
@@ -235,8 +239,13 @@ bzl_library(
 )
 
 bzl_library(
-    name = "pep508_req_bzl",
-    srcs = ["pep508_req.bzl"],
+    name = "pep508_platform_bzl",
+    srcs = ["pep508_platform.bzl"],
+)
+
+bzl_library(
+    name = "pep508_requirement_bzl",
+    srcs = ["pep508_requirement.bzl"],
     deps = [
         "//python/private:normalize_name_bzl",
     ],

python/private/pypi/evaluate_markers.bzl

@@ -14,9 +14,10 @@
 
 """A simple function that evaluates markers using a python interpreter."""
 
-load(":pep508_env.bzl", "env", _platform_from_str = "platform_from_str")
+load(":pep508_env.bzl", "env")
 load(":pep508_evaluate.bzl", "evaluate")
-load(":pep508_req.bzl", _req = "requirement")
+load(":pep508_platform.bzl", "platform_from_str")
+load(":pep508_requirement.bzl", "requirement")
 
 def evaluate_markers(requirements):
     """Return the list of supported platforms per requirements line.
@@ -29,9 +30,9 @@ def evaluate_markers(requirements):
     """
     ret = {}
     for req_string, platforms in requirements.items():
-        req = _req(req_string)
+        req = requirement(req_string)
         for platform in platforms:
-            if evaluate(req.marker, env = env(_platform_from_str(platform, None))):
+            if evaluate(req.marker, env = env(platform_from_str(platform, None))):
                 ret.setdefault(req_string, []).append(platform)
 
     return ret

python/private/pypi/pep508_env.bzl

@@ -15,7 +15,9 @@
 """This module is for implementing PEP508 environment definition.
 """
 
-# See https://stackoverflow.com/questions/45125516/possible-values-for-uname-m
+load(":pep508_platform.bzl", "platform_from_str")
+
+# See https://stackoverflow.com/a/45125525
 _platform_machine_aliases = {
     # These pairs mean the same hardware, but different values may be used
     # on different host platforms.
@@ -24,13 +26,41 @@ _platform_machine_aliases = {
     "i386": "x86_32",
     "i686": "x86_32",
 }
+
+# Platform system returns results from the `uname` call.
 _platform_system_values = {
     "linux": "Linux",
     "osx": "Darwin",
     "windows": "Windows",
 }
+
+# The copy of SO [answer](https://stackoverflow.com/a/13874620) containing
+# all of the platforms:
+# ┍━━━━━━━━━━━━━━━━━━━━━┯━━━━━━━━━━━━━━━━━━━━━┑
+# │ System              │ Value               │
+# ┝━━━━━━━━━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━━━━━━━┥
+# │ Linux               │ linux or linux2 (*) │
+# │ Windows             │ win32               │
+# │ Windows/Cygwin      │ cygwin              │
+# │ Windows/MSYS2       │ msys                │
+# │ Mac OS X            │ darwin              │
+# │ OS/2                │ os2                 │
+# │ OS/2 EMX            │ os2emx              │
+# │ RiscOS              │ riscos              │
+# │ AtheOS              │ atheos              │
+# │ FreeBSD 7           │ freebsd7            │
+# │ FreeBSD 8           │ freebsd8            │
+# │ FreeBSD N           │ freebsdN            │
+# │ OpenBSD 6           │ openbsd6            │
+# │ AIX                 │ aix (**)            │
+# ┕━━━━━━━━━━━━━━━━━━━━━┷━━━━━━━━━━━━━━━━━━━━━┙
+#
+# (*) Prior to Python 3.3, the value for any Linux version is always linux2; after, it is linux.
+# (**) Prior Python 3.8 could also be aix5 or aix7; use sys.platform.startswith()
+#
+# We are using only the subset that we actually support.
 _sys_platform_values = {
-    "linux": "posix",
+    "linux": "linux",
     "osx": "darwin",
     "windows": "win32",
 }
@@ -61,6 +91,7 @@ def env(target_platform, *, extra = None):
         "platform_release": "",
         "platform_version": "",
     }
+
     if type(target_platform) == type(""):
         target_platform = platform_from_str(target_platform, python_version = "")
 
@@ -87,31 +118,3 @@ def env(target_platform, *, extra = None):
             "platform_machine": _platform_machine_aliases,
         },
     }
-
-def _platform(*, abi = None, os = None, arch = None):
-    return struct(
-        abi = abi,
-        os = os,
-        arch = arch,
-    )
-
-def platform_from_str(p, python_version):
-    """Return a platform from a string.
-
-    Args:
-        p: {type}`str` the actual string.
-        python_version: {type}`str` the python version to add to platform if needed.
-
-    Returns:
-        A struct that is returned by the `_platform` function.
-    """
-    if p.startswith("cp"):
-        abi, _, p = p.partition("_")
-    elif python_version:
-        major, _, tail = python_version.partition(".")
-        abi = "cp{}{}".format(major, tail)
-    else:
-        abi = None
-
-    os, _, arch = p.partition("_")
-    return _platform(abi = abi, os = os or None, arch = arch or None)

python/private/pypi/pep508_platform.bzl

@@ -0,0 +1,57 @@
+# Copyright 2025 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.
+
+"""The platform abstraction
+"""
+
+def platform(*, abi = None, os = None, arch = None):
+    """platform returns a struct for the platform.
+
+    Args:
+        abi: {type}`str | None` the target ABI, e.g. `"cp39"`.
+        os: {type}`str | None` the target os, e.g. `"linux"`.
+        arch: {type}`str | None` the target CPU, e.g. `"aarch64"`.
+
+    Returns:
+        A struct.
+    """
+
+    # Note, this is used a lot as a key in dictionaries, so it cannot contain
+    # methods.
+    return struct(
+        abi = abi,
+        os = os,
+        arch = arch,
+    )
+
+def platform_from_str(p, python_version):
+    """Return a platform from a string.
+
+    Args:
+        p: {type}`str` the actual string.
+        python_version: {type}`str` the python version to add to platform if needed.
+
+    Returns:
+        A struct that is returned by the `_platform` function.
+    """
+    if p.startswith("cp"):
+        abi, _, p = p.partition("_")
+    elif python_version:
+        major, _, tail = python_version.partition(".")
+        abi = "cp{}{}".format(major, tail)
+    else:
+        abi = None
+
+    os, _, arch = p.partition("_")
+    return platform(abi = abi, os = os or None, arch = arch or None)

python/private/pypi/pep508_req.bzl renamed to python/private/pypi/pep508_requirement.bzl

@@ -17,7 +17,7 @@
 
 load("//python/private:normalize_name.bzl", "normalize_name")
 
-_STRIP = ["(", " ", ">", "=", "<", "~", "!"]
+_STRIP = ["(", " ", ">", "=", "<", "~", "!", "@"]
 
 def requirement(spec):
     """Parse a PEP508 requirement line
@@ -28,15 +28,18 @@ def requirement(spec):
     Returns:
         A struct with the information.
     """
+    spec = spec.strip()
     requires, _, maybe_hashes = spec.partition(";")
     marker, _, _ = maybe_hashes.partition("--hash")
     requires, _, extras_unparsed = requires.partition("[")
+    extras_unparsed, _, _ = extras_unparsed.partition("]")
     for char in _STRIP:
         requires, _, _ = requires.partition(char)
-    extras = extras_unparsed.strip("]").split(",")
+    extras = extras_unparsed.replace(" ", "").split(",")
+    name = requires.strip(" ")
 
     return struct(
-        name = normalize_name(requires.strip(" ")),
+        name = normalize_name(name).replace("_", "-"),
         marker = marker.strip(" "),
         extras = extras,
     )

tests/pypi/pep508/BUILD.bazel

@@ -1,5 +1,10 @@
 load(":evaluate_tests.bzl", "evaluate_test_suite")
+load(":requirement_tests.bzl", "requirement_test_suite")
 
 evaluate_test_suite(
     name = "evaluate_tests",
 )
+
+requirement_test_suite(
+    name = "requirement_tests",
+)

tests/pypi/pep508/requirement_tests.bzl

@@ -0,0 +1,47 @@
+# Copyright 2025 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.
+"""Tests for parsing the requirement specifier."""
+
+load("@rules_testing//lib:test_suite.bzl", "test_suite")
+load("//python/private/pypi:pep508_requirement.bzl", "requirement")  # buildifier: disable=bzl-visibility
+
+_tests = []
+
+def _test_requirement_line_parsing(env):
+    want = {
+        " name1[ foo ] ": ("name1", ["foo"]),
+        "Name[foo]": ("name", ["foo"]),
+        "name [fred,bar] @ http://foo.com ; python_version=='2.7'": ("name", ["fred", "bar"]),
+        "name; (os_name=='a' or os_name=='b') and os_name=='c'": ("name", [""]),
+        "name@http://foo.com": ("name", [""]),
+        "name[ Foo123 ]": ("name", ["Foo123"]),
+        "name[extra]@http://foo.com": ("name", ["extra"]),
+        "name[foo]": ("name", ["foo"]),
+        "name[quux, strange];python_version<'2.7' and platform_version=='2'": ("name", ["quux", "strange"]),
+        "name_foo[bar]": ("name-foo", ["bar"]),
+    }
+
+    got = {
+        i: (parsed.name, parsed.extras)
+        for i, parsed in {case: requirement(case) for case in want}.items()
+    }
+    env.expect.that_dict(got).contains_exactly(want)
+
+_tests.append(_test_requirement_line_parsing)
+
+def requirement_test_suite(name):  # buildifier: disable=function-docstring
+    test_suite(
+        name = name,
+        basic_tests = _tests,
+    )