Remove redundant work in sanity check of extensions

easybuild/framework/easyblock.py

@@ -85,7 +85,7 @@
 from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, dry_run_msg, dry_run_warning, dry_run_set_dirs
 from easybuild.tools.build_log import print_error_and_exit, print_msg, print_warning
 from easybuild.tools.config import CHECKSUM_PRIORITY_JSON, DEFAULT_ENVVAR_USERS_MODULES
-from easybuild.tools.config import EASYBUILD_SOURCES_URL, EBPYTHONPREFIXES  # noqa
+from easybuild.tools.config import EASYBUILD_SOURCES_URL, EBPYTHONPREFIXES  # noqa # pylint:disable=unused-import
 from easybuild.tools.config import FORCE_DOWNLOAD_ALL, FORCE_DOWNLOAD_PATCHES, FORCE_DOWNLOAD_SOURCES
 from easybuild.tools.config import MOD_SEARCH_PATH_HEADERS, PYTHONPATH, SEARCH_PATH_BIN_DIRS, SEARCH_PATH_LIB_DIRS
 from easybuild.tools.config import build_option, build_path, get_failed_install_build_dirs_path
@@ -219,7 +219,7 @@ def __init__(self, ec, logfile=None):
         self.skip = None
         self.module_extra_extensions = ''  # extra stuff for module file required by extensions
 
-        # indicates whether or not this instance represents an extension or not;
+        # indicates whether or not this instance represents an extension
         # may be set to True by ExtensionEasyBlock
         self.is_extension = False
 
@@ -685,12 +685,11 @@ def collect_exts_file_info(self, fetch_files=True, verify_checksums=True):
                         'version': ext_version,
                         'options': ext_options,
                         'github_account': ext_options.get('github_account', orig_github_account),
+                        # if a particular easyblock is specified, make sure it's used
+                        # (this is picked up by init_ext_instances)
+                        'easyblock': ext_options.get('easyblock', None),
                     }
 
-                    # if a particular easyblock is specified, make sure it's used
-                    # (this is picked up by init_ext_instances)
-                    ext_src['easyblock'] = ext_options.get('easyblock', None)
-
                     # construct dictionary with template values;
                     # inherited from parent, except for name/version templates which are specific to this extension
                     template_values = copy.deepcopy(self.cfg.template_values)
