run pip check only once for PythonBundle

easybuild/easyblocks/generic/pythonbundle.py

@@ -30,7 +30,7 @@
 import os
 
 from easybuild.easyblocks.generic.bundle import Bundle
-from easybuild.easyblocks.generic.pythonpackage import EXTS_FILTER_PYTHON_PACKAGES
+from easybuild.easyblocks.generic.pythonpackage import EXTS_FILTER_PYTHON_PACKAGES, run_pip_check
 from easybuild.easyblocks.generic.pythonpackage import PythonPackage, get_pylibdirs, find_python_cmd_from_ec
 from easybuild.tools.build_log import EasyBuildError
 from easybuild.tools.config import build_option, PYTHONPATH, EBPYTHONPREFIXES
@@ -163,6 +163,15 @@ def load_module(self, *args, **kwargs):
     def sanity_check_step(self, *args, **kwargs):
         """Custom sanity check for bundle of Python package."""
 
+        if self.pylibdir is None:
+            # Python attributes not set up yet, happens e.g. with --sanity-check-only, so do it now.
+            # This also ensures the exts_filter option for extensions is set correctly.
+            # Load module first to get the right python command.
+            if not self.sanity_check_module_loaded:
+                self.sanity_check_load_module(extension=kwargs.get('extension', False),
+                                              extra_modules=kwargs.get('extra_modules', None))
+            self.prepare_python()
+
         # inject directory path that uses %(pyshortver)s template into default value for sanity_check_paths
         # this is relevant for installations of Python bundles for multiple Python versions (via multi_deps)
         # (we can not pass this via custom_paths, since then the %(pyshortver)s template value will not be resolved)
@@ -173,3 +182,32 @@ def sanity_check_step(self, *args, **kwargs):
             }
 
         super().sanity_check_step(*args, **kwargs)
+
+    def _sanity_check_step_extensions(self):
+        """Run the pip check for extensions if enabled"""
+        super(PythonBundle, self)._sanity_check_step_extensions()
+
+        sanity_pip_check = self.cfg['sanity_pip_check']
+        unversioned_packages = set(self.cfg['unversioned_packages'])
+
+        # The options should be set in the main EC and cannot be different between extensions.
+        # For backwards compatibility and to avoid surprises enable the pip-check if it is enabled
+        # in the main EC or any extension and build the union of all unversioned_packages.
+        has_sanity_pip_check_mismatch = False
+        all_unversioned_packages = unversioned_packages.copy()
+        for ext in self.ext_instances:
+            if isinstance(ext, PythonPackage):
+                if ext.cfg['sanity_pip_check'] != sanity_pip_check:
+                    has_sanity_pip_check_mismatch = True
+                all_unversioned_packages.update(ext.cfg['unversioned_packages'])
+
+        if has_sanity_pip_check_mismatch:
+            self.log.deprecated("For bundles of PythonPackage extensions the sanity_pip_check parameter "
+                                "must be set at the top level, outside of exts_list", '6.0')
+            sanity_pip_check = True  # Either the main set it or any extension enabled it
+        if all_unversioned_packages != unversioned_packages:
+            self.log.deprecated("For bundles of PythonPackage extensions the unversioned_packages parameter "
+                                "must be set at the top level, outside of exts_list", '6.0')
+
+        if sanity_pip_check:
+            run_pip_check(python_cmd=self.python_cmd, unversioned_packages=all_unversioned_packages)

easybuild/easyblocks/generic/pythonpackage.py

@@ -51,7 +51,7 @@
 from easybuild.tools.config import build_option, PYTHONPATH, EBPYTHONPREFIXES
 from easybuild.tools.filetools import change_dir, mkdir, remove_dir, symlink, which
 from easybuild.tools.modules import ModEnvVarType, get_software_root
-from easybuild.tools.run import run_shell_cmd, subprocess_popen_text
+from easybuild.tools.run import run_shell_cmd
 from easybuild.tools.utilities import nub
 from easybuild.tools.hooks import CONFIGURE_STEP, BUILD_STEP, TEST_STEP, INSTALL_STEP
 
@@ -398,6 +398,108 @@ def symlink_dist_site_packages(install_dir, pylibdirs):
             symlink(dist_pkgs, site_pkgs_path, use_abspath_source=False)
 
 
