fix: Use the fallback PKG-INFO file if the one generated manually is incomplete (#523)

lucashuy · web-flow · commit 5b7daf648fd2 · 2023-07-25T20:06:44.000Z
* Handle UNKNOWN and version 0.0.0 source packages

* Add missing requirements file

* Added missing requirements file for other tests that validate output

* Fix no binary downloads on build deps

* Update docstring to correctly assert the behaviour of the default check

* Make lowercase package name compare
diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py
@@ -7,6 +7,7 @@
 import subprocess
 import sys
 from email.parser import FeedParser
+from typing import Tuple
 
 from aws_lambda_builders.architecture import ARM64, X86_64
 from aws_lambda_builders.utils import extract_tarfile
@@ -603,12 +604,30 @@ def _get_pkg_info_filepath(self, package_dir):
             # This might be a pep 517 package in which case this PKG-INFO file
             # should be available right in the top level directory of the sdist
             # in the case where the egg_info command fails.
-            LOG.debug("Using fallback location for PKG-INFO file in package directory: %s", package_dir)
-            pkg_info_path = self._osutils.joinpath(package_dir, "PKG-INFO")
+            pkg_info_path = self._get_fallback_pkg_info_filepath(package_dir)
         if not self._osutils.file_exists(pkg_info_path):
             raise UnsupportedPackageError(self._osutils.basename(package_dir))
         return pkg_info_path
 
+    def _get_fallback_pkg_info_filepath(self, package_dir: str) -> str:
+        """
+        Gets the string path of the fallback PKG-INFO that is generated by pip
+
+        Parameters
+        ----------
+        package_dir: str
+            The path of the current working directory that has the package
+
+        Returns
+        -------
+        str
+            Path to a potential PKG-INFO
+        """
+        LOG.debug("Using fallback location for PKG-INFO file in package directory: %s", package_dir)
+        pkg_info_path = self._osutils.joinpath(package_dir, "PKG-INFO")
+
+        return pkg_info_path
+
     def _unpack_sdist_into_dir(self, sdist_path, unpack_dir):
         if sdist_path.endswith(".zip"):
             self._osutils.extract_zipfile(sdist_path, unpack_dir)
@@ -620,13 +639,79 @@ def _unpack_sdist_into_dir(self, sdist_path, unpack_dir):
         contents = self._osutils.get_directory_contents(unpack_dir)
         return self._osutils.joinpath(unpack_dir, contents[0])
 
-    def get_package_name_and_version(self, sdist_path):
+    def _get_name_version(self, pkg_info_filepath: str) -> Tuple[str, str]:
+        """
+        Extracts the name and the version from the PKG-INFO metadata file
+
+        Parameters
+        ----------
+        pkg_info_filepath: str
+            The path to the PKG-INFO file to get data from
+
+        Returns
+        -------
+        Tuple[str, str]
+            A tuple containing the name and version
+        """
+        metadata = self._parse_pkg_info_file(pkg_info_filepath)
+        return (metadata["Name"], metadata["Version"])
+
+    def _is_default_setuptools_values(self, name: str, version: str) -> bool:
+        """
+        Checks if the name or the version are the default values that are assigned by setuptools
+
+        Parameters
+        ----------
+        name: str
+            The name of the package
+        version: str
+            The version of the package as a string
+
+        Returns
+        -------
+        bool
+            True if either name or version are the default values
+        """
+        # default values logic:
+        # https://github.com/pypa/setuptools/blob/6083e18f4afc40316c0112134c205c336afbcdfd/setuptools/_distutils/dist.py#L1185-L1189
+        return name.lower() == "unknown" or version == "0.0.0"
+
+    def get_package_name_and_version(self, sdist_path: str) -> Tuple[str, str]:
+        """
+        Gets the package's name and version from the metadata file
+
+        Parameters
+        ----------
+        sdist_path: str
+            The string path of the downloaded source distribution artifact
+
+        Returns
+        -------
+        Tuple[str, str]
+            A tuple of the name and version of the package
+        """
         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)
-            metadata = self._parse_pkg_info_file(pkg_info_filepath)
-            name = metadata["Name"]
-            version = metadata["Version"]
+            name, version = self._get_name_version(pkg_info_filepath)
+
+            # return values if it is not the default values
+            if not self._is_default_setuptools_values(name, version):
+                return name, version
+
+            # see if we can get the fallback PKG_INFO file from the sdist
+            fallback_pkg_info_fp = self._get_fallback_pkg_info_filepath(package_dir)
+
+            if self._osutils.file_exists(fallback_pkg_info_fp):
+                # use the fallback values instead of the ones we generated
+                fallback_name, fallback_version = self._get_name_version(fallback_pkg_info_fp)
+
+                if not self._is_default_setuptools_values(fallback_name, fallback_version):
+                    name = fallback_name
+                    version = fallback_version
+
         return name, version
 
 
