fix: normalize name in default_get_install_dependencies_of_sdist

The name field of sdist metadata is not guaranteed to be canonicalized
to the package's normalized name. Some packages have capitalization. The
sdist and wheel file name paths are not affected by the problem.
`parse_sdist_filename` and `parse_wheel_filename` return a
`NormalizedName`.

- `default_get_install_dependencies_of_sdist` now calls
  `canonicalize_name()` to pass a normalized name to validation
  function.
- `validate_dist_name_version` now requires a `NormalizedName` as
  `dist_name` parameter.
- `validate_dist_name_version` have more information in the exception
  (based on Doug's PR 782)
- more tests

Signed-off-by: Christian Heimes <cheimes@redhat.com>

Author: tiran

src/fromager/dependencies.py

@@ -12,7 +12,7 @@
 import tomlkit
 from packaging.metadata import Metadata
 from packaging.requirements import Requirement
-from packaging.utils import canonicalize_name
+from packaging.utils import NormalizedName, canonicalize_name
 from packaging.version import Version
 
 from . import (
@@ -318,7 +318,8 @@ def default_get_install_dependencies_of_sdist(
         req=req,
         version=version,
         what="sdist metadata",
-        dist_name=metadata.name,
+        # Metadata name is a non-normalized string
+        dist_name=canonicalize_name(metadata.name),
         dist_version=metadata.version,
     )
     if not metadata.requires_dist:
@@ -371,17 +372,30 @@ def pep517_metadata_of_sdist(
 
 
 def validate_dist_name_version(
-    req: Requirement, version: Version, what: str, dist_name: str, dist_version: Version
+    req: Requirement,
+    version: Version,
+    what: str,
+    dist_name: NormalizedName,
+    dist_version: Version,
 ) -> None:
     """Validate that dist name and version matches expected values"""
     req_name = canonicalize_name(req.name)
     if dist_name != req_name:
-        raise ValueError(f"{what} does not match requirement {req_name!r}")
-    if dist_version != version:
-        if dist_version.public != version.public:
-            raise ValueError(f"{what} does not match public version {version!r}")
+        if dist_name != canonicalize_name(dist_name):
+            # API misuse
+            raise RuntimeError("dist_name argument {dist_name!r} is not normalized")
         else:
-            logger.warning(f"{what} has different local version than {version!r}")
+            raise ValueError(
+                f"{what} {dist_name!r} does not match requirement {req_name!r}"
+            )
+    if dist_version.public != version.public:
+        raise ValueError(
+            f"{what} {dist_version.public!r} does not match public version {version.public!r}"
+        )
+    if dist_version.local != version.local:
+        logger.warning(
+            f"{what} {dist_version.local!r} has different local version than {version.local!r}"
+        )
 
 
 def get_install_dependencies_of_wheel(

tests/test_dependencies.py

@@ -2,11 +2,15 @@
 import itertools
 import pathlib
 import shutil
+import textwrap
 import typing
 from unittest.mock import Mock, patch
 
 import pytest
+from packaging.metadata import Metadata
 from packaging.requirements import Requirement
+from packaging.utils import NormalizedName
+from packaging.version import Version
 
 from fromager import build_environment, context, dependencies
 
@@ -206,3 +210,72 @@ def test_get_build_sdist_dependencies_cached(
         build_env=build_env,
     )
     assert results == set([Requirement("foo==1.0")])
+
+
+@patch("fromager.dependencies.pep517_metadata_of_sdist")
+def test_default_get_install_dependencies_of_sdist(
+    m_pep517_metadata_of_sdist: Mock,
+    tmp_context: context.WorkContext,
+    tmp_path: pathlib.Path,
+) -> None:
+    req = Requirement("huggingface-hub")
+    version = Version("1.2.3")
+    # sdist metadata name may not be normalized
+    metadata_txt = textwrap.dedent(
+        """\
+        Metadata-Version: 2.3
+        Name: HuggingFace_Hub
+        Version: 1.2.3
+        Requires-Dist: filelock
+        Requires-Dist: requests
+        """
+    )
+    metadata = Metadata.from_email(metadata_txt)
+    m_pep517_metadata_of_sdist.return_value = metadata
+
+    requirements = dependencies.default_get_install_dependencies_of_sdist(
+        ctx=tmp_context,
+        req=req,
+        version=version,
+        sdist_root_dir=tmp_path,
+        build_env=Mock(),
+        extra_environ={},
+        build_dir=tmp_path,
+        config_settings={},
+    )
+    assert requirements == {Requirement("filelock"), Requirement("requests")}
+
+
+@pytest.mark.parametrize(
+    "req_str,version_str,dist_name_str,dist_version_str,exc",
+    [
+        ("mypkg", "1.0", "mypkg", "1.0", None),
+        ("MyPKG", "1.0", "mypkg", "1.0", None),
+        ("mypkg", "1.0", "MyPKG", "1.0", RuntimeError),
+        ("mypkg", "1.0", "otherpkg", "1.0", ValueError),
+        ("mypkg", "1.0", "mypkg", "1.1", ValueError),
+        ("mypkg", "1.0+local", "mypkg", "1.0+local", None),
+        ("mypkg", "1.0", "mypkg", "1.0+local", None),
+        ("mypkg", "1.0+local", "mypkg", "1.0", None),
+    ],
+)
+def test_validate_dist_name_version(
+    req_str: str,
+    version_str: str,
+    dist_name_str: str,
+    dist_version_str: str,
+    exc: type[Exception] | None,
+) -> None:
+    validate = functools.partial(
+        dependencies.validate_dist_name_version,
+        req=Requirement(req_str),
+        version=Version(version_str),
+        what="test",
+        dist_name=typing.cast(NormalizedName, dist_name_str),
+        dist_version=Version(dist_version_str),
+    )
+    if exc is None:
+        validate()
+    else:
+        with pytest.raises(exc):
+            validate()