From 9ba2cac8ffc676ed35d4c09c470d0159cbbd91ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Prpi=C4=8D?= Date: Wed, 4 Nov 2026 14:02:38 -0500 Subject: [PATCH 1/2] fix(pkgmetadata): accept LicenseRef anywhere in SPDX expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_parse_spdx()` skipped validation only when the whole expression started with `LicenseRef-`. A valid PEP 639 expression such as `MIT OR LicenseRef-Proprietary` was rejected, while `LicenseRef-Foo OR Apache 2` and malformed ids like `LicenseRef-a_b` were accepted. It now parses without validation and rejects every unknown key that is not a well-formed `LicenseRef-` id, raising the same `ExpressionError` as before. This is needed to populate SBOM license fields (#1378). Co-Authored-By: Claude Opus 5.5 Signed-off-by: Martin Prpič --- src/fromager/pkgmetadata/pep639.py | 25 ++++++++++++++++++++----- tests/test_pkgmetadata.py | 10 +++++++++- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/fromager/pkgmetadata/pep639.py b/src/fromager/pkgmetadata/pep639.py index d01192094..cf5b526ad 100644 --- a/src/fromager/pkgmetadata/pep639.py +++ b/src/fromager/pkgmetadata/pep639.py @@ -6,7 +6,9 @@ - https://spdx.org/licenses/ """ -from license_expression import LicenseExpression, get_spdx_licensing +import re + +from license_expression import ExpressionError, LicenseExpression, get_spdx_licensing from packaging.metadata import Metadata @@ -60,12 +62,25 @@ def license_from_metadata_values( _SPDX = get_spdx_licensing() +# SPDX idstring: letters, digits, "." and "-" +_LICENSE_REF_RE = re.compile(r"LicenseRef-[A-Za-z0-9.\-]+") + def _parse_spdx(text: str, *, simplify: bool = False) -> LicenseExpression: - """Parse, validate, and simplify a SPDX license expression""" - # LicenseRef are references to non-SPDX licenses - validate = not text.startswith("LicenseRef-") - expr = _SPDX.parse(text, validate=validate) + """Parse, validate, and simplify a SPDX license expression + + ``LicenseRef-`` keys reference non-SPDX licenses (PEP 639) and may + appear anywhere in the expression, so they are exempt from the SPDX + license list check. Every other unknown key is an error. + """ + expr = _SPDX.parse(text, validate=False) + unknown = [ + key + for key in _SPDX.unknown_license_keys(expr) + if not _LICENSE_REF_RE.fullmatch(key) + ] + if unknown: + raise ExpressionError(f"Unknown license key(s): {', '.join(unknown)}") if simplify: expr = expr.simplify() return expr diff --git a/tests/test_pkgmetadata.py b/tests/test_pkgmetadata.py index 1bb408500..62b8beadb 100644 --- a/tests/test_pkgmetadata.py +++ b/tests/test_pkgmetadata.py @@ -1,4 +1,5 @@ import pytest +from license_expression import ExpressionError from packaging.metadata import Metadata, RawMetadata from fromager.pkgmetadata import ( @@ -82,6 +83,9 @@ def test_project_urls_from_metadata( ["metadata", "expected"], [ [md("MIT AND MIT or MIT"), "MIT"], + # LicenseRef is valid anywhere in a PEP 639 expression + [md("MIT OR LicenseRef-Proprietary"), "LicenseRef-Proprietary OR MIT"], + [md("LicenseRef-Proprietary"), "LicenseRef-Proprietary"], [md(license="MIT License"), "MIT"], [md(license="MIT and PSF-2.0"), "MIT AND PSF-2.0"], [md(license="BSD 3-Clause License"), "BSD-3-Clause"], @@ -107,6 +111,10 @@ def test_license_from_metadata(metadata: Metadata, expected: str) -> None: [ (None, None, None), (None, "invalid", None), + # a leading LicenseRef must not disable validation of other keys + ("LicenseRef-Foo OR Apache 2", None, None), + # SPDX idstrings only allow letters, digits, "." and "-" + ("LicenseRef-a_b", None, None), (None, None, ["License :: OSI Approved :: BSD License"]), # classifiers with no entry that maps to SPDX (see issue #1372) (None, None, ["Programming Language :: Python :: 3"]), @@ -127,7 +135,7 @@ def test_license_from_metadata_errors( license: str | None, classifiers: list[str] | None, ) -> None: - with pytest.raises((ValueError, ExceptionGroup)): + with pytest.raises((ValueError, ExceptionGroup, ExpressionError)): license_from_metadata_values( license_expression=license_expression, license_text=license, From a7c27b280f97d79779bbaa66d0e001c8065b7e7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Prpi=C4=8D?= Date: Wed, 4 Nov 2026 15:07:24 -0500 Subject: [PATCH 2/2] fix(pkgmetadata): validate SPDX expressions with packaging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validate with `packaging.licenses.canonicalize_license_expression()` so `_parse_spdx()` follows PEP 639 exactly, and use license-expression only to build the expression object. `Apache-2.0 WITH LicenseRef-x` is now rejected and `licenseref-foo` accepted, matching `packaging`. The exception type stays `ExpressionError`; its message now comes from `packaging`. The invalid-expression cases move to their own test that checks which key was rejected, and new cases cover an exception next to a `LicenseRef` and an unknown `WITH` exception. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Martin Prpič --- src/fromager/pkgmetadata/pep639.py | 25 ++++++++------------- tests/test_pkgmetadata.py | 36 +++++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/fromager/pkgmetadata/pep639.py b/src/fromager/pkgmetadata/pep639.py index cf5b526ad..2817f4a11 100644 --- a/src/fromager/pkgmetadata/pep639.py +++ b/src/fromager/pkgmetadata/pep639.py @@ -6,9 +6,8 @@ - https://spdx.org/licenses/ """ -import re - from license_expression import ExpressionError, LicenseExpression, get_spdx_licensing +from packaging.licenses import InvalidLicenseExpression, canonicalize_license_expression from packaging.metadata import Metadata @@ -62,25 +61,19 @@ def license_from_metadata_values( _SPDX = get_spdx_licensing() -# SPDX idstring: letters, digits, "." and "-" -_LICENSE_REF_RE = re.compile(r"LicenseRef-[A-Za-z0-9.\-]+") - def _parse_spdx(text: str, *, simplify: bool = False) -> LicenseExpression: """Parse, validate, and simplify a SPDX license expression - ``LicenseRef-`` keys reference non-SPDX licenses (PEP 639) and may - appear anywhere in the expression, so they are exempt from the SPDX - license list check. Every other unknown key is an error. + `canonicalize_license_expression()` validates, so the rules match + PEP 639 exactly (e.g. ``LicenseRef-`` anywhere, but only SPDX + exceptions after ``WITH``); license-expression only builds the object. """ - expr = _SPDX.parse(text, validate=False) - unknown = [ - key - for key in _SPDX.unknown_license_keys(expr) - if not _LICENSE_REF_RE.fullmatch(key) - ] - if unknown: - raise ExpressionError(f"Unknown license key(s): {', '.join(unknown)}") + try: + canonical = canonicalize_license_expression(text) + except InvalidLicenseExpression as err: + raise ExpressionError(str(err)) from err + expr = _SPDX.parse(canonical, validate=False) if simplify: expr = expr.simplify() return expr diff --git a/tests/test_pkgmetadata.py b/tests/test_pkgmetadata.py index 62b8beadb..ffc9384c2 100644 --- a/tests/test_pkgmetadata.py +++ b/tests/test_pkgmetadata.py @@ -86,6 +86,11 @@ def test_project_urls_from_metadata( # LicenseRef is valid anywhere in a PEP 639 expression [md("MIT OR LicenseRef-Proprietary"), "LicenseRef-Proprietary OR MIT"], [md("LicenseRef-Proprietary"), "LicenseRef-Proprietary"], + # exception keys must not be reported as unknown next to a LicenseRef + [ + md("MIT WITH LLVM-exception OR LicenseRef-z"), + "LicenseRef-z OR MIT WITH LLVM-exception", + ], [md(license="MIT License"), "MIT"], [md(license="MIT and PSF-2.0"), "MIT AND PSF-2.0"], [md(license="BSD 3-Clause License"), "BSD-3-Clause"], @@ -111,10 +116,6 @@ def test_license_from_metadata(metadata: Metadata, expected: str) -> None: [ (None, None, None), (None, "invalid", None), - # a leading LicenseRef must not disable validation of other keys - ("LicenseRef-Foo OR Apache 2", None, None), - # SPDX idstrings only allow letters, digits, "." and "-" - ("LicenseRef-a_b", None, None), (None, None, ["License :: OSI Approved :: BSD License"]), # classifiers with no entry that maps to SPDX (see issue #1372) (None, None, ["Programming Language :: Python :: 3"]), @@ -135,9 +136,34 @@ def test_license_from_metadata_errors( license: str | None, classifiers: list[str] | None, ) -> None: - with pytest.raises((ValueError, ExceptionGroup, ExpressionError)): + with pytest.raises((ValueError, ExceptionGroup)): license_from_metadata_values( license_expression=license_expression, license_text=license, classifiers=classifiers, ) + + +@pytest.mark.parametrize( + ["license_expression", "match"], + [ + # a leading LicenseRef must not disable validation of other keys + ("LicenseRef-Foo OR Apache 2", "Apache 2"), + # SPDX idstrings only allow letters, digits, "." and "-" + ("LicenseRef-a_b", "(?i)LicenseRef-a_b"), + ("MIT WITH NotAnException", "(?i)NotAnException"), + # PEP 639 only allows SPDX exceptions after WITH + ("Apache-2.0 WITH LicenseRef-x", "(?i)LicenseRef-x"), + ], +) +def test_license_from_metadata_invalid_expression( + license_expression: str, match: str +) -> None: + with pytest.raises(ExpressionError, match=match): + license_from_metadata_values(license_expression=license_expression) + + +def test_license_from_metadata_canonicalizes_licenseref_prefix() -> None: + # PEP 639 matches the LicenseRef- prefix case-insensitively + expr = license_from_metadata_values(license_expression="licenseref-foo") + assert str(expr) == "LicenseRef-foo"