diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 55107be6f9..a3d423663b 100644 --- a/easybuild/framework/easyblock.py +++ b/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 diff --git a/easybuild/framework/extension.py b/easybuild/framework/extension.py index 22f2b9f42e..d0497333b9 100644 --- a/easybuild/framework/extension.py +++ b/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 diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index 9fb7f1563c..f778204063 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/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) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index c818dd7187..9e83137bea 100644 --- a/test/framework/easyblock.py +++ b/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__)) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 9dddc29f46..88133d1ff4 100644 --- a/test/framework/easyconfig.py +++ b/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'], diff --git a/test/framework/options.py b/test/framework/options.py index 41ceb0b106..b4280023f2 100644 --- a/test/framework/options.py +++ b/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',", diff --git a/test/framework/sandbox/easybuild/easyblocks/t/toy.py b/test/framework/sandbox/easybuild/easyblocks/t/toy.py index da631b2390..393598e9d9 100644 --- a/test/framework/sandbox/easybuild/easyblocks/t/toy.py +++ b/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): """ diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index 9e110ba09b..91864784a6 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -1355,7 +1355,6 @@ def test_toy_extension_patches_postinstallcmds(self): test_ec = os.path.join(self.test_prefix, 'test.eb') test_ec_txt = f"{toy_ec_txt}\n" + textwrap.dedent(""" - exts_defaultclass = "DummyExtension" exts_list = [ ("bar", "0.0", { "buildopts": " && ls -l test.txt", @@ -1421,7 +1420,6 @@ def test_toy_extension_sources(self): # test use of single-element list in 'sources' with just the filename test_ec_txt = '\n'.join([ toy_ec_txt, - 'exts_defaultclass = "DummyExtension"', 'exts_list = [', ' ("bar", "0.0", {', ' "sources": %s,' % bar_sources_spec, @@ -1449,7 +1447,6 @@ def test_toy_extension_sources(self): test_ec_txt = '\n'.join([ toy_ec_txt, - 'exts_defaultclass = "DummyExtension"', 'exts_list = [', ' ("bar", "0.0", {', ' "source_urls": ["file://%s"],' % test_source_path, @@ -1465,7 +1462,6 @@ def test_toy_extension_sources(self): # check that checksums are picked up and verified test_ec_txt = '\n'.join([ toy_ec_txt, - 'exts_defaultclass = "DummyExtension"', 'exts_list = [', ' ("bar", "0.0", {', ' "source_urls": ["file://%s"],' % test_source_path, @@ -1489,7 +1485,6 @@ def test_toy_extension_sources(self): # test again with correct checksum for bar-0.0.tar.gz, but faulty checksum for patch file test_ec_txt = '\n'.join([ toy_ec_txt, - 'exts_defaultclass = "DummyExtension"', 'exts_list = [', ' ("bar", "0.0", {', ' "source_urls": ["file://%s"],' % test_source_path, @@ -1513,7 +1508,6 @@ def test_toy_extension_sources(self): # test again with correct checksums test_ec_txt = '\n'.join([ toy_ec_txt, - 'exts_defaultclass = "DummyExtension"', 'exts_list = [', ' ("bar", "0.0", {', ' "source_urls": ["file://%s"],' % test_source_path, @@ -1539,7 +1533,6 @@ def test_toy_extension_extract_cmd(self): test_ec = os.path.join(self.test_prefix, 'test.eb') test_ec_txt = '\n'.join([ toy_ec_txt, - 'exts_defaultclass = "DummyExtension"', 'exts_list = [', ' ("bar", "0.0", {', # deliberately incorrect custom extract command, just to verify that it's picked up @@ -1578,7 +1571,6 @@ def test_toy_extension_sources_git_config(self): test_ec_txt = '\n'.join([ toy_ec_txt, 'prebuildopts = "echo \\\"%s\\\" > %s && ",' % (ext_code, ext_cfile), - 'exts_defaultclass = "DummyExtension"', 'exts_list = [', ' ("exts-git", "0.0", {', ' "buildopts": "&& ls -l %s %s",' % (ext_tarball, ext_tarfile), @@ -1941,7 +1933,6 @@ def test_module_only_extensions(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',", @@ -1978,7 +1969,7 @@ def test_module_only_extensions(self): move_file(libbarbar, libbarbar + '.foobar') # check whether sanity check fails now when using --module-only - error_pattern = 'Sanity check failed: command "ls -l lib/libbarbar.a" failed' + error_pattern = 'Sanity check failed: .*command "ls -l lib/libbarbar.a" failed' for extra_args in (['--module-only'], ['--module-only', '--rebuild']): with self.mocked_stdout_stderr(): self.assertErrorRegex(EasyBuildError, error_pattern, self.eb_main, [test_ec] + extra_args, @@ -2011,7 +2002,6 @@ def test_toy_exts_parallel(self): test_ec = os.path.join(self.test_prefix, 'test.eb') test_ec_txt = read_file(toy_ec) test_ec_txt += '\n' + '\n'.join([ - "exts_defaultclass = 'DummyExtension'", "exts_list = [", " ('ls'),", " ('bar', '0.0'),", @@ -2392,7 +2382,6 @@ def test_reproducibility_ext_easyblocks(self): ec1 = os.path.join(self.test_prefix, 'toy1.eb') ec1_txt = '\n'.join([ toy_ec_txt, - "exts_defaultclass = 'DummyExtension'", "exts_list = [('barbar', '1.2', {'start_dir': 'src'})]", "", ]) @@ -3642,7 +3631,6 @@ def test_toy_build_hooks(self): test_ec = os.path.join(self.test_prefix, 'test.eb') test_ec_txt = read_file(toy_ec) + '\n'.join([ "exts_list = [('bar', '0.0'), ('toy', '0.0')]", - "exts_defaultclass = 'DummyExtension'", ]) write_file(test_ec, test_ec_txt) @@ -3806,7 +3794,6 @@ def test_toy_multi_deps(self): test_ec = os.path.join(self.test_prefix, 'test.eb') # also inject (minimal) list of extensions to test iterative installation of extensions - test_ec_txt += "\nexts_defaultclass = 'DummyExtension'" test_ec_txt += "\nexts_list = [('barbar', '1.2', {'start_dir': 'src'})]" test_ec_txt += "\nmulti_deps = {'GCC': ['4.6.3', '7.3.0-2.30']}"