+def det_installed_python_packages(names_only=True, python_cmd=None):
+    """
+    Return list of Python packages that are installed
+
+    Note that the names are reported by pip and might be different to the name that need to be used to import it.
+
+    :param names_only: boolean indicating whether only names or full info from `pip list` should be returned
+    :param python_cmd: Python command to use (if None, 'python' is used)
+    """
+    log = fancylogger.getLogger('det_installed_python_packages', fname=False)
+
+    if python_cmd is None:
+        python_cmd = 'python'
+
+    # Check installed Python packages
+    cmd = ' '.join([
+        python_cmd, '-m', 'pip',
+        'list',
+        '--isolated',
+        '--disable-pip-version-check',
+        '--format', 'json',
+    ])
+    res = run_shell_cmd(cmd, fail_on_error=False, hidden=True)
+    if res.exit_code:
+        raise EasyBuildError(f'Failed to determine installed python packages: {res.output}')
+
+    # only check stdout, not stderr which might contain user facing warnings
+    log.info(f'Got list of installed Python packages: {res.output}')
+    pkgs = json.loads(res.output.strip())
+    return [pkg['name'] for pkg in pkgs] if names_only else pkgs
+
+
+def run_pip_check(python_cmd=None, unversioned_packages=None):
+    """
+    Check installed Python packages using 'pip check'
+
+    :param unversioned_packages: list of Python packages to exclude in the version existence check
+    :param python_cmd: Python command to use (if None, 'python' is used)
+    """
+    log = fancylogger.getLogger('det_installed_python_packages', fname=False)
+
+    if python_cmd is None:
+        python_cmd = 'python'
+    if unversioned_packages is None:
+        unversioned_packages = []
+
+    pip_check_cmd = f"{python_cmd} -m pip check"
+
+    pip_version = det_pip_version(python_cmd=python_cmd)
+    if not pip_version:
+        raise EasyBuildError("Failed to determine pip version!")
+    min_pip_version = LooseVersion('9.0.0')
+    if LooseVersion(pip_version) < min_pip_version:
+        raise EasyBuildError(f"pip >= {min_pip_version} is required for '{pip_check_cmd}', found {pip_version}")
+
+    pip_check_errors = []
+
+    res = run_shell_cmd(pip_check_cmd, fail_on_error=False)
+    if res.exit_code:
+        pip_check_errors.append(f"`{pip_check_cmd}` failed:\n{res.output}")
+    else:
+        log.info(f"`{pip_check_cmd}` passed successfully")
+
+    # Also check for a common issue where the package version shows up as 0.0.0 often caused
+    # by using setup.py as the installation method for a package which is released as a generic wheel
+    # named name-version-py2.py3-none-any.whl. `tox` creates those from version controlled source code
+    # so it will contain a version, but the raw tar.gz does not.
+    pkgs = det_installed_python_packages(names_only=False, python_cmd=python_cmd)
+    faulty_version = '0.0.0'
+    faulty_pkg_names = [pkg['name'] for pkg in pkgs if pkg['version'] == faulty_version]
+
+    for unversioned_package in unversioned_packages:
+        try:
+            faulty_pkg_names.remove(unversioned_package)
+            log.debug(f"Excluding unversioned package '{unversioned_package}' from check")
+        except ValueError:
+            try:
+                version = next(pkg['version'] for pkg in pkgs if pkg['name'] == unversioned_package)
+            except StopIteration:
+                msg = f"Package '{unversioned_package}' in unversioned_packages was not found in "
+                msg += "the installed packages. Check that the name from `python -m pip list` is used "
+                msg += "which may be different than the module name."
+            else:
+                msg = f"Package '{unversioned_package}' in unversioned_packages has a version of {version} "
+                msg += "which is valid. Please remove it from unversioned_packages."
+            pip_check_errors.append(msg)
+
+    log.info("Found %s invalid packages out of %s packages", len(faulty_pkg_names), len(pkgs))
+    if faulty_pkg_names:
+        faulty_pkg_names_str = '\n'.join(faulty_pkg_names)
+        msg = "The following Python packages were likely not installed correctly because they show a "
+        msg += f"version of '{faulty_version}':\n{faulty_pkg_names_str}\n"
+        msg += "This may be solved by using a *-none-any.whl file as the source instead. "
+        msg += "See e.g. the SOURCE*_WHL templates.\n"
+        msg += "Otherwise you could check if the package provides a version at all or if e.g. poetry is "
+        msg += "required (check the source for a pyproject.toml and see PEP517 for details on that)."
+        pip_check_errors.append(msg)
+
+    if pip_check_errors:
+        raise EasyBuildError('\n'.join(pip_check_errors))
+
+
 class PythonPackage(ExtensionEasyBlock):
     """Builds and installs a Python package, and provides a dedicated module file."""
 
