Skip to content

Replaceing the metadata parser from packaging.metadata #607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Replaceing the metadata parser from packaging.metadata #607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bdada13
Using the metadata parser packaging.metadata
LalatenduMohanty Jul 4, 2025
0fc5526
Migrate candidate.py to use packaging.metadata.Metadata
LalatenduMohanty Jul 7, 2025
d948afc
Make dependencies.py metadata parsing consistent with other files
LalatenduMohanty Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/fromager/bootstrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import tempfile
import typing
import zipfile
from email.parser import BytesParser
from urllib.parse import urlparse

from packaging.metadata import Metadata, parse_email
from packaging.requirements import Requirement
from packaging.utils import NormalizedName, canonicalize_name
from packaging.version import Version
Expand Down Expand Up @@ -784,9 +784,9 @@ def _get_version_from_package_metadata(
)
metadata_filename = source_dir.parent / metadata_dir_base / "METADATA"
with open(metadata_filename, "rb") as f:
p = BytesParser()
metadata = p.parse(f, headersonly=True)
return Version(metadata["Version"])
raw_metadata, _ = parse_email(f.read())
metadata = Metadata.from_raw(raw_metadata)
Comment on lines +787 to +788
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using parse_email() + Metadata.from_raw() instead of Metadata.parse_email()? The Metadata.parse_email() combines parse_email(), Metadata.from_raw(), and additional validation.

This code should probably use fromager.dependencies.parse_metadata(metadata_filename).

return metadata.version

def _resolve_prebuilt_with_history(
self,
Expand Down
30 changes: 12 additions & 18 deletions src/fromager/candidate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
import typing
from email.message import EmailMessage, Message
from email.parser import BytesParser
from io import BytesIO
from typing import TYPE_CHECKING
from zipfile import ZipFile

from packaging.metadata import Metadata, parse_email
from packaging.requirements import Requirement
from packaging.utils import BuildTag, canonicalize_name
from packaging.version import Version

from .request_session import session

# fix for runtime errors caused by inheriting classes that are generic in stubs but not runtime
# https://mypy.readthedocs.io/en/latest/runtime_troubles.html#using-classes-that-are-generic-in-stubs-but-not-at-runtime
if TYPE_CHECKING:
Metadata = Message[str, str]
else:
Metadata = Message


class Candidate:
def __init__(
Expand Down Expand Up @@ -51,11 +42,10 @@ def metadata(self) -> Metadata:
return self._metadata

def _get_dependencies(self) -> typing.Iterable[Requirement]:
deps = self.metadata.get_all("Requires-Dist", [])
deps = self.metadata.requires_dist or []
extras = self.extras if self.extras else [""]

for d in deps:
r = Requirement(d)
for r in deps:
if r.marker is None:
yield r
else:
Expand All @@ -71,16 +61,20 @@ def dependencies(self) -> list[Requirement]:

@property
def requires_python(self) -> str | None:
return self.metadata.get("Requires-Python")
spec = self.metadata.requires_python
return str(spec) if spec is not None else None


def get_metadata_for_wheel(url: str) -> Metadata:
data = session.get(url).content
with ZipFile(BytesIO(data)) as z:
for n in z.namelist():
if n.endswith(".dist-info/METADATA"):
p = BytesParser()
return p.parse(z.open(n), headersonly=True)
metadata_content = z.read(n)
raw_metadata, _ = parse_email(metadata_content)
metadata = Metadata.from_raw(raw_metadata)
return metadata
Comment on lines +73 to +76
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this?

Suggested change
metadata_content = z.read(n)
raw_metadata, _ = parse_email(metadata_content)
metadata = Metadata.from_raw(raw_metadata)
return metadata
return Metadata.parse_email(z.read(n))


# If we didn't find the metadata, return an empty dict
return EmailMessage()
# If we didn't find the metadata, return an empty metadata object
raw_metadata, _ = parse_email(b"")
return Metadata.from_raw(raw_metadata)
Comment on lines +78 to +80
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work. A metadata object has three mandatory fields:

>>> raw_metadata, _ = parse_email(b"")
>>> Metadata.from_raw(raw_metadata)
  + Exception Group Traceback (most recent call last):
  |   File "<python-input-4>", line 1, in <module>
  |     Metadata.from_raw(raw_metadata)
  |     ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
  |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 752, in from_raw
  |     raise ExceptionGroup("invalid metadata", exceptions)
  | ExceptionGroup: invalid metadata (3 sub-exceptions)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 712, in from_raw
    |     metadata_version = ins.metadata_version
    |                        ^^^^^^^^^^^^^^^^^^^^
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 514, in __get__
    |     value = converter(value)
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 536, in _process_metadata_version
    |     raise self._invalid_metadata(f"{value!r} is not a valid metadata version")
    | packaging.metadata.InvalidMetadata: None is not a valid metadata version
    +---------------- 2 ----------------
    | Traceback (most recent call last):
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 747, in from_raw
    |     getattr(ins, key)
    |     ~~~~~~~^^^^^^^^^^
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 514, in __get__
    |     value = converter(value)
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 541, in _process_name
    |     raise self._invalid_metadata("{field} is a required field")
    | packaging.metadata.InvalidMetadata: 'name' is a required field
    +---------------- 3 ----------------
    | Traceback (most recent call last):
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 747, in from_raw
    |     getattr(ins, key)
    |     ~~~~~~~^^^^^^^^^^
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 514, in __get__
    |     value = converter(value)
    |   File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 554, in _process_version
    |     raise self._invalid_metadata("{field} is a required field")
    | packaging.metadata.InvalidMetadata: 'version' is a required field
    +------------------------------------

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate=False is not going to work either. It creates a broken Metadata object:

>>> raw_metadata, _ = parse_email(b"")
>>> m = Metadata.from_raw(raw_metadata, validate=False)
>>> m.name
Traceback (most recent call last):
  File "<python-input-4>", line 1, in <module>
    m.name
  File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 514, in __get__
    value = converter(value)
  File "/usr/lib/python3.13/site-packages/packaging/metadata.py", line 541, in _process_name
    raise self._invalid_metadata("{field} is a required field")
packaging.metadata.InvalidMetadata: 'name' is a required field

5 changes: 3 additions & 2 deletions src/fromager/dependencies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import pkginfo
import pyproject_hooks
import tomlkit
from packaging.metadata import Metadata
from packaging.metadata import Metadata, parse_email
from packaging.requirements import Requirement

from . import build_environment, external_commands, overrides, requirements_file
Expand Down Expand Up @@ -265,7 +265,8 @@ def parse_metadata(metadata_file: pathlib.Path, *, validate: bool = True) -> Met
and core metadata version, e.g. a package with metadata 2.2 and
license-expression field (added in 2.4).
"""
return Metadata.from_email(metadata_file.read_bytes(), validate=validate)
raw_metadata, _ = parse_email(metadata_file.read_bytes())
return Metadata.from_raw(raw_metadata, validate=validate)
Comment on lines -268 to +269
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change this? Metadata.from_email() is better, because it performs additional validations with validate=True.



def get_install_dependencies_of_wheel(
Expand Down
Loading