Honor sanity_check_commands & sanity_check_paths from extensions

easybuild/framework/easyblock.py

@@ -208,7 +208,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
 
@@ -670,12 +670,11 @@ def collect_exts_file_info(self, fetch_files=True, verify_checksums=True):
                         'name': ext_name,
                         'version': ext_version,
                         'options': ext_options,
+                        # 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)
@@ -1810,7 +1809,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
         """
@@ -2776,7 +2775,7 @@ def check_checksums_for(self, ent, sub='', source_cnt=None):
                 # if the filename is a dict, the actual source file name is the "filename" element
                 if isinstance(fn, dict):
                     fn = fn["filename"]
-                if fn in checksums_from_json.keys():
+                if fn in checksums_from_json:
                     checksums += [checksums_from_json[fn]]
 
         if source_cnt is None:
@@ -2967,7 +2966,8 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True):
             if os.path.isabs(self.rpath_wrappers_dir):
                 _log.info(f"Using {self.rpath_wrappers_dir} to store/use RPATH wrappers")
             else:
-                raise EasyBuildError(f"Path used for rpath_wrappers_dir is not an absolute path: {path}")
+                raise EasyBuildError("Path used for rpath_wrappers_dir is not an absolute path: %s",
+                                     self.rpath_wrappers_dir)
 
         if self.iter_idx > 0:
             # reset toolchain for iterative runs before preparing it again
@@ -3363,6 +3363,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:
@@ -4079,8 +4089,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
@@ -4100,9 +4112,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
@@ -4259,7 +4273,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.
 
@@ -4325,7 +4339,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
@@ -4354,26 +4368,27 @@ 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:
                 self._sanity_check_step_extensions()
 
-        linked_shared_lib_fails = self.sanity_check_linked_shared_libs()
-        if linked_shared_lib_fails:
-            self.log.warning("Check for required/banned linked shared libraries failed!")
-            self.sanity_check_fail_msgs.append(linked_shared_lib_fails)
-
-        # software installed with GCCcore toolchain should not have Fortran module files (.mod),
-        # unless that's explicitly allowed
-        if self.toolchain.name in ('GCCcore',) and not self.cfg['skip_mod_files_sanity_check']:
-            mod_files_found_msg = self.sanity_check_mod_files()
-            if mod_files_found_msg:
-                if build_option('fail_on_mod_files_gcccore'):
-                    self.sanity_check_fail_msgs.append(mod_files_found_msg)
-                else:
-                    print_warning(mod_files_found_msg)
+            # Do not do those checks for extensions, only in the main easyconfig
+            linked_shared_lib_fails = self.sanity_check_linked_shared_libs()
+            if linked_shared_lib_fails:
+                self.log.warning("Check for required/banned linked shared libraries failed!")
+                self.sanity_check_fail_msgs.append(linked_shared_lib_fails)
+
+            # software installed with GCCcore toolchain should not have Fortran module files (.mod),
+            # unless that's explicitly allowed
+            if self.toolchain.name in ('GCCcore',) and not self.cfg['skip_mod_files_sanity_check']:
+                mod_files_found_msg = self.sanity_check_mod_files()
+                if mod_files_found_msg:
+                    if build_option('fail_on_mod_files_gcccore'):
+                        self.sanity_check_fail_msgs.append(mod_files_found_msg)
+                    else:
+                        print_warning(mod_files_found_msg)
 
         # cleanup
         if self.fake_mod_data:
@@ -4403,14 +4418,14 @@ def xs2str(xs):
             self.log.debug("Skipping CUDA sanity check: CUDA is not in dependencies")
 
         # pass or fail
-        if self.sanity_check_fail_msgs:
+        if not self.sanity_check_fail_msgs:
+            self.log.debug("Sanity check passed!")
+        elif not self.is_extension:
             raise EasyBuildError(
                 "Sanity check failed: " + '\n'.join(self.sanity_check_fail_msgs),
                 exit_code=EasyBuildExit.FAIL_SANITY_CHECK,
             )
 
-        self.log.debug("Sanity check passed!")
-
     def _set_module_as_default(self, fake=False):
         """
         Sets the default module version except if we are in dry run

