From 3914e3efd3c8a07d630fffd2437e3a3c7d6124d4 Mon Sep 17 00:00:00 2001 From: Rohan Devasthale Date: Mon, 29 Jun 2026 15:27:55 -0400 Subject: [PATCH] fix(candidate): use exact dist-info path in `get_metadata_for_wheel()` `get_metadata_for_wheel()` returned the first zip entry matching `*.dist-info/METADATA`, which could be a vendored package's metadata when a wheel bundles multiple dist-info directories. Derive the correct `{name}-{version}.dist-info/METADATA` path from the wheel URL instead and read it directly. Closes: #1146 Co-Authored-By: Claude Signed-off-by: Rohan Devasthale --- src/fromager/candidate.py | 24 ++++--- src/fromager/dependencies.py | 13 ++-- src/fromager/pkgmetadata/__init__.py | 4 ++ src/fromager/pkgmetadata/pep376.py | 30 ++++++++ src/fromager/wheels.py | 19 ++--- tests/test_pep658_support.py | 100 ++++++++++++++++++++++++++- 6 files changed, 158 insertions(+), 32 deletions(-) create mode 100644 src/fromager/pkgmetadata/pep376.py diff --git a/src/fromager/candidate.py b/src/fromager/candidate.py index 36d5955a5..46426169f 100644 --- a/src/fromager/candidate.py +++ b/src/fromager/candidate.py @@ -1,8 +1,10 @@ import dataclasses import datetime import logging +import posixpath import typing from io import BytesIO +from urllib.parse import urlparse from zipfile import ZipFile from packaging.metadata import Metadata @@ -10,6 +12,7 @@ from packaging.utils import BuildTag, canonicalize_name from packaging.version import Version +from .pkgmetadata.pep376 import dist_info_name from .request_session import session logger = logging.getLogger(__name__) @@ -141,12 +144,15 @@ def get_metadata_for_wheel( logger.debug(f"Downloading full wheel to extract metadata: {url}") data = session.get(url).content with ZipFile(BytesIO(data)) as z: - for n in z.namelist(): - if n.endswith(".dist-info/METADATA"): - metadata_content = z.read(n) - # Parse metadata directly using packaging.metadata.Metadata - # (avoiding circular import with dependencies module) - return Metadata.from_email(metadata_content, validate=validate) - - # If we didn't find the metadata, raise an error - raise ValueError(f"Could not find METADATA file in wheel: {url}") + metadata_path = _wheel_metadata_path(url) + try: + metadata_content = z.read(metadata_path) + except KeyError as err: + raise ValueError(f"Could not find {metadata_path} in wheel: {url}") from err + return Metadata.from_email(metadata_content, validate=validate) + + +def _wheel_metadata_path(url: str) -> str: + """Derive the METADATA path inside a wheel from its URL.""" + filename = posixpath.basename(urlparse(url).path) + return f"{dist_info_name(filename)}/METADATA" diff --git a/src/fromager/dependencies.py b/src/fromager/dependencies.py index e7964e4db..d80899e85 100644 --- a/src/fromager/dependencies.py +++ b/src/fromager/dependencies.py @@ -12,7 +12,7 @@ import tomlkit from packaging.metadata import Metadata from packaging.requirements import Requirement -from packaging.utils import NormalizedName, canonicalize_name, parse_wheel_filename +from packaging.utils import NormalizedName, canonicalize_name from packaging.version import Version from . import ( @@ -22,6 +22,7 @@ packagesettings, requirements_file, ) +from .pkgmetadata.pep376 import dist_info_name if typing.TYPE_CHECKING: from . import context @@ -475,19 +476,15 @@ def _get_metadata_from_wheel( Raises: ValueError: If no METADATA file is found in the wheel """ - # Get dist-info path from wheel filename. - # Uses same pattern as wheels.extract_info_from_wheel_file: - _, dist_version, _, _ = parse_wheel_filename(wheel_filename.name) - dist_name = wheel_filename.name.split("-", 1)[0] - metadata_path = f"{dist_name}-{dist_version}.dist-info/METADATA" + metadata_path = f"{dist_info_name(wheel_filename.name)}/METADATA" with zipfile.ZipFile(wheel_filename) as whl: try: metadata_content = whl.read(metadata_path) - except KeyError: + except KeyError as err: raise ValueError( f"Could not find METADATA file in wheel: {wheel_filename}" - ) from None + ) from err return parse_metadata(metadata_content, validate=validate) diff --git a/src/fromager/pkgmetadata/__init__.py b/src/fromager/pkgmetadata/__init__.py index 0b9707ab6..33ea5c89e 100644 --- a/src/fromager/pkgmetadata/__init__.py +++ b/src/fromager/pkgmetadata/__init__.py @@ -1,15 +1,19 @@ """Package metadata helpers +- PEP 376 dist-info directory handling - PEP 639-based license detection - PEP 753 project URL normalization """ +from .pep376 import dist_info_name, verbatim_dist_name from .pep639 import license_from_metadata, license_from_metadata_values from .pep753 import normalize_project_urls, project_urls_from_metadata __all__ = ( + "dist_info_name", "license_from_metadata", "license_from_metadata_values", "normalize_project_urls", "project_urls_from_metadata", + "verbatim_dist_name", ) diff --git a/src/fromager/pkgmetadata/pep376.py b/src/fromager/pkgmetadata/pep376.py new file mode 100644 index 000000000..c7e03862a --- /dev/null +++ b/src/fromager/pkgmetadata/pep376.py @@ -0,0 +1,30 @@ +"""Dist-info directory helpers + +- https://peps.python.org/pep-0376/ +""" + +from packaging.utils import parse_wheel_filename + + +def verbatim_dist_name(wheel_filename: str) -> str: + """Return the verbatim distribution name from a wheel filename. + + ``parse_wheel_filename`` normalises the name, but the dist-info + directory inside a wheel uses the original casing (e.g. + ``MarkupSafe``, not ``markupsafe``). This helper extracts the + first ``-``-separated segment after validating the filename. + """ + parse_wheel_filename(wheel_filename) + return wheel_filename.split("-", 1)[0] + + +def dist_info_name(wheel_filename: str) -> str: + """Return the dist-info directory name for a wheel filename. + + Preserves the verbatim distribution name (e.g. ``MarkupSafe``, not + ``markupsafe``) because the dist-info directory inside a wheel uses + the original casing. + """ + parse_wheel_filename(wheel_filename) + name, version = wheel_filename.split("-", 2)[:2] + return f"{name}-{version}.dist-info" diff --git a/src/fromager/wheels.py b/src/fromager/wheels.py index 0998a9dea..3f9f64a6a 100644 --- a/src/fromager/wheels.py +++ b/src/fromager/wheels.py @@ -17,7 +17,6 @@ from packaging.tags import Tag from packaging.utils import ( BuildTag, - canonicalize_name, parse_wheel_filename, ) from packaging.version import Version @@ -33,6 +32,7 @@ sbom, sources, ) +from .pkgmetadata.pep376 import verbatim_dist_name if typing.TYPE_CHECKING: from . import build_environment, context @@ -143,20 +143,11 @@ def extract_info_from_wheel_file( Returns the **verbatim** dist name (not normalized) because the dist-info directory inside the wheel uses the original casing - (e.g. ``MarkupSafe``, not ``markupsafe``). Uses ``parse_wheel_filename`` - for validation and to extract version, build tag, and platform tags. + (e.g. ``MarkupSafe``, not ``markupsafe``). """ - # parse_wheel_filename normalizes the dist name, however the dist-info - # directory uses the verbatim distribution name from the wheel file. - # Packages with upper case names like "MarkupSafe" are affected. - dist_name_normalized, dist_version, build_tag, wheel_tags = parse_wheel_filename( - wheel_file.name - ) - dist_name = wheel_file.name.split("-", 1)[0] - if dist_name_normalized != canonicalize_name(dist_name): - # sanity check, should never fail - raise ValueError(f"{dist_name_normalized} does not match {dist_name}") - return (dist_name, dist_version, build_tag, wheel_tags) + _, version, build_tag, wheel_tags = parse_wheel_filename(wheel_file.name) + dist_name = verbatim_dist_name(wheel_file.name) + return (dist_name, version, build_tag, wheel_tags) def default_add_extra_metadata_to_wheels( diff --git a/tests/test_pep658_support.py b/tests/test_pep658_support.py index 3af992ce6..1bb2310ee 100644 --- a/tests/test_pep658_support.py +++ b/tests/test_pep658_support.py @@ -1,11 +1,19 @@ """Tests for PEP 658 metadata support.""" import typing +from io import BytesIO from unittest.mock import Mock, patch +from zipfile import ZipFile +import pytest from packaging.version import Version -from fromager.candidate import Candidate, get_metadata_for_wheel +from fromager.candidate import ( + Candidate, + _wheel_metadata_path, + get_metadata_for_wheel, +) +from fromager.pkgmetadata.pep376 import dist_info_name class TestPEP658Support: @@ -191,3 +199,93 @@ def test_pep658_integration_with_resolver(self) -> None: assert candidate_with_metadata.name == candidate_without_metadata.name assert candidate_with_metadata.version == candidate_without_metadata.version assert candidate_with_metadata.url == candidate_without_metadata.url + + +class TestDistInfoName: + """Test dist_info_name — single source of truth for dist-info directory names.""" + + def test_standard_wheel(self) -> None: + assert ( + dist_info_name("test_package-1.0.0-py3-none-any.whl") + == "test_package-1.0.0.dist-info" + ) + + def test_preserves_verbatim_casing(self) -> None: + assert ( + dist_info_name("MarkupSafe-2.1.0-cp311-cp311-linux_x86_64.whl") + == "MarkupSafe-2.1.0.dist-info" + ) + + def test_non_wheel_extension_raises(self) -> None: + with pytest.raises(ValueError, match="Invalid wheel filename"): + dist_info_name("pkg-1.0.tar.gz") + + def test_malformed_wheel_filename_raises(self) -> None: + with pytest.raises(ValueError, match="Invalid wheel filename"): + dist_info_name("pkg-1.0-bad.whl") + + +class TestWheelMetadataPath: + """Test _wheel_metadata_path — URL to METADATA zip path.""" + + def test_simple_wheel_url(self) -> None: + url = "https://pkg.test/simple/test_package-1.0.0-py3-none-any.whl" + assert _wheel_metadata_path(url) == "test_package-1.0.0.dist-info/METADATA" + + def test_preserves_original_casing(self) -> None: + url = "https://pkg.test/simple/MarkupSafe-2.1.0-cp311-cp311-linux_x86_64.whl" + assert _wheel_metadata_path(url) == "MarkupSafe-2.1.0.dist-info/METADATA" + + def test_url_with_path_segments(self) -> None: + url = "https://pkg.test/packages/ab/cd/my_pkg-0.9-py3-none-any.whl" + assert _wheel_metadata_path(url) == "my_pkg-0.9.dist-info/METADATA" + + def test_url_with_fragment(self) -> None: + url = "https://pkg.test/simple/pkg-1.0-py3-none-any.whl#sha256=abc123" + assert _wheel_metadata_path(url) == "pkg-1.0.dist-info/METADATA" + + @patch("fromager.candidate.session") + def test_fallback_selects_correct_distinfo(self, mock_session: typing.Any) -> None: + """Wheels with vendored dist-info directories must not confuse extraction.""" + # Build a wheel zip with a vendored dist-info AND the real one + buf = BytesIO() + with ZipFile(buf, "w") as zf: + zf.writestr( + "vendored/six-1.16.0.dist-info/METADATA", + "Metadata-Version: 2.1\nName: six\nVersion: 1.16.0\n", + ) + zf.writestr( + "my_pkg-1.0.0.dist-info/METADATA", + ( + "Metadata-Version: 2.1\n" + "Name: my-pkg\n" + "Version: 1.0.0\n" + "Summary: The real package\n" + ), + ) + + mock_response = Mock() + mock_response.content = buf.getvalue() + mock_session.get.return_value = mock_response + + wheel_url = "https://pkg.test/simple/my_pkg-1.0.0-py3-none-any.whl" + metadata = get_metadata_for_wheel(wheel_url, metadata_url=None) + + assert metadata.name == "my-pkg" + assert str(metadata.version) == "1.0.0" + assert metadata.summary == "The real package" + + @patch("fromager.candidate.session") + def test_fallback_missing_metadata_raises(self, mock_session: typing.Any) -> None: + """A wheel without the expected dist-info/METADATA raises ValueError.""" + buf = BytesIO() + with ZipFile(buf, "w") as zf: + zf.writestr("some_other_file.txt", "hello") + + mock_response = Mock() + mock_response.content = buf.getvalue() + mock_session.get.return_value = mock_response + + wheel_url = "https://pkg.test/simple/my_pkg-1.0.0-py3-none-any.whl" + with pytest.raises(ValueError, match="Could not find"): + get_metadata_for_wheel(wheel_url, metadata_url=None)