From 196af552b66d864a56240c92f12e5452f2676b7b Mon Sep 17 00:00:00 2001 From: Justice Sidhu Date: Wed, 22 Nov 2023 12:10:50 -0800 Subject: [PATCH 1/4] add missing arg --- .../workflows/python_pip/packager.py | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 252e45f7a..4d3aeb488 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -97,7 +97,7 @@ def get_lambda_abi(runtime): class PythonPipDependencyBuilder(object): - def __init__(self, runtime, osutils=None, dependency_builder=None, architecture=X86_64): + def __init__(self, runtime, python_exe, osutils=None, dependency_builder=None, architecture=X86_64): """Initialize a PythonPipDependencyBuilder. :type runtime: str @@ -122,7 +122,7 @@ def __init__(self, runtime, osutils=None, dependency_builder=None, architecture= self.osutils = OSUtils() if dependency_builder is None: - dependency_builder = DependencyBuilder(self.osutils, runtime, architecture=architecture) + dependency_builder = DependencyBuilder(self.osutils, python_exe, runtime, architecture=architecture) self._dependency_builder = dependency_builder def build_dependencies(self, artifacts_dir_path, scratch_dir_path, requirements_path, ui=None, config=None): @@ -211,7 +211,7 @@ class DependencyBuilder(object): # Unlikely to hit this case. _DEFAULT_GLIBC = (2, 17) - def __init__(self, osutils, runtime, pip_runner=None, architecture=X86_64): + def __init__(self, osutils, runtime, python_exe, pip_runner=None, architecture=X86_64): """Initialize a DependencyBuilder. :type osutils: :class:`lambda_builders.utils.OSUtils` @@ -229,8 +229,9 @@ def __init__(self, osutils, runtime, pip_runner=None, architecture=X86_64): :param architecture: Architecture to build for. """ self._osutils = osutils + self.python_exe = python_exe if pip_runner is None: - pip_runner = PipRunner(python_exe=None, pip=SubprocessPip(osutils)) + pip_runner = PipRunner(python_exe=python_exe, pip=SubprocessPip(osutils)) self._pip = pip_runner self.runtime = runtime self.architecture = architecture @@ -364,7 +365,7 @@ def _download_all_dependencies(self, requirements_filename, directory): # which will serve as the primary list of dependencies needed to deploy # successfully. self._pip.download_all_dependencies(requirements_filename, directory) - deps = {Package(directory, filename) for filename in self._osutils.get_directory_contents(directory)} + deps = {Package(directory, filename, self.python_exe) for filename in self._osutils.get_directory_contents(directory)} LOG.debug("Full dependency closure: %s", deps) return deps @@ -383,7 +384,7 @@ def _build_sdists(self, sdists, directory, compile_c=True): def _categorize_wheel_files(self, directory): final_wheels = [ - Package(directory, filename) + Package(directory, filename, self.python_exe) for filename in self._osutils.get_directory_contents(directory) if filename.endswith(".whl") ] @@ -506,13 +507,14 @@ def _install_wheels(self, src_dir, dst_dir, wheels): class Package(object): """A class to represent a package downloaded but not yet installed.""" - def __init__(self, directory, filename, osutils=None): + def __init__(self, directory, filename, python_exe, osutils=None): self.dist_type = "wheel" if filename.endswith(".whl") else "sdist" self._directory = directory self.filename = filename if osutils is None: osutils = OSUtils() self._osutils = osutils + self.python_exe = python_exe self._name, self._version = self._calculate_name_and_version() @property @@ -553,7 +555,7 @@ def _calculate_name_and_version(self): # {platform tag}.whl name, version = self.filename.split("-")[:2] else: - info_fetcher = SDistMetadataFetcher(osutils=self._osutils) + info_fetcher = SDistMetadataFetcher(self.python_exe, osutils=self._osutils) sdist_path = self._osutils.joinpath(self._directory, self.filename) name, version = info_fetcher.get_package_name_and_version(sdist_path) normalized_name = self._normalize_name(name) @@ -572,10 +574,11 @@ class SDistMetadataFetcher(object): "exec(compile(code, __file__, 'exec'))" ) - def __init__(self, osutils=None): + def __init__(self, python_exe, osutils=None): if osutils is None: osutils = OSUtils() self._osutils = osutils + self.python_exe = python_exe def _parse_pkg_info_file(self, filepath): # The PKG-INFO generated by the egg-info command is in an email feed @@ -590,7 +593,7 @@ def _get_pkg_info_filepath(self, package_dir): setup_py = self._osutils.joinpath(package_dir, "setup.py") script = self._SETUPTOOLS_SHIM % setup_py - cmd = [sys.executable, "-c", script, "--no-user-cfg", "egg_info", "--egg-base", "egg-info"] + cmd = [self.python_exe, "-c", script, "--no-user-cfg", "egg_info", "--egg-base", "egg-info"] egg_info_dir = self._osutils.joinpath(package_dir, "egg-info") self._osutils.makedirs(egg_info_dir) p = subprocess.Popen( From 96fc3c63da416161531ce2a57985afcd687c9ebf Mon Sep 17 00:00:00 2001 From: Justice Sidhu Date: Wed, 22 Nov 2023 16:34:14 -0800 Subject: [PATCH 2/4] use python_exe instead of sys.executable --- aws_lambda_builders/workflows/python_pip/actions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/actions.py b/aws_lambda_builders/workflows/python_pip/actions.py index d325559de..966cd1737 100644 --- a/aws_lambda_builders/workflows/python_pip/actions.py +++ b/aws_lambda_builders/workflows/python_pip/actions.py @@ -49,11 +49,11 @@ def execute(self) -> None: pip_runner = PipRunner(python_exe=python_with_pip, pip=pip) dependency_builder = DependencyBuilder( - osutils=self._os_utils, pip_runner=pip_runner, runtime=self.runtime, architecture=self.architecture + osutils=self._os_utils, python_exe=python_with_pip, pip_runner=pip_runner, runtime=self.runtime, architecture=self.architecture ) package_builder = PythonPipDependencyBuilder( - osutils=self._os_utils, runtime=self.runtime, dependency_builder=dependency_builder + osutils=self._os_utils, runtime=self.runtime, python_exe=python_with_pip, dependency_builder=dependency_builder ) try: target_artifact_dir = self.artifacts_dir From 913df147f6a32254c41d146edbf39c496727db92 Mon Sep 17 00:00:00 2001 From: Justice Sidhu Date: Fri, 1 Dec 2023 14:53:18 -0800 Subject: [PATCH 3/4] update tests --- .../workflows/python_pip/actions.py | 11 ++++- .../workflows/python_pip/packager.py | 6 ++- .../workflows/python_pip/test_packager.py | 18 ++++----- .../unit/workflows/python_pip/test_actions.py | 8 +++- .../workflows/python_pip/test_packager.py | 40 ++++++++++--------- 5 files changed, 49 insertions(+), 34 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/actions.py b/aws_lambda_builders/workflows/python_pip/actions.py index 966cd1737..dcef47a96 100644 --- a/aws_lambda_builders/workflows/python_pip/actions.py +++ b/aws_lambda_builders/workflows/python_pip/actions.py @@ -49,11 +49,18 @@ def execute(self) -> None: pip_runner = PipRunner(python_exe=python_with_pip, pip=pip) dependency_builder = DependencyBuilder( - osutils=self._os_utils, python_exe=python_with_pip, pip_runner=pip_runner, runtime=self.runtime, architecture=self.architecture + osutils=self._os_utils, + python_exe=python_with_pip, + pip_runner=pip_runner, + runtime=self.runtime, + architecture=self.architecture, ) package_builder = PythonPipDependencyBuilder( - osutils=self._os_utils, runtime=self.runtime, python_exe=python_with_pip, dependency_builder=dependency_builder + osutils=self._os_utils, + runtime=self.runtime, + python_exe=python_with_pip, + dependency_builder=dependency_builder, ) try: target_artifact_dir = self.artifacts_dir diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 4d3aeb488..575ecb7d2 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -5,7 +5,6 @@ import logging import re import subprocess -import sys from email.parser import FeedParser from typing import Tuple @@ -365,7 +364,10 @@ def _download_all_dependencies(self, requirements_filename, directory): # which will serve as the primary list of dependencies needed to deploy # successfully. self._pip.download_all_dependencies(requirements_filename, directory) - deps = {Package(directory, filename, self.python_exe) for filename in self._osutils.get_directory_contents(directory)} + deps = { + Package(directory, filename, self.python_exe) + for filename in self._osutils.get_directory_contents(directory) + } LOG.debug("Full dependency closure: %s", deps) return deps diff --git a/tests/functional/workflows/python_pip/test_packager.py b/tests/functional/workflows/python_pip/test_packager.py index 615d5e60c..610f5cdd9 100644 --- a/tests/functional/workflows/python_pip/test_packager.py +++ b/tests/functional/workflows/python_pip/test_packager.py @@ -33,7 +33,7 @@ def _create_app_structure(tmpdir): @pytest.fixture def sdist_reader(): - return SDistMetadataFetcher() + return SDistMetadataFetcher(python_exe=sys.executable) @pytest.fixture @@ -141,7 +141,7 @@ def __init__(self, filename, dirarg, expected_args, whl_contents=None, expected_ def _build_fake_whl(self, directory, filename): filepath = os.path.join(directory, filename) if not os.path.isfile(filepath): - package = Package(directory, filename) + package = Package(directory, filename, python_exe=sys.executable) with zipfile.ZipFile(filepath, "w") as z: for content_path in self._whl_contents: z.writestr(content_path.format(package_name=self._package_name, data_dir=package.data_dir), b"") @@ -204,7 +204,7 @@ def _write_requirements_txt(self, packages, directory): def _make_appdir_and_dependency_builder(self, reqs, tmpdir, runner, **kwargs): appdir = str(_create_app_structure(tmpdir)) self._write_requirements_txt(reqs, appdir) - builder = DependencyBuilder(OSUtils(), "python3.9", runner, **kwargs) + builder = DependencyBuilder(OSUtils(), "python3.9", sys.executable, runner, **kwargs) return appdir, builder def test_can_build_local_dir_as_whl(self, tmpdir, pip_runner, osutils): @@ -1020,22 +1020,22 @@ def test_same_pkg_sdist_and_wheel_collide(self, osutils, sdist_builder): with osutils.tempdir() as tempdir: sdist_builder.write_fake_sdist(tempdir, "foobar", "1.0") pkgs = set() - pkgs.add(Package("", "foobar-1.0-py3-none-any.whl")) - pkgs.add(Package(tempdir, "foobar-1.0.zip")) + pkgs.add(Package("", "foobar-1.0-py3-none-any.whl", python_exe=sys.executable)) + pkgs.add(Package(tempdir, "foobar-1.0.zip", python_exe=sys.executable)) assert len(pkgs) == 1 def test_ensure_sdist_name_normalized_for_comparison(self, osutils, sdist_builder): with osutils.tempdir() as tempdir: sdist_builder.write_fake_sdist(tempdir, "Foobar", "1.0") pkgs = set() - pkgs.add(Package("", "foobar-1.0-py3-none-any.whl")) - pkgs.add(Package(tempdir, "Foobar-1.0.zip")) + pkgs.add(Package("", "foobar-1.0-py3-none-any.whl", python_exe=sys.executable)) + pkgs.add(Package(tempdir, "Foobar-1.0.zip", python_exe=sys.executable)) assert len(pkgs) == 1 def test_ensure_wheel_name_normalized_for_comparison(self, osutils, sdist_builder): with osutils.tempdir() as tempdir: sdist_builder.write_fake_sdist(tempdir, "foobar", "1.0") pkgs = set() - pkgs.add(Package("", "Foobar-1.0-py3-none-any.whl")) - pkgs.add(Package(tempdir, "foobar-1.0.zip")) + pkgs.add(Package("", "Foobar-1.0-py3-none-any.whl", python_exe=sys.executable)) + pkgs.add(Package(tempdir, "foobar-1.0.zip", python_exe=sys.executable)) assert len(pkgs) == 1 diff --git a/tests/unit/workflows/python_pip/test_actions.py b/tests/unit/workflows/python_pip/test_actions.py index 2917fdf42..8cd43c047 100644 --- a/tests/unit/workflows/python_pip/test_actions.py +++ b/tests/unit/workflows/python_pip/test_actions.py @@ -30,7 +30,9 @@ def test_action_must_call_builder(self, find_runtime_mock, dependency_builder_mo ) action.execute() - dependency_builder_mock.assert_called_with(osutils=ANY, pip_runner=ANY, runtime="runtime", architecture=X86_64) + dependency_builder_mock.assert_called_with( + osutils=ANY, pip_runner=ANY, runtime="runtime", architecture=X86_64, python_exe=ANY + ) builder_instance.build_dependencies.assert_called_with( artifacts_dir_path="artifacts", scratch_dir_path="scratch_dir", requirements_path="manifest" @@ -56,7 +58,9 @@ def test_action_must_call_builder_with_architecture( ) action.execute() - dependency_builder_mock.assert_called_with(osutils=ANY, pip_runner=ANY, runtime="runtime", architecture=ARM64) + dependency_builder_mock.assert_called_with( + osutils=ANY, pip_runner=ANY, runtime="runtime", architecture=ARM64, python_exe=ANY + ) builder_instance.build_dependencies.assert_called_with( artifacts_dir_path="artifacts", scratch_dir_path="scratch_dir", requirements_path="manifest" diff --git a/tests/unit/workflows/python_pip/test_packager.py b/tests/unit/workflows/python_pip/test_packager.py index 5b8db0216..e2d4300bc 100644 --- a/tests/unit/workflows/python_pip/test_packager.py +++ b/tests/unit/workflows/python_pip/test_packager.py @@ -1,7 +1,7 @@ import sys from collections import namedtuple from unittest import TestCase, mock -from unittest.mock import patch +from unittest.mock import ANY, patch import pytest from parameterized import parameterized @@ -115,7 +115,7 @@ def test_can_call_dependency_builder(self, osutils): mock_dep_builder = mock.Mock(spec=DependencyBuilder) osutils_mock = mock.Mock(spec=osutils) builder = PythonPipDependencyBuilder( - osutils=osutils_mock, dependency_builder=mock_dep_builder, runtime="runtime" + osutils=osutils_mock, dependency_builder=mock_dep_builder, runtime="runtime", python_exe=sys.executable ) builder.build_dependencies("artifacts/path/", "scratch_dir/path/", "path/to/requirements.txt") mock_dep_builder.build_site_packages.assert_called_once_with( @@ -125,24 +125,26 @@ def test_can_call_dependency_builder(self, osutils): @mock.patch("aws_lambda_builders.workflows.python_pip.packager.DependencyBuilder") def test_can_create_new_dependency_builder(self, DependencyBuilderMock, osutils): osutils_mock = mock.Mock(spec=osutils) - builder = PythonPipDependencyBuilder(osutils=osutils_mock, runtime="runtime") - DependencyBuilderMock.assert_called_with(osutils_mock, "runtime", architecture=X86_64) + builder = PythonPipDependencyBuilder(osutils=osutils_mock, runtime="runtime", python_exe=sys.executable) + DependencyBuilderMock.assert_called_with(osutils_mock, ANY, "runtime", architecture=X86_64) @mock.patch("aws_lambda_builders.workflows.python_pip.packager.DependencyBuilder") def test_can_call_dependency_builder_with_architecture(self, DependencyBuilderMock, osutils): osutils_mock = mock.Mock(spec=osutils) - builder = PythonPipDependencyBuilder(osutils=osutils_mock, runtime="runtime", architecture=ARM64) - DependencyBuilderMock.assert_called_with(osutils_mock, "runtime", architecture=ARM64) + builder = PythonPipDependencyBuilder( + osutils=osutils_mock, runtime="runtime", architecture=ARM64, python_exe=sys.executable + ) + DependencyBuilderMock.assert_called_with(osutils_mock, ANY, "runtime", architecture=ARM64) class TestPackage(object): def test_can_create_package_with_custom_osutils(self, osutils): - pkg = Package("", "foobar-1.0-py3-none-any.whl", osutils) + pkg = Package("", "foobar-1.0-py3-none-any.whl", sys.executable, osutils) assert pkg._osutils == osutils def test_wheel_package(self): filename = "foobar-1.0-py3-none-any.whl" - pkg = Package("", filename) + pkg = Package("", filename, python_exe=sys.executable) assert pkg.dist_type == "wheel" assert pkg.filename == filename assert pkg.identifier == "foobar==1.0" @@ -150,42 +152,42 @@ def test_wheel_package(self): def test_invalid_package(self): with pytest.raises(InvalidSourceDistributionNameError): - Package("", "foobar.jpg") + Package("", "foobar.jpg", python_exe=sys.executable) def test_diff_pkg_sdist_and_whl_do_not_collide(self): pkgs = set() - pkgs.add(Package("", "foobar-1.0-py3-none-any.whl")) - pkgs.add(Package("", "badbaz-1.0-py3-none-any.whl")) + pkgs.add(Package("", "foobar-1.0-py3-none-any.whl", python_exe=sys.executable)) + pkgs.add(Package("", "badbaz-1.0-py3-none-any.whl", python_exe=sys.executable)) assert len(pkgs) == 2 def test_same_pkg_is_eq(self): - pkg = Package("", "foobar-1.0-py3-none-any.whl") + pkg = Package("", "foobar-1.0-py3-none-any.whl", python_exe=sys.executable) assert pkg == pkg def test_pkg_is_eq_to_similar_pkg(self): - pure_pkg = Package("", "foobar-1.0-py3-none-any.whl") - plat_pkg = Package("", "foobar-1.0-py3-py39-manylinux1_x86_64.whl") + pure_pkg = Package("", "foobar-1.0-py3-none-any.whl", python_exe=sys.executable) + plat_pkg = Package("", "foobar-1.0-py3-py39-manylinux1_x86_64.whl", python_exe=sys.executable) assert pure_pkg == plat_pkg def test_pkg_is_not_equal_to_different_type(self): - pkg = Package("", "foobar-1.0-py3-none-any.whl") + pkg = Package("", "foobar-1.0-py3-none-any.whl", python_exe=sys.executable) non_package_type = 1 assert not (pkg == non_package_type) def test_pkg_repr(self): - pkg = Package("", "foobar-1.0-py3-none-any.whl") + pkg = Package("", "foobar-1.0-py3-none-any.whl", python_exe=sys.executable) assert repr(pkg) == "foobar==1.0(wheel)" def test_wheel_data_dir(self): - pkg = Package("", "foobar-2.0-py3-none-any.whl") + pkg = Package("", "foobar-2.0-py3-none-any.whl", python_exe=sys.executable) assert pkg.data_dir == "foobar-2.0.data" def test_can_read_packages_with_underscore_in_name(self): - pkg = Package("", "foo_bar-2.0-py3-none-any.whl") + pkg = Package("", "foo_bar-2.0-py3-none-any.whl", python_exe=sys.executable) assert pkg.identifier == "foo-bar==2.0" def test_can_read_packages_with_period_in_name(self): - pkg = Package("", "foo.bar-2.0-py3-none-any.whl") + pkg = Package("", "foo.bar-2.0-py3-none-any.whl", python_exe=sys.executable) assert pkg.identifier == "foo-bar==2.0" From e86be7c308ed9116405058dd148c455f2602d4c2 Mon Sep 17 00:00:00 2001 From: Justice Sidhu Date: Tue, 5 Dec 2023 09:59:04 -0800 Subject: [PATCH 4/4] comment explaining sys.executable in tests --- tests/functional/workflows/python_pip/test_packager.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/workflows/python_pip/test_packager.py b/tests/functional/workflows/python_pip/test_packager.py index 610f5cdd9..2a674afe8 100644 --- a/tests/functional/workflows/python_pip/test_packager.py +++ b/tests/functional/workflows/python_pip/test_packager.py @@ -33,6 +33,7 @@ def _create_app_structure(tmpdir): @pytest.fixture def sdist_reader(): + # We are removing references to sys.executable from the business logic but are using it here for testing purposes return SDistMetadataFetcher(python_exe=sys.executable)