From 910ea5bd7787e26db70f70ebdc0f50aba270ad7e Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Wed, 15 Mar 2023 12:20:31 +0000 Subject: [PATCH] (backport) pin pip<23.1 and setuptools<67 to allow PEP-440 non compliance. (backported relevant parts from master branch in #653) PEP-440 is more strict about requirement lines. As an example: pytz>dev is non-compliant, but is in 5.2.4 of kombu's requirements. This breaks building wheelhouses from source (such as the octavia charm). This patch pins pip and setuptools to the latest versions that will defintely still allow PEP-440 non-compliant packages to install. The option --upgrade-buildvenv-core-deps can be used to override this and will install the latest versions of pip and setuptools available. Fixes-Bug: #652 --- charmtools/build/tactics.py | 1 + charmtools/utils.py | 33 +++++++++++++++++++++++++++++++++ tests/test_build.py | 6 ++++-- tests/test_utils.py | 25 +++++++++++++++++++++++++ tests/wh-over.txt | 2 +- 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/charmtools/build/tactics.py b/charmtools/build/tactics.py index 68c35e08..42fb2a17 100644 --- a/charmtools/build/tactics.py +++ b/charmtools/build/tactics.py @@ -1230,6 +1230,7 @@ def __call__(self): utils.Process( ('virtualenv', '--python', 'python3', self._venv) ).exit_on_error()() + utils.pin_setuptools_for_pep440(self._venv, env=utils.host_env()) if self.per_layer: self._process_per_layer(wheelhouse) else: diff --git a/charmtools/utils.py b/charmtools/utils.py index 3c98da30..77219e49 100644 --- a/charmtools/utils.py +++ b/charmtools/utils.py @@ -633,3 +633,36 @@ def validate_display_name(entity, linter): linter.err('display-name: not in valid format. ' 'Only letters, numbers, dashes, and hyphens are permitted.') return + + +def host_env(): + """Get environment appropriate for executing commands outside snap context. + :returns: Dictionary with environment variables + :rtype: Dict[str,str] + """ + env = os.environ.copy() + for key in ('PREFIX', 'PYTHONHOME', 'PYTHONPATH', + 'GIT_TEMPLATE_DIR', 'GIT_EXEC_PATH'): + if key in env: + del(env[key]) + env['PATH'] = ':'.join([ + element + for element in env['PATH'].split(':') + if not element.startswith('/snap/charm/')]) + return env + + +def pin_setuptools_for_pep440(venv_dir, env=None): + """Pin setuptools so that pep440 non-compliant packages can be installed. + Also pins pip as it's a combination that definitely works. + :param venv_dir: Full path to virtualenv in which packages will be upgraded + :type venv_dir: str + :param env: Environment to use when executing command + :type env: Optional[Dict[str,str]] + :returns: This function is called for its side effect + """ + log.debug('Pinning setuptools < 67 for pep440 non compliant packages "{}"' + .format(venv_dir)) + Process((os.path.join(venv_dir, 'bin/pip'), + 'install', '-U', 'pip<23.1', 'setuptools<67'), + env=env).exit_on_error()() diff --git a/tests/test_build.py b/tests/test_build.py index e3cbebd1..5de09033 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -431,6 +431,7 @@ def test_version_tactic_with_existing_version_file(self, mcall, ph, pi, self.assertEqual((bu.target_dir / 'version').text(), 'sha2') + @mock.patch("charmtools.utils.pin_setuptools_for_pep440") @mock.patch("charmtools.utils.sign") @mock.patch("charmtools.build.builder.Builder.plan_version") @mock.patch("charmtools.build.builder.Builder.plan_interfaces") @@ -438,7 +439,8 @@ def test_version_tactic_with_existing_version_file(self, mcall, ph, pi, @mock.patch("path.Path.rmtree_p") @mock.patch("tempfile.mkdtemp") @mock.patch("charmtools.utils.Process") - def test_wheelhouse(self, Process, mkdtemp, rmtree_p, ph, pi, pv, sign): + def test_wheelhouse(self, Process, mkdtemp, rmtree_p, ph, pi, pv, sign, + pin_setuptools_for_pep440): build.tactics.WheelhouseTactic.per_layer = False mkdtemp.return_value = '/tmp' bu = build.Builder() @@ -486,7 +488,7 @@ def _store_wheelhouses(args): '', '# --wheelhouse-overrides', 'git+https://github.com/me/qux#egg=qux', - 'setuptools_scm>=3.0<=3.4.1', + 'setuptools_scm>=3.0,<=3.4.1', '', ]) diff --git a/tests/test_utils.py b/tests/test_utils.py index 4b9fc56d..3e634788 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,6 @@ from __future__ import print_function +import unittest from unittest import TestCase from charmtools import utils from six import StringIO @@ -43,3 +44,27 @@ def react(db): self.assertIn("Beta", output) self.assertIn("@when('db.ready'", output) self.assertIn("bar", output) + + @unittest.mock.patch("os.environ") + def test_host_env(self, mock_environ): + mock_environ.copy.return_value = { + 'PREFIX': 'fake-prefix', + 'PYTHONHOME': 'fake-pythonhome', + 'PYTHONPATH': 'fake-pythonpath', + 'GIT_TEMPLATE_DIR': 'fake-git-template-dir', + 'GIT_EXEC_PATH': 'fake-git-exec-path', + 'SOME_OTHER_KEY': 'fake-some-other-key', + 'PATH': '/snap/charm/current/bin:/usr/bin:' + '/snap/charm/current/usr/bin:/bin', + } + self.assertDictEqual( + {'SOME_OTHER_KEY': 'fake-some-other-key', 'PATH': '/usr/bin:/bin'}, + utils.host_env()) + + @unittest.mock.patch.object(utils, "Process") + def test_pin_setuptools_for_pep440(self, mock_Process): + utils.pin_setuptools_for_pep440('/some/dir', env={'some': 'envvar'}) + mock_Process.assert_called_once_with( + ('/some/dir/bin/pip', 'install', '-U', 'pip<23.1', + 'setuptools<67'), + env={'some': 'envvar'}) diff --git a/tests/wh-over.txt b/tests/wh-over.txt index 73f53e54..7a653774 100644 --- a/tests/wh-over.txt +++ b/tests/wh-over.txt @@ -1,2 +1,2 @@ git+https://github.com/me/qux#egg=qux -setuptools_scm>=3.0<=3.4.1 +setuptools_scm>=3.0,<=3.4.1