From a4980a1111a52f63d336f9a9545c900c607e52f8 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam Date: Wed, 9 Sep 2026 11:19:22 -0700 Subject: [PATCH 1/6] fix: use pyproject.toml metadata when sdist has no setup.py or PKG-INFO --- .../workflows/python_pip/packager.py | 94 ++++++++++++++++++- .../workflows/python_pip/test_packager.py | 90 +++++++++++++++++- 2 files changed, 178 insertions(+), 6 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 8bb8de0b4..2a6395551 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -7,7 +7,12 @@ import re import subprocess from email.parser import FeedParser -from typing import List, Tuple +from typing import List, Optional, Tuple + +try: + import tomllib +except ImportError: # Python 3.10 does not have tomllib in the standard library + tomllib = None from aws_lambda_builders.architecture import ARM64, X86_64 from aws_lambda_builders.utils import extract_tarfile @@ -31,6 +36,45 @@ class PackagerError(Exception): pass +def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optional[str]]: + """ + Reads the PEP 621 ``[project]`` name and version from pyproject.toml contents. + + Returns (None, None) when no usable static name/version can be determined + (missing table, dynamic version, or unparsable file). Uses stdlib + ``tomllib`` when available (Python 3.11+) and a minimal line-based parse + of the ``[project]`` section otherwise, so this keeps working on + Python 3.10. + """ + name, version = None, None + if tomllib is not None: + try: + project = tomllib.loads(contents).get("project") or {} + candidate_name, candidate_version = project.get("name"), project.get("version") + if isinstance(candidate_name, str) and isinstance(candidate_version, str): + return candidate_name, candidate_version + except Exception: + # Invalid TOML; fall through to the line-based parse below. + pass + in_project_section = False + for line in contents.splitlines(): + stripped = line.strip() + if stripped.startswith("["): + in_project_section = stripped == "[project]" + continue + if not in_project_section or stripped.startswith("#") or "=" not in stripped: + continue + key, _, value = stripped.partition("=") + value = value.strip().strip("\"'") + if key.strip() == "name" and not name: + name = value or None + elif key.strip() == "version" and not version: + version = value or None + if not name or not version: + return None, None + return name, version + + class InvalidSourceDistributionNameError(PackagerError): pass @@ -755,6 +799,40 @@ def _get_fallback_pkg_info_filepath(self, package_dir: str) -> str: return pkg_info_path + def _get_name_version_from_pyproject(self, package_dir: str) -> Tuple[str, str]: + """ + Extracts the name and version from the PEP 621 [project] table of a + pyproject.toml file. + + This is a last resort for sdists that carry no setup.py or PKG-INFO + metadata (e.g. PEP 517-only projects downloaded from git+https URLs), + where `setup.py egg_info` cannot produce any metadata. + + Parameters + ---------- + package_dir: str + The path of the unpacked sdist directory + + Returns + ------- + Tuple[str, str] + A tuple containing the name and version + + Raises + ------ + UnsupportedPackageError + If no usable static name/version can be read from pyproject.toml + """ + pyproject_path = self._osutils.joinpath(package_dir, "pyproject.toml") + if not self._osutils.file_exists(pyproject_path): + raise UnsupportedPackageError(self._osutils.basename(package_dir)) + contents = self._osutils.get_file_contents(pyproject_path, binary=False) + name, version = _parse_pyproject_name_version(contents) + if not name or not version: + raise UnsupportedPackageError(self._osutils.basename(package_dir)) + LOG.debug("Using name/version from pyproject.toml [project] table: %s==%s", name, version) + return name, version + def _unpack_sdist_into_dir(self, sdist_path, unpack_dir): if sdist_path.endswith(".zip"): self._osutils.extract_zipfile(sdist_path, unpack_dir) @@ -820,9 +898,17 @@ def get_package_name_and_version(self, sdist_path: str) -> Tuple[str, str]: with self._osutils.tempdir() as tempdir: package_dir = self._unpack_sdist_into_dir(sdist_path, tempdir) - # get the name and version from the result setup.py - pkg_info_filepath = self._get_pkg_info_filepath(package_dir) - name, version = self._get_name_version(pkg_info_filepath) + try: + # get the name and version from the result setup.py + pkg_info_filepath = self._get_pkg_info_filepath(package_dir) + name, version = self._get_name_version(pkg_info_filepath) + except UnsupportedPackageError: + # PEP 517-only sdists (e.g. downloaded from a git+https + # requirement) may carry no setup.py or PKG-INFO metadata for + # `setup.py egg_info` to read, which fails outright in Python + # 3.12+ build environments where setuptools is not installed. + # Fall back to the PEP 621 [project] metadata in pyproject.toml. + name, version = self._get_name_version_from_pyproject(package_dir) # return values if it is not the default values if not self._is_default_setuptools_values(name, version): diff --git a/tests/unit/workflows/python_pip/test_packager.py b/tests/unit/workflows/python_pip/test_packager.py index 877b3b9b5..9d4a10cdb 100644 --- a/tests/unit/workflows/python_pip/test_packager.py +++ b/tests/unit/workflows/python_pip/test_packager.py @@ -18,11 +18,12 @@ from aws_lambda_builders.workflows.python_pip.packager import get_lambda_abi from aws_lambda_builders.workflows.python_pip.packager import InvalidSourceDistributionNameError from aws_lambda_builders.workflows.python_pip.packager import NoSuchPackageError +from aws_lambda_builders.workflows.python_pip.packager import UnsupportedPackageError +from aws_lambda_builders.workflows.python_pip.packager import UnsupportedPythonVersion +from aws_lambda_builders.workflows.python_pip.packager import _parse_pyproject_name_version from aws_lambda_builders.workflows.python_pip.packager import PackageDownloadError from aws_lambda_builders.workflows.python_pip.packager import RequirementsFileNotFoundError from aws_lambda_builders.workflows.python_pip.packager import MissingDependencyError -from aws_lambda_builders.workflows.python_pip.packager import UnsupportedPackageError -from aws_lambda_builders.workflows.python_pip.packager import UnsupportedPythonVersion from aws_lambda_builders.workflows.python_pip import packager @@ -519,6 +520,91 @@ def test_get_package_name_version( self.assertEqual(name, not_default_name) self.assertEqual(version, not_default_version) + @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._unpack_sdist_into_dir") + @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._get_pkg_info_filepath") + @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._get_name_version_from_pyproject") + @patch("aws_lambda_builders.workflows.python_pip.utils.OSUtils.tempdir") + def test_get_package_name_version_pyproject_fallback( + self, tempdir_mock, get_pyproject_mock, get_pkg_mock, unpack_mock + ): + """ + Tests that a PEP 517-only sdist (no setup.py/PKG-INFO, e.g. downloaded + from a git+https requirement) falls back to pyproject.toml metadata + instead of raising UnsupportedPackageError (#675). + """ + get_pkg_mock.side_effect = UnsupportedPackageError("pkgdir") + get_pyproject_mock.return_value = ("pyproject-pkg", "2.0.0") + + sdist = SDistMetadataFetcher(OSUtils) + name, version = sdist.get_package_name_and_version(mock.Mock()) + + self.assertEqual(name, "pyproject-pkg") + self.assertEqual(version, "2.0.0") + + @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._unpack_sdist_into_dir") + @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._get_pkg_info_filepath") + @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._get_name_version_from_pyproject") + @patch("aws_lambda_builders.workflows.python_pip.utils.OSUtils.tempdir") + def test_get_package_name_version_pyproject_fallback_still_raises( + self, tempdir_mock, get_pyproject_mock, get_pkg_mock, unpack_mock + ): + """ + Tests that UnsupportedPackageError is still raised when neither + PKG-INFO nor pyproject.toml metadata is available. + """ + get_pkg_mock.side_effect = UnsupportedPackageError("pkgdir") + get_pyproject_mock.side_effect = UnsupportedPackageError("pkgdir") + + sdist = SDistMetadataFetcher(OSUtils) + with self.assertRaises(UnsupportedPackageError): + sdist.get_package_name_and_version(mock.Mock()) + + def test_get_name_version_from_pyproject(self): + import os + import tempfile + + with tempfile.TemporaryDirectory() as package_dir: + with open(os.path.join(package_dir, "pyproject.toml"), "w") as f: + f.write('[project]\nname = "my-pkg"\nversion = "1.2.3"\n') + + sdist = SDistMetadataFetcher(OSUtils) + self.assertEqual(sdist._get_name_version_from_pyproject(package_dir), ("my-pkg", "1.2.3")) + + def test_get_name_version_from_pyproject_missing_file(self): + sdist = SDistMetadataFetcher(OSUtils) + with self.assertRaises(UnsupportedPackageError): + sdist._get_name_version_from_pyproject("/nonexistent-dir") + + def test_get_name_version_from_pyproject_dynamic_version(self): + import os + import tempfile + + with tempfile.TemporaryDirectory() as package_dir: + with open(os.path.join(package_dir, "pyproject.toml"), "w") as f: + f.write('[project]\nname = "my-pkg"\ndynamic = ["version"]\n') + + sdist = SDistMetadataFetcher(OSUtils) + with self.assertRaises(UnsupportedPackageError): + sdist._get_name_version_from_pyproject(package_dir) + + @parameterized.expand( + [ + ('[project]\nname = "foo"\nversion = "1.2.3"\n', ("foo", "1.2.3")), + ("[build-system]\nrequires = []\n", (None, None)), + ('[project]\nname = "foo"\ndynamic = ["version"]\n', (None, None)), + ('[project]\nname = "foo"\nversion = "1.0"\n[tool.x]\nname = "nope"\nversion = "9"\n', ("foo", "1.0")), + ] + ) + def test_parse_pyproject_name_version(self, contents, expected): + self.assertEqual(_parse_pyproject_name_version(contents), expected) + + def test_parse_pyproject_name_version_without_tomllib(self): + # Python 3.10 has no stdlib tomllib; the line-based parse must work. + with patch("aws_lambda_builders.workflows.python_pip.packager.tomllib", None): + self.assertEqual( + _parse_pyproject_name_version('[project]\nname = "foo"\nversion = "1.2.3"\n'), ("foo", "1.2.3") + ) + class TestDependencyBuilder(object): def test_has_at_least_one_package_file_not_exists(self): From 15df6417a1db4c039e89122d4de3d6f190c29392 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam Date: Wed, 9 Sep 2026 11:46:27 -0700 Subject: [PATCH 2/6] Address review: tighten pyproject fallback parser - Line-based fallback (3.10-only): require quoted scalars via QUOTED_VALUE regex, tolerate trailing comments on the [project] header, strip inline comments outside quotes, skip non-scalar values like dynamic = ["version"] - Narrow except Exception to tomllib.TOMLDecodeError with LOG.debug; malformed TOML on 3.11+ now returns (None, None) instead of falling through to the hand-rolled parser - Add tests: trailing-comment version, commented header, non-scalar rejection, malformed-TOML-on-3.11+ behavior --- .../workflows/python_pip/packager.py | 33 +++++++++++------ .../workflows/python_pip/test_packager.py | 35 ++++++++++++++++++- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 2a6395551..8c0f4679a 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -23,6 +23,12 @@ LOG = logging.getLogger(__name__) +# Matches a quoted scalar value with an optional trailing comment, e.g. +# "1.2.3" -> group(2) == 1.2.3 +# 'foo' # comment -> group(2) == foo +QUOTED_VALUE = re.compile(r"""^(["'])(.*?)\1\s*(?:#.*)?$""") + + # TODO update the wording here MISSING_DEPENDENCIES_TEMPLATE = r""" Could not install dependencies: @@ -44,28 +50,35 @@ def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optiona (missing table, dynamic version, or unparsable file). Uses stdlib ``tomllib`` when available (Python 3.11+) and a minimal line-based parse of the ``[project]`` section otherwise, so this keeps working on - Python 3.10. + Python 3.10. The line-based parse only accepts properly quoted scalars and + tolerates trailing comments. """ - name, version = None, None if tomllib is not None: try: project = tomllib.loads(contents).get("project") or {} - candidate_name, candidate_version = project.get("name"), project.get("version") - if isinstance(candidate_name, str) and isinstance(candidate_version, str): - return candidate_name, candidate_version - except Exception: - # Invalid TOML; fall through to the line-based parse below. - pass + except tomllib.TOMLDecodeError as ex: + LOG.debug("Unable to parse pyproject.toml with tomllib: %s", ex) + return None, None + candidate_name, candidate_version = project.get("name"), project.get("version") + if isinstance(candidate_name, str) and isinstance(candidate_version, str): + return candidate_name, candidate_version + return None, None + name, version = None, None in_project_section = False for line in contents.splitlines(): stripped = line.strip() if stripped.startswith("["): - in_project_section = stripped == "[project]" + # tolerate trailing comments on the header, e.g. "[project] # main" + in_project_section = stripped.split("#")[0].strip() == "[project]" continue if not in_project_section or stripped.startswith("#") or "=" not in stripped: continue key, _, value = stripped.partition("=") - value = value.strip().strip("\"'") + match = QUOTED_VALUE.match(value.strip()) + if not match: + # Skip anything that is not a quoted scalar (e.g. dynamic = ["version"]) + continue + value = match.group(2) if key.strip() == "name" and not name: name = value or None elif key.strip() == "version" and not version: diff --git a/tests/unit/workflows/python_pip/test_packager.py b/tests/unit/workflows/python_pip/test_packager.py index 9d4a10cdb..9a1820a5a 100644 --- a/tests/unit/workflows/python_pip/test_packager.py +++ b/tests/unit/workflows/python_pip/test_packager.py @@ -593,17 +593,50 @@ def test_get_name_version_from_pyproject_dynamic_version(self): ("[build-system]\nrequires = []\n", (None, None)), ('[project]\nname = "foo"\ndynamic = ["version"]\n', (None, None)), ('[project]\nname = "foo"\nversion = "1.0"\n[tool.x]\nname = "nope"\nversion = "9"\n', ("foo", "1.0")), + # trailing comments (e.g. release-please version markers) must be stripped + ('[project]\nname = "foo"\nversion = "1.2.3" # x-release-please-version\n', ("foo", "1.2.3")), + ("[project] # main table\nname = 'foo'\nversion = '1.0'\n", ("foo", "1.0")), + # non-scalar values must not be picked up as name/version + ('[project]\nname = "foo"\ndynamic = ["version"]\n', (None, None)), + ('[project]\nname = ["foo"]\nversion = "1.0"\n', (None, None)), ] ) def test_parse_pyproject_name_version(self, contents, expected): self.assertEqual(_parse_pyproject_name_version(contents), expected) def test_parse_pyproject_name_version_without_tomllib(self): - # Python 3.10 has no stdlib tomllib; the line-based parse must work. + # Python 3.10 has no stdlib tomllib; the line-based parse must work, + # including inline comments that tools like release-please add. with patch("aws_lambda_builders.workflows.python_pip.packager.tomllib", None): self.assertEqual( _parse_pyproject_name_version('[project]\nname = "foo"\nversion = "1.2.3"\n'), ("foo", "1.2.3") ) + self.assertEqual( + _parse_pyproject_name_version( + '[project] # header comment\nname = "foo" # n\nversion = "1.2.3" # x-release-please-version\n' + ), + ("foo", "1.2.3"), + ) + self.assertEqual( + _parse_pyproject_name_version('[project]\nname = "foo"\ndynamic = ["version"]\n'), (None, None) + ) + # unparsable TOML is best-effort on the line-based path (it is the + # only parser available on 3.10); quoted scalars still extract. + self.assertEqual( + _parse_pyproject_name_version('[project]\nname = "foo"\nversion = "1.2.3"\nunclosed = ["x"\n'), + ("foo", "1.2.3"), + ) + + def test_parse_pyproject_name_version_malformed_toml_returns_none(self): + # On 3.11+, a real TOML parser is available: a malformed file must + # yield (None, None) rather than best-effort values from the fallback. + from aws_lambda_builders.workflows.python_pip.packager import tomllib as _tomllib + + if _tomllib is None: + self.skipTest("requires stdlib tomllib (Python 3.11+)") + self.assertEqual( + _parse_pyproject_name_version('[project]\nname = "foo"\nversion = "1.2.3"\nunclosed = ["x"\n'), (None, None) + ) class TestDependencyBuilder(object): From 17dfd524e8e4cc7f3d49b5004b32d595b055db49 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam Date: Wed, 9 Sep 2026 11:54:48 -0700 Subject: [PATCH 3/6] Address bot review: harden pyproject fallback (non-dict project, guarded read, demote misleading warning) --- .../workflows/python_pip/packager.py | 26 ++++++++- .../workflows/python_pip/test_packager.py | 57 +++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 8c0f4679a..d80eccd59 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -55,10 +55,16 @@ def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optiona """ if tomllib is not None: try: - project = tomllib.loads(contents).get("project") or {} + parsed = tomllib.loads(contents) except tomllib.TOMLDecodeError as ex: LOG.debug("Unable to parse pyproject.toml with tomllib: %s", ex) return None, None + project = parsed.get("project") + if not isinstance(project, dict): + # `project = "something"` is valid TOML but not a table; do not + # assume shape on third-party input. + LOG.debug("pyproject.toml [project] is not a table; cannot read name/version") + return None, None candidate_name, candidate_version = project.get("name"), project.get("version") if isinstance(candidate_name, str) and isinstance(candidate_version, str): return candidate_name, candidate_version @@ -784,7 +790,11 @@ def _get_pkg_info_filepath(self, package_dir): LOG.debug("Error while searching for existing .egg-info directories: %s", e) if not self._osutils.file_exists(pkg_info_path): - LOG.warning( + # This used to be a warning, but the caller may now recover via + # pyproject.toml metadata, in which case the build succeeds and + # a warning would be misleading. Keep it at debug; the caller + # logs an explicit message when recovery succeeds. + LOG.debug( "Unable to find PKG-INFO file for package in %s. " "This may be due to missing setuptools/distutils in Python 3.12+ " "or an incomplete sdist package.", @@ -839,7 +849,12 @@ def _get_name_version_from_pyproject(self, package_dir: str) -> Tuple[str, str]: pyproject_path = self._osutils.joinpath(package_dir, "pyproject.toml") if not self._osutils.file_exists(pyproject_path): raise UnsupportedPackageError(self._osutils.basename(package_dir)) - contents = self._osutils.get_file_contents(pyproject_path, binary=False) + try: + # utf-8-sig also tolerates a BOM so BOM-prefixed files still parse. + contents = self._osutils.get_file_contents(pyproject_path, binary=False, encoding="utf-8-sig") + except (OSError, UnicodeDecodeError) as ex: + LOG.debug("Unable to read %s: %s", pyproject_path, ex) + raise UnsupportedPackageError(self._osutils.basename(package_dir)) from ex name, version = _parse_pyproject_name_version(contents) if not name or not version: raise UnsupportedPackageError(self._osutils.basename(package_dir)) @@ -922,6 +937,11 @@ def get_package_name_and_version(self, sdist_path: str) -> Tuple[str, str]: # 3.12+ build environments where setuptools is not installed. # Fall back to the PEP 621 [project] metadata in pyproject.toml. name, version = self._get_name_version_from_pyproject(package_dir) + LOG.info( + "Recovered name/version for package in %s from pyproject.toml " + "[project] table; PKG-INFO metadata was unavailable.", + package_dir, + ) # return values if it is not the default values if not self._is_default_setuptools_values(name, version): diff --git a/tests/unit/workflows/python_pip/test_packager.py b/tests/unit/workflows/python_pip/test_packager.py index 9a1820a5a..d53be87d7 100644 --- a/tests/unit/workflows/python_pip/test_packager.py +++ b/tests/unit/workflows/python_pip/test_packager.py @@ -638,6 +638,63 @@ def test_parse_pyproject_name_version_malformed_toml_returns_none(self): _parse_pyproject_name_version('[project]\nname = "foo"\nversion = "1.2.3"\nunclosed = ["x"\n'), (None, None) ) + def test_parse_pyproject_name_version_non_dict_project(self): + # `project = "something"` is valid TOML; the parser must not assume + # [project] is a table (previously raised AttributeError once the + # broad except was narrowed). + from aws_lambda_builders.workflows.python_pip.packager import tomllib as _tomllib + + if _tomllib is None: + self.skipTest("requires stdlib tomllib (Python 3.11+)") + self.assertEqual(_parse_pyproject_name_version('project = "something"\n'), (None, None)) + self.assertEqual(_parse_pyproject_name_version("project = 42\n"), (None, None)) + + def test_get_name_version_from_pyproject_non_utf8(self): + # A pyproject.toml that is not valid UTF-8 must degrade to + # UnsupportedPackageError, not an unhandled UnicodeDecodeError. + import os + import tempfile + + with tempfile.TemporaryDirectory() as package_dir: + with open(os.path.join(package_dir, "pyproject.toml"), "wb") as f: + f.write(b'[project]\nname = "my-pkg"\nversion = "\xff\xfe"\n') + + sdist = SDistMetadataFetcher(sys.executable, OSUtils()) + with self.assertRaises(UnsupportedPackageError): + sdist._get_name_version_from_pyproject(package_dir) + + def test_get_name_version_from_pyproject_bom(self): + # A UTF-8 BOM must be tolerated (utf-8-sig) rather than failing the parse. + import os + import tempfile + + with tempfile.TemporaryDirectory() as package_dir: + with open(os.path.join(package_dir, "pyproject.toml"), "wb") as f: + f.write(b'\xef\xbb\xbf[project]\nname = "my-pkg"\nversion = "1.2.3"\n') + + sdist = SDistMetadataFetcher(sys.executable, OSUtils()) + self.assertEqual(sdist._get_name_version_from_pyproject(package_dir), ("my-pkg", "1.2.3")) + + def test_get_pkg_info_filepath_no_warning_when_missing(self): + # The PKG-INFO probe is a recoverable step (pyproject.toml fallback), + # so a missing PKG-INFO must not emit a misleading WARNING. + osutils = mock.Mock(spec=OSUtils) + osutils.joinpath.side_effect = lambda *a: "/".join(a) + osutils.basename.side_effect = lambda p: p.rsplit("/", 1)[-1] + osutils.file_exists.return_value = False + osutils.directory_exists.return_value = False + osutils.get_directory_contents.return_value = [] + + sdist = SDistMetadataFetcher(sys.executable, osutils) + with patch("aws_lambda_builders.workflows.python_pip.packager.subprocess") as subprocess_mock: + popen = subprocess_mock.Popen.return_value + popen.communicate.return_value = (b"", b"") + popen.returncode = 0 + subprocess_mock.run.return_value = mock.Mock(returncode=0) + with self.assertNoLogs("aws_lambda_builders.workflows.python_pip.packager", level="WARNING"): + with self.assertRaises(UnsupportedPackageError): + sdist._get_pkg_info_filepath("/pkgdir") + class TestDependencyBuilder(object): def test_has_at_least_one_package_file_not_exists(self): From 4532db06cdbde88338ec9497b13a8afe71dad260 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam Date: Wed, 9 Sep 2026 12:12:44 -0700 Subject: [PATCH 4/6] fix: warn on terminal metadata failure; keep recovery path quiet --- .../workflows/python_pip/packager.py | 21 +++++++++ .../workflows/python_pip/test_packager.py | 45 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index d80eccd59..93a7c08e0 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -848,19 +848,40 @@ def _get_name_version_from_pyproject(self, package_dir: str) -> Tuple[str, str]: """ pyproject_path = self._osutils.joinpath(package_dir, "pyproject.toml") if not self._osutils.file_exists(pyproject_path): + self._warn_unrecoverable_metadata(package_dir) raise UnsupportedPackageError(self._osutils.basename(package_dir)) try: # utf-8-sig also tolerates a BOM so BOM-prefixed files still parse. contents = self._osutils.get_file_contents(pyproject_path, binary=False, encoding="utf-8-sig") except (OSError, UnicodeDecodeError) as ex: LOG.debug("Unable to read %s: %s", pyproject_path, ex) + self._warn_unrecoverable_metadata(package_dir) raise UnsupportedPackageError(self._osutils.basename(package_dir)) from ex name, version = _parse_pyproject_name_version(contents) if not name or not version: + self._warn_unrecoverable_metadata(package_dir) raise UnsupportedPackageError(self._osutils.basename(package_dir)) LOG.debug("Using name/version from pyproject.toml [project] table: %s==%s", name, version) return name, version + @staticmethod + def _warn_unrecoverable_metadata(package_dir: str) -> None: + """ + Emits the user-visible diagnostic when no metadata source remains. + + The PKG-INFO probe now logs at debug because the pyproject.toml + fallback may recover; this warning is emitted only at the point where + recovery definitively fails so `sam build` output still names the + likely cause instead of just the opaque UnsupportedPackageError. + """ + LOG.warning( + "Unable to determine a static name/version for the package in %s. " + "No PKG-INFO metadata was available (this may be due to missing " + "setuptools/distutils in Python 3.12+) and pyproject.toml has no " + "static [project] name/version.", + package_dir, + ) + def _unpack_sdist_into_dir(self, sdist_path, unpack_dir): if sdist_path.endswith(".zip"): self._osutils.extract_zipfile(sdist_path, unpack_dir) diff --git a/tests/unit/workflows/python_pip/test_packager.py b/tests/unit/workflows/python_pip/test_packager.py index d53be87d7..ec1ce7fa8 100644 --- a/tests/unit/workflows/python_pip/test_packager.py +++ b/tests/unit/workflows/python_pip/test_packager.py @@ -675,6 +675,51 @@ def test_get_name_version_from_pyproject_bom(self): sdist = SDistMetadataFetcher(sys.executable, OSUtils()) self.assertEqual(sdist._get_name_version_from_pyproject(package_dir), ("my-pkg", "1.2.3")) + def test_get_name_version_from_pyproject_terminal_failure_logs_warning(self): + # Terminal failure (no pyproject.toml at all) must emit a + # user-visible WARNING naming the cause, not just the opaque + # UnsupportedPackageError. + sdist = SDistMetadataFetcher(OSUtils) + with self.assertLogs("aws_lambda_builders.workflows.python_pip.packager", level="WARNING") as logs: + with self.assertRaises(UnsupportedPackageError): + sdist._get_name_version_from_pyproject("/nonexistent-dir") + self.assertTrue( + any("Unable to determine a static name/version" in msg for msg in logs.output), + "expected the diagnostic warning on terminal failure", + ) + + def test_get_name_version_from_pyproject_no_metadata_logs_warning(self): + # pyproject.toml exists but has no static [project] name/version + # (e.g. dynamic = ["version"]) -> warning on the terminal failure. + import os + import tempfile + + with tempfile.TemporaryDirectory() as package_dir: + with open(os.path.join(package_dir, "pyproject.toml"), "w") as f: + f.write('[project]\nname = "my-pkg"\ndynamic = ["version"]\n') + + sdist = SDistMetadataFetcher(OSUtils) + with self.assertLogs("aws_lambda_builders.workflows.python_pip.packager", level="WARNING") as logs: + with self.assertRaises(UnsupportedPackageError): + sdist._get_name_version_from_pyproject(package_dir) + self.assertTrue( + any("Unable to determine a static name/version" in msg for msg in logs.output), + "expected the diagnostic warning on terminal failure", + ) + + def test_get_name_version_from_pyproject_success_no_warning(self): + # A successful recovery must stay clean: no WARNING on the success path. + import os + import tempfile + + with tempfile.TemporaryDirectory() as package_dir: + with open(os.path.join(package_dir, "pyproject.toml"), "w") as f: + f.write('[project]\nname = "my-pkg"\nversion = "1.2.3"\n') + + sdist = SDistMetadataFetcher(OSUtils) + with self.assertNoLogs("aws_lambda_builders.workflows.python_pip.packager", level="WARNING"): + self.assertEqual(sdist._get_name_version_from_pyproject(package_dir), ("my-pkg", "1.2.3")) + def test_get_pkg_info_filepath_no_warning_when_missing(self): # The PKG-INFO probe is a recoverable step (pyproject.toml fallback), # so a missing PKG-INFO must not emit a misleading WARNING. From 8b5ab48eb99b63407d9ac31453943fa689967ba6 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam Date: Wed, 9 Sep 2026 20:49:50 -0700 Subject: [PATCH 5/6] Normalize pyproject versions to PEP 440 canonical form; add packaging dependency --- .../workflows/python_pip/packager.py | 35 +++++++++++++++++-- requirements/python_pip.txt | 5 ++- .../workflows/python_pip/test_packager.py | 13 +++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 93a7c08e0..7126cb9a0 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -17,6 +17,8 @@ from aws_lambda_builders.architecture import ARM64, X86_64 from aws_lambda_builders.utils import extract_tarfile +from packaging.version import InvalidVersion, Version + from .compat import pip_import_string, pip_no_compile_c_env_vars, pip_no_compile_c_shim from .utils import OSUtils @@ -29,6 +31,24 @@ QUOTED_VALUE = re.compile(r"""^(["'])(.*?)\1\s*(?:#.*)?$""") +def _canonicalize_version(version): + """ + Return the PEP 440 canonical form of an author-written version string. + + Every other version producer in this module (PKG-INFO, wheel filenames) + supplies the canonical form, and ``Package`` identity comparison is an + exact string match -- so a non-canonical ``pyproject.toml`` version would + never reconcile with the wheel built from it. Returns None when the + version is not valid PEP 440, in which case the package is treated as + unrecoverable. + """ + try: + return str(Version(version)) + except InvalidVersion: + LOG.debug("pyproject.toml version %r is not a valid PEP 440 version", version) + return None + + # TODO update the wording here MISSING_DEPENDENCIES_TEMPLATE = r""" Could not install dependencies: @@ -47,7 +67,10 @@ def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optiona Reads the PEP 621 ``[project]`` name and version from pyproject.toml contents. Returns (None, None) when no usable static name/version can be determined - (missing table, dynamic version, or unparsable file). Uses stdlib + (missing table, dynamic version, unparsable file, or a version that is not + valid PEP 440). The returned version is normalized to its PEP 440 + canonical form so it matches the wheel filename produced by the build + backend. Uses stdlib ``tomllib`` when available (Python 3.11+) and a minimal line-based parse of the ``[project]`` section otherwise, so this keeps working on Python 3.10. The line-based parse only accepts properly quoted scalars and @@ -67,7 +90,10 @@ def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optiona return None, None candidate_name, candidate_version = project.get("name"), project.get("version") if isinstance(candidate_name, str) and isinstance(candidate_version, str): - return candidate_name, candidate_version + canonical_version = _canonicalize_version(candidate_version) + if canonical_version is None: + return None, None + return candidate_name, canonical_version return None, None name, version = None, None in_project_section = False @@ -91,7 +117,10 @@ def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optiona version = value or None if not name or not version: return None, None - return name, version + canonical_version = _canonicalize_version(version) + if canonical_version is None: + return None, None + return name, canonical_version class InvalidSourceDistributionNameError(PackagerError): diff --git a/requirements/python_pip.txt b/requirements/python_pip.txt index b0958467c..5b2e4af45 100644 --- a/requirements/python_pip.txt +++ b/requirements/python_pip.txt @@ -2,4 +2,7 @@ # Following packages are required by `python_pip` workflow to run. # TODO: Consider moving this dependency directly into the `python_pip` workflow module setuptools -wheel \ No newline at end of file +wheel +# Used to normalize PEP 621 versions to PEP 440 canonical form when reading +# name/version from pyproject.toml +packaging diff --git a/tests/unit/workflows/python_pip/test_packager.py b/tests/unit/workflows/python_pip/test_packager.py index ec1ce7fa8..66eb8c391 100644 --- a/tests/unit/workflows/python_pip/test_packager.py +++ b/tests/unit/workflows/python_pip/test_packager.py @@ -599,6 +599,13 @@ def test_get_name_version_from_pyproject_dynamic_version(self): # non-scalar values must not be picked up as name/version ('[project]\nname = "foo"\ndynamic = ["version"]\n', (None, None)), ('[project]\nname = ["foo"]\nversion = "1.0"\n', (None, None)), + # the version is normalized to PEP 440 canonical form so it matches + # the wheel filename produced by the build backend + ('[project]\nname = "foo"\nversion = "2024.01.15"\n', ("foo", "2024.1.15")), + ('[project]\nname = "foo"\nversion = "1.0.0-rc1"\n', ("foo", "1.0.0rc1")), + ('[project]\nname = "foo"\nversion = "v1.2.3"\n', ("foo", "1.2.3")), + # a version that is not valid PEP 440 is unrecoverable + ('[project]\nname = "foo"\nversion = "not a version!!"\n', (None, None)), ] ) def test_parse_pyproject_name_version(self, contents, expected): @@ -627,6 +634,12 @@ def test_parse_pyproject_name_version_without_tomllib(self): ("foo", "1.2.3"), ) + # the line-based path also normalizes to PEP 440 canonical form + self.assertEqual( + _parse_pyproject_name_version('[project]\nname = "foo"\nversion = "2024.01.15"\n'), + ("foo", "2024.1.15"), + ) + def test_parse_pyproject_name_version_malformed_toml_returns_none(self): # On 3.11+, a real TOML parser is available: a malformed file must # yield (None, None) rather than best-effort values from the fallback. From dc953d596f8f8ba5dbb5174d2efdb91ac5ea754a Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam Date: Wed, 9 Sep 2026 21:36:11 -0700 Subject: [PATCH 6/6] Address review: lazy packaging import, ruff-clean pyproject fallback --- .../workflows/python_pip/packager.py | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 7126cb9a0..abfe64dd8 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -17,8 +17,6 @@ from aws_lambda_builders.architecture import ARM64, X86_64 from aws_lambda_builders.utils import extract_tarfile -from packaging.version import InvalidVersion, Version - from .compat import pip_import_string, pip_no_compile_c_env_vars, pip_no_compile_c_shim from .utils import OSUtils @@ -41,7 +39,17 @@ def _canonicalize_version(version): never reconcile with the wheel built from it. Returns None when the version is not valid PEP 440, in which case the package is treated as unrecoverable. + + The ``packaging`` import is local and degradable: this is a rare fallback + path, and importing it at module scope would make it a hard import-time + requirement for the entire python_pip workflow (PLC0415 is already in the + ruff ignore list, so a function-local import is idiomatic here). """ + try: + from packaging.version import InvalidVersion, Version + except ImportError: + LOG.debug("packaging is unavailable; using the pyproject.toml version as written") + return version try: return str(Version(version)) except InvalidVersion: @@ -62,6 +70,21 @@ class PackagerError(Exception): pass +def _finalize_name_version(name, version) -> Tuple[Optional[str], Optional[str]]: + """ + Validate a parsed (name, version) pair and normalize the version to its + PEP 440 canonical form. Returns (None, None) when the pair is unusable + (missing values, non-string values, or a version that is not valid + PEP 440). + """ + if not isinstance(name, str) or not isinstance(version, str): + return None, None + canonical_version = _canonicalize_version(version) + if canonical_version is None: + return None, None + return name, canonical_version + + def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optional[str]]: """ Reads the PEP 621 ``[project]`` name and version from pyproject.toml contents. @@ -88,13 +111,7 @@ def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optiona # assume shape on third-party input. LOG.debug("pyproject.toml [project] is not a table; cannot read name/version") return None, None - candidate_name, candidate_version = project.get("name"), project.get("version") - if isinstance(candidate_name, str) and isinstance(candidate_version, str): - canonical_version = _canonicalize_version(candidate_version) - if canonical_version is None: - return None, None - return candidate_name, canonical_version - return None, None + return _finalize_name_version(project.get("name"), project.get("version")) name, version = None, None in_project_section = False for line in contents.splitlines(): @@ -115,12 +132,7 @@ def _parse_pyproject_name_version(contents: str) -> Tuple[Optional[str], Optiona name = value or None elif key.strip() == "version" and not version: version = value or None - if not name or not version: - return None, None - canonical_version = _canonicalize_version(version) - if canonical_version is None: - return None, None - return name, canonical_version + return _finalize_name_version(name, version) class InvalidSourceDistributionNameError(PackagerError):