diff --git a/tests/functional/workflows/python_pip/test_packager.py b/tests/functional/workflows/python_pip/test_packager.py
@@ -997,6 +997,23 @@ def test_pkg_info_fallback_fails_raises_error(self, osutils, sdist_reader):
             with pytest.raises(UnsupportedPackageError):
                 sdist_reader.get_package_name_and_version(filepath)
 
+    def test_pkg_info_uses_fallback(self, osutils, sdist_reader):
+        # similar to test_cant_get_egg_info_filename
+        # but checks for UNKNOWN and/or 0.0.0 before
+        # using fallback
+        fallback_name = "mypkg"
+        fallback_version = "1.0.0"
+
+        setup_py = self._SETUP_PY % ("", "UNKNOWN", "0.0.0")
+        fallback_pkg_info = "Name: %s\nVersion: %s\n" % (fallback_name, fallback_version)
+
+        with osutils.tempdir() as tempdir:
+            filepath = self._write_fake_sdist(setup_py, tempdir, "tar.gz", fallback_pkg_info)
+            name, version = sdist_reader.get_package_name_and_version(filepath)
+
+            assert name == fallback_name
+            assert version == fallback_version
+
 
 class TestPackage(object):
     def test_same_pkg_sdist_and_wheel_collide(self, osutils, sdist_builder):
diff --git a/tests/integration/workflows/python_pip/test_python_pip.py b/tests/integration/workflows/python_pip/test_python_pip.py
@@ -39,13 +39,15 @@ def setUp(self):
         self.manifest_path_valid = os.path.join(self.TEST_DATA_FOLDER, "requirements-numpy.txt")
         self.manifest_path_invalid = os.path.join(self.TEST_DATA_FOLDER, "requirements-invalid.txt")
         self.manifest_path_sdist = os.path.join(self.TEST_DATA_FOLDER, "requirements-wrapt.txt")
+        self.manifest_path_inflate = os.path.join(self.TEST_DATA_FOLDER, "requirements-inflate.txt")
 
         self.test_data_files = {
             "__init__.py",
             "main.py",
             "requirements-invalid.txt",
             "requirements-numpy.txt",
             "requirements-wrapt.txt",
+            "requirements-inflate.txt",
             "local-dependencies",
         }
 
@@ -234,6 +236,20 @@ def test_must_resolve_local_dependency(self):
         for f in expected_files:
             self.assertIn(f, output_files)
 
