Merge pull request #650 from fnordahl/issue/646

Add option to upgrade core deps in build venv

.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

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)
 

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:

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()

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)