@@ -611,25 +713,7 @@ def get_installed_python_packages(self, names_only=True, python_cmd=None):
         """
         if python_cmd is None:
             python_cmd = self.python_cmd
-        # Check installed python packages but only check stdout, not stderr which might contain user facing warnings
-        cmd_list = [python_cmd, '-m', 'pip', 'list', '--isolated', '--disable-pip-version-check',
-                    '--format', 'json']
-        full_cmd = ' '.join(cmd_list)
-        self.log.info("Running command '%s'" % full_cmd)
-        proc = subprocess_popen_text(cmd_list, env=os.environ)
-        (stdout, stderr) = proc.communicate()
-        ec = proc.returncode
-        msg = "Command '%s' returned with %s: stdout: %s; stderr: %s" % (full_cmd, ec, stdout, stderr)
-        if ec:
-            self.log.info(msg)
-            raise EasyBuildError('Failed to determine installed python packages: %s', stderr)
-
-        self.log.debug(msg)
-        pkgs = json.loads(stdout.strip())
-        if names_only:
-            return [pkg['name'] for pkg in pkgs]
-        else:
-            return pkgs
+        return det_installed_python_packages(names_only=names_only, python_cmd=python_cmd)
 
     def using_pip_install(self):
         """
@@ -1019,10 +1103,10 @@ def sanity_check_step(self, *args, **kwargs):
         # load module early ourselves rather than letting parent sanity_check_step method do so,
         # since custom actions taken below require that environment is set up properly already
         # (especially when using --sanity-check-only)
-        if hasattr(self, 'sanity_check_module_loaded') and not self.sanity_check_module_loaded:
+        if not self.sanity_check_module_loaded:
             extension = self.is_extension or kwargs.get('extension', False)
             extra_modules = kwargs.get('extra_modules', None)
-            self.fake_mod_data = self.sanity_check_load_module(extension=extension, extra_modules=extra_modules)
+            self.sanity_check_load_module(extension=extension, extra_modules=extra_modules)
 
         # don't add user site directory to sys.path (equivalent to python -s)
         # see https://www.python.org/dev/peps/pep-0370/;
@@ -1076,78 +1160,31 @@ def sanity_check_step(self, *args, **kwargs):
                 exts_filter = (orig_exts_filter[0].replace('python', self.python_cmd), orig_exts_filter[1])
                 kwargs.update({'exts_filter': exts_filter})
 