+    def test_must_resolve_unknown_package_name(self):
+        self.builder.build(
+            self.source_dir,
+            self.artifacts_dir,
+            self.scratch_dir,
+            self.manifest_path_inflate,
+            runtime=self.runtime,
+            experimental_flags=self.experimental_flags,
+        )
+        expected_files = self.test_data_files.union(["inflate64", "inflate64.libs", "inflate64-0.1.4.dist-info"])
+        output_files = set(os.listdir(self.artifacts_dir))
+        for f in expected_files:
+            self.assertIn(f, output_files)
+
     def test_must_fail_to_resolve_dependencies(self):
         with self.assertRaises(WorkflowFailedError) as ctx:
             self.builder.build(
diff --git a/tests/integration/workflows/python_pip/testdata/requirements-inflate.txt b/tests/integration/workflows/python_pip/testdata/requirements-inflate.txt
@@ -0,0 +1 @@
+inflate64==0.1.4 --no-binary=:inflate64:
diff --git a/tests/unit/workflows/python_pip/test_packager.py b/tests/unit/workflows/python_pip/test_packager.py
@@ -1,14 +1,16 @@
 import sys
 from collections import namedtuple
 from unittest import TestCase, mock
+from unittest.mock import patch
 
 import pytest
+from parameterized import parameterized
 
 from aws_lambda_builders.architecture import ARM64, X86_64
 from aws_lambda_builders.workflows.python_pip.utils import OSUtils
 from aws_lambda_builders.workflows.python_pip.compat import pip_no_compile_c_env_vars
 from aws_lambda_builders.workflows.python_pip.compat import pip_no_compile_c_shim
-from aws_lambda_builders.workflows.python_pip.packager import DependencyBuilder
+from aws_lambda_builders.workflows.python_pip.packager import DependencyBuilder, SDistMetadataFetcher
 from aws_lambda_builders.workflows.python_pip.packager import PythonPipDependencyBuilder
 from aws_lambda_builders.workflows.python_pip.packager import Package
 from aws_lambda_builders.workflows.python_pip.packager import PipRunner
@@ -343,3 +345,115 @@ def test_check_pip_runner_string_pip(self):
 
         pip_runner_string = fake_osutils.popens[0][0][0][2].split(";")[-1:][0]
         self.assertIn("main", pip_runner_string)
+
+
+class TestSDistMetadataFetcher(TestCase):
+    @parameterized.expand(
+        [
+            (False,),
+            (True,),
+        ]
+    )
+    @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")
+    @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._get_fallback_pkg_info_filepath")
+    @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._is_default_setuptools_values")
+    @patch("aws_lambda_builders.workflows.python_pip.utils.OSUtils.file_exists")
+    @patch("aws_lambda_builders.workflows.python_pip.utils.OSUtils.tempdir")
+    def test_get_package_name_version_fails_fallback(
+        self,
+        fallback_file_exists,
+        tempdir_mock,
+        file_exists_mock,
+        is_default_values_mock,
+        get_fallback_mock,
+        get_name_ver_mock,
+        get_pkg_mock,
+        unpack_mock,
+    ):
+        """
+        Tests if both our generated PKG-INFO and PKG-INFO are missing/invalid
+        """
+        file_exists_mock.return_value = fallback_file_exists
+        is_default_values_mock.return_value = True
+
+        original_name = "UNKNOWN"
+        original_version = "5.5.0"
+
+        get_name_ver_mock.side_effect = [(original_name, original_version), ("UNKNOWN", "0.0.0")]
+
+        sdist = SDistMetadataFetcher(OSUtils)
+        name, version = sdist.get_package_name_and_version(mock.Mock())
+
+        self.assertEqual(name, original_name)
+        self.assertEqual(version, original_version)
+
+    @parameterized.expand(
+        [
+            (("UNKNOWN", "1.2.3"),),
+            (("unknown", "1.2.3"),),
+            (("foobar", "0.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")
+    @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._get_fallback_pkg_info_filepath")
+    @patch("aws_lambda_builders.workflows.python_pip.utils.OSUtils.file_exists")
+    @patch("aws_lambda_builders.workflows.python_pip.utils.OSUtils.tempdir")
+    def test_get_package_name_version_fallback(
+        self,
+        name_version,
+        tempdir_mock,
+        file_exists_mock,
+        get_fallback_mock,
+        get_name_ver_mock,
+        get_pkg_mock,
+        unpack_mock,
+    ):
+        """
+        Tests if we have UNKNOWN and if we use the fall back values
+        """
+        fallback_name = "fallback"
+        fallback_version = "1.0.0"
+
+        get_name_ver_mock.side_effect = [name_version, (fallback_name, fallback_version)]
+
+        sdist = SDistMetadataFetcher(OSUtils)
+        name, version = sdist.get_package_name_and_version(mock.Mock())
+
+        self.assertEqual(name, fallback_name)
+        self.assertEqual(version, fallback_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")
+    @patch("aws_lambda_builders.workflows.python_pip.packager.SDistMetadataFetcher._get_fallback_pkg_info_filepath")
+    @patch("aws_lambda_builders.workflows.python_pip.utils.OSUtils.file_exists")
+    @patch("aws_lambda_builders.workflows.python_pip.utils.OSUtils.tempdir")
+    def test_get_package_name_version(
+        self,
+        tempdir_mock,
+        file_exists_mock,
+        get_fallback_mock,
+        get_name_ver_mock,
+        get_pkg_mock,
+        unpack_mock,
+    ):
+        """
+        Tests return original results
+        """
+        not_default_name = "real"
+        not_default_version = "1.2.3"
+
+        fallback_name = "fallback"
+        fallback_version = "1.0.0"
+
+        get_name_ver_mock.side_effect = [(not_default_name, not_default_version), (fallback_name, fallback_version)]
+
+        sdist = SDistMetadataFetcher(OSUtils)
+        name, version = sdist.get_package_name_and_version(mock.Mock())
+
+        self.assertEqual(name, not_default_name)
+        self.assertEqual(version, not_default_version)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+inflate64==0.1.4 --no-binary=:inflate64:`