easybuild/framework/extension.py

@@ -294,8 +294,6 @@ def sanity_check_step(self):
         """
         Sanity check to run after installing extension
         """
-        res = (True, '')
-
         if os.path.isdir(self.installdir):
             change_dir(self.installdir)
 
@@ -330,9 +328,6 @@ def sanity_check_step(self):
                     fail_msg = 'command "%s" failed' % cmd
                 fail_msg += "; output:\n%s" % cmd_res.output.strip()
                 self.log.warning("Sanity check for '%s' extension failed: %s", self.name, fail_msg)
-                res = (False, fail_msg)
                 # keep track of all reasons of failure
                 # (only relevant when this extension is installed stand-alone via ExtensionEasyBlock)
                 self.sanity_check_fail_msgs.append(fail_msg)
-
-        return res

easybuild/framework/extensioneasyblock.py

@@ -72,8 +72,6 @@ def extra_options(extra_vars=None):
     def __init__(self, *args, **kwargs):
         """Initialize either as EasyBlock or as Extension."""
 
-        self.is_extension = False
-
         if isinstance(args[0], EasyBlock):
             # make sure that extra custom easyconfig parameters are known
             extra_params = self.__class__.extra_options()
@@ -187,20 +185,20 @@ def sanity_check_step(self, exts_filter=None, custom_paths=None, custom_commands
                         self.log.info(info_msg)
                         trace_msg(info_msg)
                     # perform sanity check for stand-alone extension
-                    (sanity_check_ok, fail_msg) = Extension.sanity_check_step(self)
+                    Extension.sanity_check_step(self)
         else:
             # perform single sanity check for extension
-            (sanity_check_ok, fail_msg) = Extension.sanity_check_step(self)
+            Extension.sanity_check_step(self)
 
-        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)
+        super().sanity_check_step(custom_paths=custom_paths,
+                                  custom_commands=custom_commands)
 
         # pass or fail sanity check
-        if sanity_check_ok:
+        if not self.sanity_check_fail_msgs:
+            sanity_check_ok = True
             self.log.info("Sanity check for %s successful!", self.name)
         else:
+            sanity_check_ok = False
             if not self.is_extension:
                 msg = "Sanity check for %s failed: %s" % (self.name, '; '.join(self.sanity_check_fail_msgs))
                 raise EasyBuildError(msg)

test/framework/easyblock.py

@@ -2596,7 +2596,6 @@ def test_extensions_sanity_check(self):
             exts_list = toy_ec['exts_list']
             exts_list[-1][2]['exts_filter'] = ("thisshouldfail", '')
             toy_ec['exts_list'] = exts_list
-            toy_ec['exts_defaultclass'] = 'DummyExtension'
 
         eb = EB_toy(toy_ec)
         eb.silent = True
@@ -2611,12 +2610,58 @@ def test_extensions_sanity_check(self):
         # sanity check commands are checked after checking sanity check paths, so this should work
         toy_ec = EasyConfig(toy_ec_fn)
         toy_ec.update('sanity_check_commands', [("%(installdir)s/bin/toy && rm %(installdir)s/bin/toy", '')])
-        toy_ec['exts_defaultclass'] = 'DummyExtension'
         eb = EB_toy(toy_ec)
         eb.silent = True
         with self.mocked_stdout_stderr(), self.saved_env():
             eb.run_all_steps(True)
 
