move METADATA reading to starlark and add extra tests for the requirement spec parsing

Author: aignas

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,
     )

python/private/pypi/whl_installer/wheel_installer.py

@@ -120,14 +120,8 @@ def _extract_wheel(
     if not enable_implicit_namespace_pkgs:
         _setup_namespace_pkg_compatibility(installation_dir)
 
-    requires_dist = whl.metadata.get_all("Requires-Dist", [])
-    abi = f"cp{sys.version_info.major}{sys.version_info.minor}"
     metadata = {
-        "name": whl.name,
-        "version": whl.version,
         "python_version": sys.version.partition(" ")[0],
-        "requires_dist": requires_dist,
-        "abi": abi,
         "entry_points": [
             {
                 "name": name,
@@ -137,7 +131,6 @@ def _extract_wheel(
             for name, (module, attribute) in sorted(whl.entry_points().items())
         ],
     }
-    print(metadata)
 
     with open(os.path.join(installation_dir, "metadata.json"), "w") as f:
         json.dump(metadata, f)

python/private/pypi/whl_library.bzl

@@ -228,6 +228,61 @@ def _create_repository_execution_environment(rctx, python_interpreter, logger =
         env[_CPPFLAGS] = " ".join(cppflags)
     return env
 
+def _read_and_parse_required_metadata(rctx, logger):
+    contents = []
+    for entry in rctx.path("site-packages").readdir():
+        if not entry.basename.endswith(".dist-info"):
+            continue
+
+        if not entry.is_dir:
+            continue
+
+        metadata_file = entry.get_child("METADATA")
+
+        if not metadata_file.exists:
+            logger.fail("The METADATA file for the wheel could not be found")
+            return None
+
+        contents.extend(rctx.read(metadata_file).split("\n"))
+        break
+
+    single_value_fields = {
+        "License: ": "license",
+        "Name: ": "name",
+        "Version: ": "version",
+    }
+    requires_dist = "Requires-Dist: "
+    parsed = {}
+    for line in contents:
+        if not line or line.startswith("Dynamic"):
+            # Stop parsing on first empty line
+            break
+
+        found_prefix = None
+        for prefix in single_value_fields:
+            if line.startswith(prefix):
+                found_prefix = prefix
+                break
+
+        if found_prefix:
+            key = single_value_fields.pop(found_prefix)
+            _, _, value = line.partition(found_prefix)
+            parsed[key] = value.strip()
+            continue
+
+        if not line.startswith(requires_dist):
+            continue
+
+        _, _, value = line.partition(requires_dist)
+        parsed.setdefault("requires_dist", []).append(value.strip(" "))
+
+    return struct(
+        name = parsed["name"],
+        version = parsed["version"],
+        license = parsed.get("license"),
+        requires_dist = parsed.get("requires_dist", []),
+    )
+
 def _whl_library_impl(rctx):
     logger = repo_utils.logger(rctx)
     python_interpreter = pypi_repo_utils.resolve_python_interpreter(
@@ -400,7 +455,7 @@ def _whl_library_impl(rctx):
     #
     # This means that whl_library_targets will have to accept the following args:
     # * name - the name of the package in the METADATA.
-    # * requires_dist - the list of METADATA RequiresDist.
+    # * requires_dist - the list of METADATA Requires-Dist.
     # * platforms - the list of target platforms. The target_platforms
     #   should come from the hub repo via a 'load' statement so that they don't
     #   need to be passed as an argument to `whl_library`.
@@ -419,20 +474,19 @@ def _whl_library_impl(rctx):
     # * group_name, group_deps - this info can stay in the hub repository so that
     #   it is piped at the analysis time and changing the requirement groups does
     #   cause to re-fetch the deps.
+    whl_metadata = _read_and_parse_required_metadata(rctx, logger)
+    python_version = metadata["python_version"]
+
+    # TODO @aignas 2025-04-09: this will later be removed when loaded through the hub
+    major_minor, _, _ = python_version.rpartition(".")
     package_deps = deps(
-        # TODO @aignas 2025-04-04: get the following from manually parsing
-        # METADATA to avoid Python dependency:
-        # * name of the package
-        # * version of the package
-        # * RequiresDist
-        # * ProvidesExtras
-        name = metadata["name"],
-        requires_dist = metadata["requires_dist"],
+        name = whl_metadata.name,
+        requires_dist = whl_metadata.requires_dist,
         platforms = target_platforms or [
-            "{}_{}".format(metadata["abi"], host_platform(rctx)),
+            "cp{}_{}".format(major_minor.replace(".", ""), host_platform(rctx)),
         ],
         extras = requirement(rctx.attr.requirement).extras,
-        host_python_version = metadata["python_version"],
+        host_python_version = python_version,
     )
 
     build_file_contents = generate_whl_library_build_bazel(
@@ -444,8 +498,8 @@ def _whl_library_impl(rctx):
         group_deps = rctx.attr.group_deps,
         data_exclude = rctx.attr.pip_data_exclude,
         tags = [
-            "pypi_name=" + metadata["name"],
-            "pypi_version=" + metadata["version"],
+            "pypi_name=" + whl_metadata.name,
+            "pypi_version=" + whl_metadata.version,
         ],
         entry_points = entry_points,
         annotation = None if not rctx.attr.annotation else struct(**json.decode(rctx.read(rctx.attr.annotation))),

tests/pypi/pep508/BUILD.bazel

@@ -1,5 +1,6 @@
 load(":deps_tests.bzl", "deps_test_suite")
 load(":evaluate_tests.bzl", "evaluate_test_suite")
+load(":requirement_tests.bzl", "requirement_test_suite")
 
 evaluate_test_suite(
     name = "evaluate_tests",
@@ -8,3 +9,7 @@ evaluate_test_suite(
 deps_test_suite(
     name = "deps_tests",
 )
+
+requirement_test_suite(
+    name = "requirement_tests",
+)

tests/pypi/pep508/requirement_tests.bzl

@@ -0,0 +1,58 @@
+# 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_all(env):
+    cases = [
+        ("name[foo]", ("name", ["foo"])),
+        ("name[ Foo123 ]", ("name", ["Foo123"])),
+        (" name1[ foo ] ", ("name1", ["foo"])),
+        ("Name[foo]", ("name", ["foo"])),
+        ("name_foo[bar]", ("name-foo", ["bar"])),
+        (
+            "name [fred,bar] @ http://foo.com ; python_version=='2.7'",
+            ("name", ["fred", "bar"]),
+        ),
+        (
+            "name[quux, strange];python_version<'2.7' and platform_version=='2'",
+            ("name", ["quux", "strange"]),
+        ),
+        (
+            "name; (os_name=='a' or os_name=='b') and os_name=='c'",
+            ("name", None),
+        ),
+        (
+            "name@http://foo.com",
+            ("name", None),
+        ),
+    ]
+
+    for case, expected in cases:
+        got = requirement(case)
+        env.expect.that_str(got.name).equals(expected[0])
+        if expected[1] != None:
+            env.expect.that_collection(got.extras).contains_exactly(expected[1])
+
+_tests.append(_test_all)
+
+def requirement_test_suite(name):  # buildifier: disable=function-docstring
+    test_suite(
+        name = name,
+        basic_tests = _tests,
+    )

tests/pypi/whl_installer/wheel_installer_test.py

@@ -22,39 +22,6 @@
 from python.private.pypi.whl_installer import wheel_installer
 
 
-class TestRequirementExtrasParsing(unittest.TestCase):
-    def test_parses_requirement_for_extra(self) -> None:
-        cases = [
-            ("name[foo]", ("name", frozenset(["foo"]))),
-            ("name[ Foo123 ]", ("name", frozenset(["Foo123"]))),
-            (" name1[ foo ] ", ("name1", frozenset(["foo"]))),
-            ("Name[foo]", ("name", frozenset(["foo"]))),
-            ("name_foo[bar]", ("name-foo", frozenset(["bar"]))),
-            (
-                "name [fred,bar] @ http://foo.com ; python_version=='2.7'",
-                ("name", frozenset(["fred", "bar"])),
-            ),
-            (
-                "name[quux, strange];python_version<'2.7' and platform_version=='2'",
-                ("name", frozenset(["quux", "strange"])),
-            ),
-            (
-                "name; (os_name=='a' or os_name=='b') and os_name=='c'",
-                (None, None),
-            ),
-            (
-                "name@http://foo.com",
-                (None, None),
-            ),
-        ]
-
-        for case, expected in cases:
-            with self.subTest():
-                self.assertTupleEqual(
-                    wheel_installer._parse_requirement_for_extra(case), expected
-                )
-
-
 class TestWhlFilegroup(unittest.TestCase):
     def setUp(self) -> None:
         self.wheel_name = "example_minimal_package-0.0.1-py3-none-any.whl"
@@ -91,13 +58,8 @@ def test_wheel_exists(self) -> None:
             metadata_file_content = json.load(metadata_file)
 
         want = dict(
-            abi="cp311",
-            version="0.0.1",
-            name="example-minimal-package",
             entry_points=[],
-            extras=[],
             python_version="3.11.11",
-            requires_dist=[],
         )
         self.assertEqual(want, metadata_file_content)