From 85f1e59021a4e836c7afdf30554b4e24c43cb0c6 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Thu, 10 Nov 2022 23:34:46 +0100 Subject: [PATCH] Add option to upgrade core deps in build venv Some packages may require newer versions of core dependencies for successful execution of the `pip download` or `pip wheel` commands. Most notable example is Jinja2 and MarkupSafe when installed on Focal ref pallets/jinja#1496. At this stage of the process the input from the charm wheelhouse.txt is not sufficient. Add an option to upgrade pip and setuptools in the build venv to deal with these situations. Fixes: #646 Signed-off-by: Frode Nordahl --- .github/workflows/tests.yml | 12 +----------- charmtools/build/builder.py | 6 ++++++ charmtools/build/tactics.py | 29 ++++++++++++++++++++++------- charmtools/utils.py | 33 +++++++++++++++++++++++++++++++++ tests/test_utils.py | 20 ++++++++++++++++++++ 5 files changed, 82 insertions(+), 18 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7e3e6da..b5398ce 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -54,17 +54,6 @@ jobs: with: repository: juju-solutions/layer-basic - - name: Fixup wheelhouse - run: | - set -euxo pipefail - cat << EOF | tee -a tests/charm-minimal/wheelhouse.txt - # https://github.com/pallets/jinja/issues/1496 - # - # We ought to teach charm-tools to seed the virtualenv used to build - # wheels with newer versions of pip and setuptools. - Jinja2<3 - EOF - - name: Download built charm snap uses: actions/download-artifact@v3 with: @@ -90,6 +79,7 @@ jobs: reactive-charm-build-arguments: - -v - --binary-wheels-from-source + - --upgrade-buildvenv-core-deps build-packages: - python3-dev - libpq-dev diff --git a/charmtools/build/builder.py b/charmtools/build/builder.py index 2020127..c828521 100755 --- a/charmtools/build/builder.py +++ b/charmtools/build/builder.py @@ -1205,6 +1205,11 @@ def main(args=None): default=False, help='Use Python and associated tools from the snap ' 'when building the wheelhouse.') + parser.add_argument('--upgrade-buildvenv-core-deps', action='store_true', + default=False, + help='Upgrade core dependencies in virtualenv used to ' + 'build/download wheels: pip setuptools to the latest ' + 'version in PyPI.') parser.add_argument('charm', nargs="?", default=".", type=path, help='Source directory for charm layer to build ' '(default: .)') @@ -1226,6 +1231,7 @@ def main(args=None): WheelhouseTactic.binary_build = build.binary_wheels WheelhouseTactic.binary_build_from_source = build.binary_wheels_from_source WheelhouseTactic.use_python_from_snap = build.use_python_from_snap + WheelhouseTactic.upgrade_deps = build.upgrade_buildvenv_core_deps configLogging(build) diff --git a/charmtools/build/tactics.py b/charmtools/build/tactics.py index 667fcfc..c3ca269 100644 --- a/charmtools/build/tactics.py +++ b/charmtools/build/tactics.py @@ -1047,6 +1047,7 @@ class WheelhouseTactic(ExactMatch, Tactic): binary_build = False binary_build_from_source = False use_python_from_snap = False + upgrade_deps = False def __init__(self, *args, **kwargs): super(WheelhouseTactic, self).__init__(*args, **kwargs) @@ -1124,13 +1125,20 @@ def _add(self, wheelhouse, *reqs): with utils.tempdir(chdir=False) as temp_dir: # put in a temp dir first to ensure we track all of the files _no_binary_opts = ('--no-binary', ':all:') - if self.binary_build_from_source or self.binary_build: - self._pip('wheel', - *_no_binary_opts - if self.binary_build_from_source else tuple(), - '-w', temp_dir, *reqs) - else: - self._pip('download', *_no_binary_opts, '-d', temp_dir, *reqs) + try: + if self.binary_build_from_source or self.binary_build: + self._pip('wheel', + *_no_binary_opts + if self.binary_build_from_source else tuple(), + '-w', temp_dir, *reqs) + else: + self._pip('download', *_no_binary_opts, '-d', temp_dir, *reqs) + except BuildError: + log.info('Build failed. If you are building on Focal and have ' + 'Jinja2 or MarkupSafe as part of your dependencies, ' + 'try passing the `--upgrade-buildvenv-core-deps` ' + 'argument.') + raise log.debug('Copying wheels:') for wheel in temp_dir.files(): log.debug(' ' + wheel.name) @@ -1242,6 +1250,13 @@ def __call__(self): ('virtualenv', '--python', 'python3', self._venv), env=self._get_env() ).exit_on_error()() + if self.upgrade_deps: + utils.upgrade_venv_core_packages(self._venv, env=self._get_env()) + log.debug( + 'Packages in buildvenv:\n{}' + .format( + utils.get_venv_package_list(self._venv, + env=self._get_env()))) if self.per_layer: self._process_per_layer(wheelhouse) else: diff --git a/charmtools/utils.py b/charmtools/utils.py index 10808fa..ed41983 100644 --- a/charmtools/utils.py +++ b/charmtools/utils.py @@ -651,3 +651,36 @@ def host_env(): for element in env['PATH'].split(':') if not element.startswith('/snap/charm/')]) return env + + +def upgrade_venv_core_packages(venv_dir, env=None): + """Upgrade core dependencies in venv. + + :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('Upgrade pip and setuptools in virtualenv "{}"'.format(venv_dir)) + # We can replace this with a call to `python3 -m venv --upgrade-deps` once + # support for bionic and focal has been removed. + Process((os.path.join(venv_dir, 'bin/pip'), + 'install', '-U', 'pip', 'setuptools'), + env=env).exit_on_error()() + + +def get_venv_package_list(venv_dir, env=None): + """Get list of packages and their version in virtualenv. + + :param venv_dir: Full path to virtualenv in which packages will be listed + :type venv_dir: str + :param env: Environment to use when executing command + :type env: Optional[Dict[str,str]] + :returns: List of packages with their versions + :rtype: str + """ + result = Process((os.path.join(venv_dir, 'bin/pip'), 'list'), env=env)() + if result: + return result.output + result.exit_on_error() diff --git a/tests/test_utils.py b/tests/test_utils.py index 105c836..800d7ac 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -60,3 +60,23 @@ def test_host_env(self, mock_environ): self.assertDictEqual( {'SOME_OTHER_KEY': 'fake-some-other-key', 'PATH': '/usr/bin:/bin'}, utils.host_env()) + + @unittest.mock.patch.object(utils, "Process") + def test_upgrade_venv_core_packages(self, mock_Process): + utils.upgrade_venv_core_packages('/some/dir', env={'some': 'envvar'}) + mock_Process.assert_called_once_with( + ('/some/dir/bin/pip', 'install', '-U', 'pip', 'setuptools'), + env={'some': 'envvar'}) + + @unittest.mock.patch("sys.exit") + @unittest.mock.patch.object(utils, "Process") + def test_get_venv_package_list(self, mock_Process, mock_sys_exit): + mock_Process().return_value = utils.ProcessResult('fakecmd', 0, '', '') + utils.get_venv_package_list('/some/dir', env={'some': 'envvar'}) + mock_Process.assert_called_with( + ('/some/dir/bin/pip', 'list'), + env={'some': 'envvar'}) + self.assertFalse(mock_sys_exit.called) + mock_Process().return_value = utils.ProcessResult('fakecmd', 1, '', '') + utils.get_venv_package_list('/some/dir', env={'some': 'envvar'}) + mock_sys_exit.assert_called_once_with(1)