-        if self.cfg.get('sanity_pip_check', True):
-            pip_version = det_pip_version(python_cmd=python_cmd)
-
-            if pip_version:
-                pip_check_command = "%s -m pip check" % python_cmd
-
-                if LooseVersion(pip_version) >= LooseVersion('9.0.0'):
-
-                    if not self.is_extension:
-                        # for stand-alone Python package installations (not part of a bundle of extensions),
-                        # the (fake or real) module file must be loaded at this point,
-                        # otherwise the Python package being installed is not "in view",
-                        # and we will overlook missing dependencies...
-                        loaded_modules = [x['mod_name'] for x in self.modules_tool.list()]
-                        if self.short_mod_name not in loaded_modules:
-                            self.log.debug("Currently loaded modules: %s", loaded_modules)
-                            raise EasyBuildError("%s module is not loaded, this should never happen...",
-                                                 self.short_mod_name)
-
-                    pip_check_errors = []
-
-                    res = run_shell_cmd(pip_check_command, fail_on_error=False)
-                    pip_check_msg = res.output
-                    if res.exit_code:
-                        pip_check_errors.append('`%s` failed:\n%s' % (pip_check_command, pip_check_msg))
-                    else:
-                        self.log.info('`%s` completed successfully' % pip_check_command)
-
-                    # Also check for a common issue where the package version shows up as 0.0.0 often caused
-                    # by using setup.py as the installation method for a package which is released as a generic wheel
-                    # named name-version-py2.py3-none-any.whl. `tox` creates those from version controlled source code
-                    # so it will contain a version, but the raw tar.gz does not.
-                    pkgs = self.get_installed_python_packages(names_only=False, python_cmd=python_cmd)
-                    faulty_version = '0.0.0'
-                    faulty_pkg_names = [pkg['name'] for pkg in pkgs if pkg['version'] == faulty_version]
-
-                    for unversioned_package in self.cfg.get('unversioned_packages', []):
-                        try:
-                            faulty_pkg_names.remove(unversioned_package)
-                            self.log.debug('Excluding unversioned package %s from check', unversioned_package)
-                        except ValueError:
-                            try:
-                                version = next(pkg['version'] for pkg in pkgs if pkg['name'] == unversioned_package)
-                            except StopIteration:
-                                msg = ('Package %s in unversioned_packages was not found in the installed packages. '
-                                       'Check that the name from `python -m pip list` is used which may be different '
-                                       'than the module name.' % unversioned_package)
-                            else:
-                                msg = ('Package %s in unversioned_packages has a version of %s which is valid. '
-                                       'Please remove it from unversioned_packages.' % (unversioned_package, version))
-                            pip_check_errors.append(msg)
-
-                    self.log.info('Found %s invalid packages out of %s packages', len(faulty_pkg_names), len(pkgs))
-                    if faulty_pkg_names:
-                        msg = (
-                            "The following Python packages were likely not installed correctly because they show a "
-                            "version of '%s':\n%s\n"
-                            "This may be solved by using a *-none-any.whl file as the source instead. "
-                            "See e.g. the SOURCE*_WHL templates.\n"
-                            "Otherwise you could check if the package provides a version at all or if e.g. poetry is "
-                            "required (check the source for a pyproject.toml and see PEP517 for details on that)."
-                         ) % (faulty_version, '\n'.join(faulty_pkg_names))
-                        pip_check_errors.append(msg)
-
-                    if pip_check_errors:
-                        raise EasyBuildError('\n'.join(pip_check_errors))
-                else:
-                    raise EasyBuildError("pip >= 9.0.0 is required for running '%s', found %s",
-                                         pip_check_command,
-                                         pip_version)
-            else:
-                raise EasyBuildError("Failed to determine pip version!")
+        sanity_pip_check = self.cfg.get('sanity_pip_check', True)
+        if self.is_extension:
+            sanity_pip_check_main = self.master.cfg.get('sanity_pip_check')
+            if sanity_pip_check_main is not None:
+                # If the main easyblock (e.g. PythonBundle) defines the variable
+                # we trust it does the pip check if requested and checks for mismatches
+                sanity_pip_check = False
+                msg = "Sanity 'pip check' disabled for {self.name} extension, "
+                msg += "assuming that parent will take care of it"
+                self.log.info(msg)
+
+        if sanity_pip_check:
+            if not self.is_extension:
+                # for stand-alone Python package installations (not part of a bundle of extensions),
+                # the (fake or real) module file must be loaded at this point,
+                # otherwise the Python package being installed is not "in view",
+                # and we will overlook missing dependencies...
+                loaded_modules = [x['mod_name'] for x in self.modules_tool.list()]
+                if self.short_mod_name not in loaded_modules:
+                    self.log.debug("Currently loaded modules: %s", loaded_modules)
+                    raise EasyBuildError("%s module is not loaded, this should never happen...",
+                                         self.short_mod_name)
+
+            unversioned_packages = self.cfg.get('unversioned_packages', [])
+            run_pip_check(python_cmd=python_cmd, unversioned_packages=unversioned_packages)
 
         # ExtensionEasyBlock handles loading modules correctly for multi_deps, so we clean up fake_mod_data
         # and let ExtensionEasyBlock do its job

test/easyblocks/easyblock_specific.py

@@ -29,6 +29,7 @@
 """
 import copy
 import os
+import re
 import stat
 import sys
 import tempfile
@@ -266,6 +267,29 @@ def test_det_cmake_version(self):
         """))
         self.assertEqual(det_cmake_version(), '1.2.3-rc4')
 
+    def test_det_installed_python_packages(self):
+        """
+        Test det_installed_python_packages function providyed by PythonPackage easyblock
+        """
+        pkg1 = None
+        # we can't make too much assumptions on which installed Python packages are found
+        res = pythonpackage.det_installed_python_packages(python_cmd=sys.executable)
+        self.assertTrue(isinstance(res, list))
+        if res:
+            pkg1_name = res[0]
+            self.assertTrue(isinstance(pkg1_name, str))
+
+        res_detailed = pythonpackage.det_installed_python_packages(python_cmd=sys.executable, names_only=False)
+        self.assertTrue(isinstance(res_detailed, list))
+        if res_detailed:
+            pkg1 = res_detailed[0]
+            self.assertTrue(isinstance(pkg1, dict))
+            self.assertTrue(sorted(pkg1.keys()), ['name', 'version'])
+            self.assertEqual(pkg1['name'], pkg1_name)
+            regex = re.compile('^[0-9].*')
+            ver = pkg1['version']
+            self.assertTrue(regex.match(ver), f"Pattern {regex.pattern} matches for pkg version: {ver}")
+
     def test_det_py_install_scheme(self):
         """Test det_py_install_scheme function provided by PythonPackage easyblock."""
         res = pythonpackage.det_py_install_scheme(sys.executable)