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.
diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel
index 21e05f2..e0a2f20 100644
--- a/python/private/pypi/BUILD.bazel
+++ b/python/private/pypi/BUILD.bazel
@@ -77,7 +77,8 @@
deps = [
":pep508_env_bzl",
":pep508_evaluate_bzl",
- ":pep508_req_bzl",
+ ":pep508_platform_bzl",
+ ":pep508_requirement_bzl",
],
)
@@ -223,6 +224,9 @@
bzl_library(
name = "pep508_env_bzl",
srcs = ["pep508_env.bzl"],
+ deps = [
+ ":pep508_platform_bzl",
+ ],
)
bzl_library(
@@ -235,8 +239,13 @@
)
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",
],
diff --git a/python/private/pypi/evaluate_markers.bzl b/python/private/pypi/evaluate_markers.bzl
index 1d4c307..a0223ab 100644
--- a/python/private/pypi/evaluate_markers.bzl
+++ b/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 @@
"""
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
diff --git a/python/private/pypi/pep508_env.bzl b/python/private/pypi/pep508_env.bzl
index 17d4187..265a8e9 100644
--- a/python/private/pypi/pep508_env.bzl
+++ b/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 @@
"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 @@
"platform_release": "",
"platform_version": "",
}
+
if type(target_platform) == type(""):
target_platform = platform_from_str(target_platform, python_version = "")
@@ -87,31 +118,3 @@
"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)
diff --git a/python/private/pypi/pep508_platform.bzl b/python/private/pypi/pep508_platform.bzl
new file mode 100644
index 0000000..381a8d7
--- /dev/null
+++ b/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)
diff --git a/python/private/pypi/pep508_req.bzl b/python/private/pypi/pep508_requirement.bzl
similarity index 81%
rename from python/private/pypi/pep508_req.bzl
rename to python/private/pypi/pep508_requirement.bzl
index 618ffaf..11f2b3e 100644
--- a/python/private/pypi/pep508_req.bzl
+++ b/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 @@
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,
)
diff --git a/tests/pypi/pep508/BUILD.bazel b/tests/pypi/pep508/BUILD.bazel
index b795db0..575f28a 100644
--- a/tests/pypi/pep508/BUILD.bazel
+++ b/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",
+)
diff --git a/tests/pypi/pep508/requirement_tests.bzl b/tests/pypi/pep508/requirement_tests.bzl
new file mode 100644
index 0000000..7c81ea5
--- /dev/null
+++ b/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,
+ )