+        # Verify custom paths and commands of extensions are checked
+        toy_ec_fn = os.path.join(test_ecs_dir, 't', 'toy', 'toy-0.0.eb')
+        toy_ec_txt = read_file(toy_ec_fn)
+
+        self.contents = toy_ec_txt + textwrap.dedent("""
+            exts_defaultclass = 'DummyExtension'
+            exts_filter = ('true', '')
+            exts_list = [
+                ('bar', '0.0', {
+                    'sanity_check_commands': ['echo "%(name)s extension output"'],
+                    'sanity_check_paths': {'dirs': ['.'], 'files': ['any_file']},
+                }),
+                ('barbar', '0.0', {'sanity_check_commands': ['echo "%(name)s extension output"']}),
+            ]
+            sanity_check_commands = ['echo "%(name)s output"']
+            sanity_check_paths = {
+                'dirs': ['.'],
+                'files': [],
+            }
+        """)
+
+        self.writeEC()
+        eb = EB_toy(EasyConfig(self.eb_file))
+        eb.silent = True
+        write_file(os.path.join(eb.installdir, 'any_file'), '')
+        with self.mocked_stdout_stderr(), self.saved_env():
+            eb.sanity_check_step()
+        logtxt = read_file(eb.logfile)
+        self.assertRegex(logtxt, 'Running .*command.*echo "toy output"')
+        self.assertRegex(logtxt, 'Running .*command.*echo "bar extension output"')
+        self.assertRegex(logtxt, 'Using .*sanity check paths .*any_file')
+        self.assertRegex(logtxt, 'Running .*command.*echo "barbar extension output"')
+        self.assertRegex(logtxt, 'Running .*command.*echo "barbar extension output"')
+        # Only do this once, not for every extension which would be redundant
+        self.assertEqual(logtxt.count('Checking for banned/required linked shared libraries'), 1)
+
+        # Verify that sanity_check_paths are actually verified
+        self.contents += "\nexts_list[-1][2]['sanity_check_paths'] = {'dirs': [], 'files': ['nosuchfile']}"
+        self.writeEC()
+        eb = EB_toy(EasyConfig(self.eb_file))
+        eb.silent = True
+        write_file(os.path.join(eb.installdir, 'any_file'), '')
+        with self.mocked_stdout_stderr():
+            self.assertRaisesRegex(EasyBuildError,
+                                   "extensions sanity check failed for 1 extensions: barbar.*nosuchfile",
+                                   eb.sanity_check_step)
+
     def test_parallel(self):
         """Test defining of parallelism."""
         topdir = os.path.abspath(os.path.dirname(__file__))

test/framework/easyconfig.py

@@ -1490,7 +1490,6 @@ def test_start_dir_template(self):
             preconfigopts = 'echo start_dir in configure is %(start_dir)s && '
             prebuildopts = 'echo start_dir in build is %(start_dir)s && '
 
-            exts_defaultclass = 'EB_Toy'
             exts_list = [
                ('bar', '0.0', {
                    'sources': ['bar-0.0-local.tar.gz'],

test/framework/options.py

@@ -6598,7 +6598,6 @@ def test_sanity_check_only(self):
         test_ec_txt += '\n' + '\n'.join([
             "sanity_check_commands = ['barbar', 'toy']",
             "sanity_check_paths = {'files': ['bin/barbar', 'bin/toy'], 'dirs': ['bin']}",
-            "exts_defaultclass = 'DummyExtension'",
             "exts_list = [",
             "    ('barbar', '0.0', {",
             "        'start_dir': 'src',",

test/framework/sandbox/easybuild/easyblocks/t/toy.py

@@ -81,9 +81,10 @@ def prepare_for_extensions(self):
         """
         Prepare for installing toy extensions.
         """
-        # insert new packages by building them with RPackage
-        self.cfg['exts_defaultclass'] = "Toy_Extension"
-        self.cfg['exts_filter'] = ("%(ext_name)s", "")
+        if not self.cfg.get('exts_defaultclass', resolve=False):
+            self.cfg['exts_defaultclass'] = "Toy_Extension"
+        if not self.cfg.get('exts_filter', resolve=False):
+            self.cfg['exts_filter'] = ("%(ext_name)s", "")
 
     def run_all_steps(self, *args, **kwargs):
         """