From 9133418ce1bec51fe9ee8d9abdce483c826339e2 Mon Sep 17 00:00:00 2001 From: Sriram Madapusi Vasudevan Date: Wed, 16 Oct 2019 15:55:27 -0700 Subject: [PATCH 1/2] fix: change the pip runner string based on a pip version check - if pip version is less than 19.3, choose to run main directly as its a function, whereas with pip versions starting with 19.3, execute the function `main` inside the pip.main module --- .../workflows/python_pip/compat.py | 17 +++++++++++++++++ .../workflows/python_pip/packager.py | 5 +++-- .../unit/workflows/python_pip/test_packager.py | 17 ++++++++++++++--- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/compat.py b/aws_lambda_builders/workflows/python_pip/compat.py index ff35cbabe..912856d95 100644 --- a/aws_lambda_builders/workflows/python_pip/compat.py +++ b/aws_lambda_builders/workflows/python_pip/compat.py @@ -21,6 +21,23 @@ def pip_import_string(python_exe): return 'from pip._internal import main' +def pip_runner_string(python_exe): + os_utils = OSUtils() + cmd = [ + python_exe, + "-c", + "import pip; assert (int(pip.__version__.split('.')[0]) <= 19 and int(pip.__version__.split('.')[1]) < 3)" + ] + p = os_utils.popen(cmd, stdout=os_utils.pipe, stderr=os_utils.pipe) + p.communicate() + # Pip changed main to be a module instead of a function from 19.3 + # and added a separate main function within the main module. + if p.returncode == 0: + return 'import sys; %s; sys.exit(main(%s))' + else: + return 'import sys; %s; sys.exit(main.main(%s))' + + if os.name == 'nt': # windows # This is the actual patch used on windows to prevent distutils from diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index a89ae626e..0bdc3f731 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -9,7 +9,7 @@ from email.parser import FeedParser -from .compat import pip_import_string +from .compat import pip_import_string, pip_runner_string from .compat import pip_no_compile_c_env_vars from .compat import pip_no_compile_c_shim from .utils import OSUtils @@ -553,6 +553,7 @@ def __init__(self, osutils=None, python_exe=None, import_string=None): self.python_exe = python_exe if import_string is None: import_string = pip_import_string(python_exe=self.python_exe) + self.run_pip_string = pip_runner_string(python_exe=self.python_exe) self._import_string = import_string def main(self, args, env_vars=None, shim=None): @@ -561,7 +562,7 @@ def main(self, args, env_vars=None, shim=None): if shim is None: shim = '' run_pip = ( - 'import sys; %s; sys.exit(main(%s))' + self.run_pip_string ) % (self._import_string, args) exec_string = '%s%s' % (shim, run_pip) invoke_pip = [self.python_exe, '-c', exec_string] diff --git a/tests/unit/workflows/python_pip/test_packager.py b/tests/unit/workflows/python_pip/test_packager.py index 937f52aad..452d23987 100644 --- a/tests/unit/workflows/python_pip/test_packager.py +++ b/tests/unit/workflows/python_pip/test_packager.py @@ -1,5 +1,6 @@ import sys from collections import namedtuple +from unittest import TestCase import mock import pytest @@ -70,7 +71,7 @@ def osutils(): class FakePopen(object): def __init__(self, rc, out, err): - self.returncode = 0 + self.returncode = rc self._out = out self._err = err @@ -303,14 +304,24 @@ def test_inject_unknown_error_if_no_stderr(self, pip_factory): assert str(einfo.value) == 'Unknown error' -class TestSubprocessPip(object): +class TestSubprocessPip(TestCase): def test_does_use_custom_pip_import_string(self): fake_osutils = FakePopenOSUtils([FakePopen(0, '', '')]) expected_import_statement = 'foobarbaz' pip = SubprocessPip(osutils=fake_osutils, - import_string=expected_import_statement) + import_string=expected_import_statement, + python_exe=sys.executable) pip.main(['--version']) pip_execution_string = fake_osutils.popens[0][0][0][2] import_statement = pip_execution_string.split(';')[1].strip() assert import_statement == expected_import_statement + + def test_check_pip_runner_string_pip(self): + fake_osutils = FakePopenOSUtils([FakePopen(0, '', '')]) + pip = SubprocessPip(osutils=fake_osutils, + python_exe=sys.executable) + pip.main(['--version']) + + pip_runner_string = fake_osutils.popens[0][0][0][2].split(";")[-1:][0] + self.assertIn("main", pip_runner_string) From c1f09ba50edc4cc0c49779cbc9c472587165fecd Mon Sep 17 00:00:00 2001 From: Sriram Madapusi Vasudevan Date: Thu, 17 Oct 2019 12:33:50 -0700 Subject: [PATCH 2/2] fix: adopt same methodology as chalice for determining pip import string --- .../workflows/python_pip/compat.py | 33 +++++++------------ .../workflows/python_pip/packager.py | 5 ++- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/aws_lambda_builders/workflows/python_pip/compat.py b/aws_lambda_builders/workflows/python_pip/compat.py index 912856d95..2eb885f20 100644 --- a/aws_lambda_builders/workflows/python_pip/compat.py +++ b/aws_lambda_builders/workflows/python_pip/compat.py @@ -8,36 +8,27 @@ def pip_import_string(python_exe): cmd = [ python_exe, "-c", - "import pip; assert int(pip.__version__.split('.')[0]) <= 9" + "import pip; print(pip.__version__)" ] - p = os_utils.popen(cmd,stdout=os_utils.pipe, stderr=os_utils.pipe) - p.communicate() + p = os_utils.popen(cmd, stdout=os_utils.pipe, stderr=os_utils.pipe) + stdout, stderr = p.communicate() + pip_version = stdout.decode('utf-8').strip() + pip_major_version = int(pip_version.split('.')[0]) + pip_minor_version = int(pip_version.split('.')[1]) + # Pip moved its internals to an _internal module in version 10. # In order to be compatible with version 9 which has it at at the # top level we need to figure out the correct import path here. - if p.returncode == 0: + if pip_major_version == 9: return 'from pip import main' + # Pip changed their import structure again in 19.3 + # https://github.com/pypa/pip/commit/09fd200 + elif pip_major_version >= 19 and pip_minor_version >= 3: + return 'from pip._internal.main import main' else: return 'from pip._internal import main' -def pip_runner_string(python_exe): - os_utils = OSUtils() - cmd = [ - python_exe, - "-c", - "import pip; assert (int(pip.__version__.split('.')[0]) <= 19 and int(pip.__version__.split('.')[1]) < 3)" - ] - p = os_utils.popen(cmd, stdout=os_utils.pipe, stderr=os_utils.pipe) - p.communicate() - # Pip changed main to be a module instead of a function from 19.3 - # and added a separate main function within the main module. - if p.returncode == 0: - return 'import sys; %s; sys.exit(main(%s))' - else: - return 'import sys; %s; sys.exit(main.main(%s))' - - if os.name == 'nt': # windows # This is the actual patch used on windows to prevent distutils from diff --git a/aws_lambda_builders/workflows/python_pip/packager.py b/aws_lambda_builders/workflows/python_pip/packager.py index 0bdc3f731..a89ae626e 100644 --- a/aws_lambda_builders/workflows/python_pip/packager.py +++ b/aws_lambda_builders/workflows/python_pip/packager.py @@ -9,7 +9,7 @@ from email.parser import FeedParser -from .compat import pip_import_string, pip_runner_string +from .compat import pip_import_string from .compat import pip_no_compile_c_env_vars from .compat import pip_no_compile_c_shim from .utils import OSUtils @@ -553,7 +553,6 @@ def __init__(self, osutils=None, python_exe=None, import_string=None): self.python_exe = python_exe if import_string is None: import_string = pip_import_string(python_exe=self.python_exe) - self.run_pip_string = pip_runner_string(python_exe=self.python_exe) self._import_string = import_string def main(self, args, env_vars=None, shim=None): @@ -562,7 +561,7 @@ def main(self, args, env_vars=None, shim=None): if shim is None: shim = '' run_pip = ( - self.run_pip_string + 'import sys; %s; sys.exit(main(%s))' ) % (self._import_string, args) exec_string = '%s%s' % (shim, run_pip) invoke_pip = [self.python_exe, '-c', exec_string]