@@ -1144,14 +1143,14 @@ def obtain_file_raise_on_failure(self, filename, extension=False, urls=None, dow
     @property
     def name(self):
         """
-        Shortcut the get the module name.
+        Shortcut to get the module name.
         """
         return self.cfg['name']
 
     @property
     def version(self):
         """
-        Shortcut the get the module version.
+        Shortcut to get the module version.
         """
         return self.cfg['version']
 
@@ -1865,7 +1864,7 @@ def inject_module_extra_paths(self):
                 msg += f"and paths='{env_var}'"
                 self.log.debug(msg)
 
-    def expand_module_search_path(self, search_path, path_type=ModEnvVarType.PATH_WITH_FILES):
+    def expand_module_search_path(self, *_, **__):
         """
         REMOVED in EasyBuild 5.1, use EasyBlock.module_load_environment.expand_paths instead
         """
@@ -2409,6 +2408,31 @@ def fake_module_environment(self, extra_modules=None, with_build_deps=False):
             if fake_mod_data:
                 self.clean_up_fake_module(fake_mod_data)
 
+    @contextmanager
+    def sanity_check_module_environment(self, extra_modules=None, with_build_deps=False, check_loaded=True):
+        """Load/Unload module for performing sanity checks"""
+        if self.sanity_check_module_loaded and check_loaded:
+            raise EasyBuildError("Sanity check module is already loaded and must not be loaded again")
+
+        if self.sanity_check_module_loaded:
+            unload_module = False
+        else:
+            if with_build_deps:
+                # load modules for build dependencies as extra modules
+                extra_modules = [dep['short_mod_name'] for dep in self.cfg.dependencies(build_only=True)]
+            self.sanity_check_load_module(extension=self.is_extension, extra_modules=extra_modules)
+            unload_module = True
+
+        try:
+            yield
+        finally:
+            # cleanup (unload fake module, remove fake module dir)
+            if unload_module:
+                if self.fake_mod_data:
+                    self.clean_up_fake_module(self.fake_mod_data)
+                    self.fake_mod_data = None
+                self.sanity_check_module_loaded = False
+
     def guess_start_dir(self):
         """
         Return the directory where to start the whole configure/make/make install cycle from
@@ -3444,6 +3468,16 @@ def post_processing_step(self):
 
     def _dispatch_sanity_check_step(self, *args, **kwargs):
         """Decide whether to run the dry-run or the real version of the sanity-check step"""
+        if 'extension' in kwargs:
+            extension = kwargs.pop('extension')
+            self.log.deprecated(
+                "Passing `extension` to `sanity_check_step` is no longer necessary and will be ignored "
+                f"(Easyblock: {self.__class__.__name__}).",
+                '6.0',
+            )
+            if extension != self.is_extension:
+                raise EasyBuildError('Unexpected value for `extension` argument. '
+                                     f'Should be: {self.is_extension}, got:  {extension}')
         if self.dry_run:
             self._sanity_check_step_dry_run(*args, **kwargs)
         else:
@@ -4160,8 +4194,10 @@ def _sanity_check_step_common(self, custom_paths, custom_commands):
                 paths = {}
                 for key in path_keys_and_check:
                     paths.setdefault(key, [])
-                paths.update({SANITY_CHECK_PATHS_DIRS: ['bin', ('lib', 'lib64')]})
-                self.log.info("Using default sanity check paths: %s", paths)
+                # Default paths for extensions are handled in the parent easyconfig if desired
+                if not self.is_extension:
+                    paths.update({SANITY_CHECK_PATHS_DIRS: ['bin', ('lib', 'lib64')]})
+                    self.log.info("Using default sanity check paths: %s", paths)
 
             # if enhance_sanity_check is enabled *and* sanity_check_paths are specified in the easyconfig,
             # those paths are used to enhance the paths provided by the easyblock
@@ -4181,9 +4217,11 @@ def _sanity_check_step_common(self, custom_paths, custom_commands):
         # verify sanity_check_paths value: only known keys, correct value types, at least one non-empty value
         only_list_values = all(isinstance(x, list) for x in paths.values())
         only_empty_lists = all(not x for x in paths.values())
-        if sorted_keys != known_keys or not only_list_values or only_empty_lists:
+        if sorted_keys != known_keys or not only_list_values or (only_empty_lists and not self.is_extension):
             error_msg = "Incorrect format for sanity_check_paths: should (only) have %s keys, "
-            error_msg += "values should be lists (at least one non-empty)."
+            error_msg += "values should be lists"
+            if not self.is_extension:
+                error_msg += " (at least one non-empty)."
             raise EasyBuildError(error_msg % ', '.join("'%s'" % k for k in known_keys))
 
         # Resolve arch specific entries
@@ -4316,16 +4354,20 @@ def sanity_check_load_module(self, extension=False, extra_modules=None):
         """
         Load module to prepare environment for sanity check
         """
+        if extension != self.is_extension:
+            raise RuntimeError(f"{self} {self.name} {extension}!={self.is_extension}")
+
+        if self.is_extension:
+            return self.fake_mod_data
 
         # skip loading of fake module when using --sanity-check-only, load real module instead
-        if build_option('sanity_check_only') and not extension:
+        if build_option('sanity_check_only'):
             self.log.info("Loading real module for %s %s: %s", self.name, self.version, self.short_mod_name)
             self.load_module(extra_modules=extra_modules)
             self.sanity_check_module_loaded = True
 
         # only load fake module for non-extensions, and not during dry run
-        elif not (extension or self.dry_run):
-
+        elif not self.dry_run:
             if extra_modules:
                 self.log.info("Loading extra modules for sanity check: %s", ', '.join(extra_modules))
 
@@ -4340,7 +4382,7 @@ def sanity_check_load_module(self, extension=False, extra_modules=None):
 
         return self.fake_mod_data
 
-    def _sanity_check_step(self, custom_paths=None, custom_commands=None, extension=False, extra_modules=None):
+    def _sanity_check_step(self, custom_paths=None, custom_commands=None, extra_modules=None):
         """
         Real version of sanity_check_step method.
 
@@ -4349,6 +4391,11 @@ def _sanity_check_step(self, custom_paths=None, custom_commands=None, extension=
         :param extension: indicates whether or not sanity check is run for an extension
         :param extra_modules: extra modules to load before running sanity check commands
         """
+        if extra_modules is not None and not isinstance(extra_modules, list):
+            if isinstance(extra_modules, bool):
+                self.log.nosupport("Do not pass `extension` to `_sanity_check_step`", '5.3.1')
+            raise EasyBuildError("extra_modules should be a list of module names, got %s (type: %s)",
+                                 extra_modules, type(extra_modules))
         paths, path_keys_and_check, commands = self._sanity_check_step_common(custom_paths, custom_commands)
 
         # helper function to sanity check (alternatives for) one particular path
@@ -4406,7 +4453,7 @@ def xs2str(xs):
                 trace_msg("%s %s found: %s" % (typ, xs2str(xs), ('FAILED', 'OK')[found]))
 
         if not self.sanity_check_module_loaded:
-            self.sanity_check_load_module(extension=extension, extra_modules=extra_modules)
+            self.sanity_check_load_module(extension=self.is_extension, extra_modules=extra_modules)
 
         # allow oversubscription of P processes on C cores (P>C) for software installed on top of Open MPI;
         # this is useful to avoid failing of sanity check commands that involve MPI
@@ -4435,7 +4482,7 @@ def xs2str(xs):
             trace_msg(f"result for command '{cmd}': {cmd_result_str}")
 
         # also run sanity check for extensions (unless we are an extension ourselves)
-        if not extension:
+        if not self.is_extension:
             if build_option('skip_extensions'):
                 self.log.info("Skipping sanity check for extensions since skip-extensions is enabled...")
             else:

easybuild/framework/extensioneasyblock.py

@@ -181,7 +181,7 @@ def sanity_check_step(self, exts_filter=None, custom_paths=None, custom_commands
         # take into account that module may already be loaded earlier in sanity check
         if not (self.sanity_check_module_loaded or self.is_extension or self.dry_run):
             for extra_modules in lists_of_extra_modules:
-                with self.fake_module_environment(extra_modules=extra_modules):
+                with self.sanity_check_module_environment(extra_modules=extra_modules):
                     if extra_modules:
                         info_msg = f"Running extension sanity check with extra modules: {', '.join(extra_modules)}"
                         self.log.info(info_msg)
@@ -194,8 +194,7 @@ def sanity_check_step(self, exts_filter=None, custom_paths=None, custom_commands
 
         if custom_paths or custom_commands or not self.is_extension:
             super().sanity_check_step(custom_paths=custom_paths,
-                                      custom_commands=custom_commands,
-                                      extension=self.is_extension)
+                                      custom_commands=custom_commands)
 
         # pass or fail sanity check
         if sanity_check_ok:

test/framework/sandbox/easybuild/easyblocks/generic/toy_extension.py

@@ -44,6 +44,8 @@ def extra_options():
         """Custom easyconfig parameters for toy extensions."""
         extra_vars = {
             'toy_ext_param': ['', "Toy extension parameter", CUSTOM],
+            'toy_custom_cmd': [None, "Custom command to run in sanity check", CUSTOM],
+            'use_custom_sanity_check_paths': [True, "Add default paths to check for in sanity check", CUSTOM],
         }
         return ExtensionEasyBlock.extra_options(extra_vars=extra_vars)
 
@@ -106,10 +108,18 @@ def post_install_extension(self):
     def sanity_check_step(self, *args, **kwargs):
         """Custom sanity check for toy extensions."""
         self.log.info("Loaded modules: %s", self.modules_tool.list())
-        custom_paths = {
-            'files': [],
-            'dirs': ['.'],  # minor hack to make sure there's always a non-empty list
-        }
-        if self.src:
-            custom_paths['files'].extend(['bin/%s' % self.name, 'lib/lib%s.a' % self.name])
-        return super().sanity_check_step(custom_paths=custom_paths)
+        if self.cfg['use_custom_sanity_check_paths']:
+            custom_paths = {
+                'files': [],
+                'dirs': ['.'],  # minor hack to make sure there's always a non-empty list
+            }
+            if self.src:
+                custom_paths['files'].extend(['bin/%s' % self.name, 'lib/lib%s.a' % self.name])
+        else:
+            custom_paths = None
+        custom_cmd = self.cfg['toy_custom_cmd']
+        if custom_cmd:
+            custom_cmds = [custom_cmd]
+        else:
+            custom_cmds = None
+        return super().sanity_check_step(custom_paths=custom_paths, custom_commands=custom_cmds)

test/framework/toy_build.py

@@ -2494,6 +2494,37 @@ def test_toy_sanity_check_commands(self):
         regex = re.compile('^.*/eb-[^/]+/eb-sanity-check-[^/]+\n[ ]*0$')
         self.assertTrue(regex.match(out_txt), f"Pattern '{regex.pattern}' should match in: {out_txt}")
 
+    def test_extension_sanity_check_custom_commands(self):
+        """Test whether fallback in sanity check for lib64/ equivalents of library files works."""
+        test_ec_txt = TOY_EC_TXT
+        test_ec_txt += '\n' + textwrap.dedent("""
+            exts_list = [
+                ('barbar', '0.0', {
+                    'exts_filter': ('ls -l lib/lib%(ext_name)s.a', ''),
+                    'use_custom_sanity_check_paths': False,
+                    'toy_custom_cmd': 'echo "Run-Custom-Cmd for %(name)s" && false',
+                    #'sanity_check_paths': {'dirs': [], 'files': []},
+                })
+            ]
+        """)
+        test_ec = os.path.join(self.test_prefix, 'test.eb')
+        write_file(test_ec, test_ec_txt)
+        error_pattern = 'sanity check command echo "Run-Custom-Cmd for barbar" && false failed with exit code 1'
+        with self.mocked_stdout_stderr(), self.log_to_testlogfile() as logfile:
+            self.assertErrorRegex(EasyBuildError, error_pattern, self._test_toy_build, ec_file=test_ec,
+                                  raise_error=True, verbose=False)
+            logtxt = read_file(logfile)
+        check_bin_msg = 'Sanity check: found (non-empty) directory bin'
+        self.assertEqual(logtxt.count(check_bin_msg), 1, "Check for 'bin' folder should only be done once")
+
+        test_ec_txt += "\nexts_list[0][2]['toy_custom_cmd'] = 'echo \"Run-Custom-Cmd for %(name)s\" && true'"
+        write_file(test_ec, test_ec_txt)
+        with self.mocked_stdout_stderr(), self.log_to_testlogfile() as logfile:
+            self._test_toy_build(ec_file=test_ec, raise_error=True)
+            logtxt = read_file(logfile)
+        self.assertRegex(logtxt, 'sanity check command .*Run-Custom-Cmd for barbar.*ran successfully',)
+        self.assertEqual(logtxt.count(check_bin_msg), 1, "Check for 'bin' folder should only be done once")
+
     def test_sanity_check_paths_lib64(self):
         """Test whether fallback in sanity check for lib64/ equivalents of library files works."""
         # modify test easyconfig: move lib/libtoy.a to lib64/libtoy.a
@@ -2712,6 +2743,7 @@ def test_toy_build_enhanced_sanity_check(self):
 
         pattern_lines = [
             r"^== sanity checking\.\.\.",
+            r"  >> loading modules: toy/0.0...",
             r"  >> file 'bin/toy' found: OK",
         ]
         regex = re.compile(r'\n'.join(pattern_lines), re.M)
@@ -3544,9 +3576,10 @@ def test_toy_build_trace(self):
             r"  >> command completed: exit 0, ran in .*",
             r'^' + r'\n'.join([
                 r"== sanity checking\.\.\.",
+                r"  >> loading modules: toy/0.0...",
                 r"  >> file 'bin/yot' or 'bin/toy' found: OK",
                 r"  >> \(non-empty\) directory 'bin' found: OK",
-                r"  >> loading modules: toy/0.0\.\.\.",
+                r"  >> loading modules: toy/0.0...",
                 r"  >> running command 'toy' \.\.\.",
                 r"  >> result for command 'toy': OK",
             ]) + r'$',