From 24e657c44dfb9d16bb73e1958ca4e28890fa8f5d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 5 Dec 2024 13:30:16 +0100 Subject: [PATCH 1/3] add support for multiple modulenames in extensions This changes the allowed types of the `modulename` extension option: - `False` to skip the sanity check and always install it when `--skip` is used - `str`: Value for `%(ext_name)s` in the `exts_filter` template - List of `str`: Multiple names to be used in the `exts_filter` template. All resulting commands must succeed. It replaces `resolve_exts_filter_template` by `construct_exts_filter_cmds` as the method now returns a, potentially empty, list which might cause errors if used without expecting a list. --- easybuild/framework/easyblock.py | 47 +++++++++++------ easybuild/framework/extension.py | 89 ++++++++++++++++++-------------- test/framework/easyconfig.py | 26 ++++++---- 3 files changed, 95 insertions(+), 67 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 9ee91b48d6..36f81535ad 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -67,7 +67,7 @@ from easybuild.framework.easyconfig.style import MAX_LINE_LENGTH from easybuild.framework.easyconfig.tools import dump_env_easyblock, get_paths_for from easybuild.framework.easyconfig.templates import TEMPLATE_NAMES_EASYBLOCK_RUN_STEP, template_constant_dict -from easybuild.framework.extension import Extension, resolve_exts_filter_template +from easybuild.framework.extension import Extension, construct_exts_filter_cmds from easybuild.tools import LooseVersion, config from easybuild.tools.build_details import get_build_stats from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, dry_run_msg, dry_run_warning, dry_run_set_dirs @@ -1872,10 +1872,15 @@ def skip_extensions_sequential(self, exts_filter): exts = [] for idx, ext_inst in enumerate(self.ext_instances): - cmd, stdin = resolve_exts_filter_template(exts_filter, ext_inst) - res = run_shell_cmd(cmd, stdin=stdin, fail_on_error=False, hidden=True) - self.log.info(f"exts_filter result for {ext_inst.name}: exit code {res.exit_code}; output: {res.output}") - if res.exit_code == EasyBuildExit.SUCCESS: + cmds = construct_exts_filter_cmds(exts_filter, ext_inst) or [] + for cmd, stdin in cmds: + res = run_shell_cmd(cmd, stdin=stdin, fail_on_error=False, hidden=True) + self.log.info(f"exts_filter result for {ext_inst.name}:cmd {cmd}; " + f"exit code {res.exit_code}; output: {res.output}") + if res.exit_code != EasyBuildExit.SUCCESS: + break + # Don't skip extension if there were no commands to check, e.g. modulename=False + if cmds and res.exit_code == EasyBuildExit.SUCCESS: print_msg(f"skipping extension {ext_inst.name}", silent=self.silent, log=self.log) else: self.log.info(f"Not skipping {ext_inst.name}") @@ -1893,37 +1898,45 @@ def skip_extensions_parallel(self, exts_filter): """ print_msg("skipping installed extensions (in parallel)", log=self.log) - installed_exts_ids = [] - checked_exts_cnt = 0 + cmds = [construct_exts_filter_cmds(exts_filter, ext) for ext in self.ext_instances] + # Consider extensions that don't need checking as checked + checked_exts_cnt = sum(0 if ext_cmds else 1 for ext_cmds in cmds) exts_cnt = len(self.ext_instances) - cmds = [resolve_exts_filter_template(exts_filter, ext) for ext in self.ext_instances] with ThreadPoolExecutor(max_workers=self.cfg['parallel']) as thread_pool: # list of command to run asynchronously async_cmds = [thread_pool.submit(run_shell_cmd, cmd, stdin=stdin, hidden=True, fail_on_error=False, - asynchronous=True, task_id=idx) for (idx, (cmd, stdin)) in enumerate(cmds)] + asynchronous=True, task_id=idx) + for (idx, ext_cmds) in enumerate(cmds) + for (cmd, stdin) in ext_cmds] + pending_cmds_per_ext = [len(ext_cmds) for ext_cmds in cmds] + installed_exts = [True] * exts_cnt # process result of commands as they have completed running for done_task in concurrent.futures.as_completed(async_cmds): res = done_task.result() idx = res.task_id ext_name = self.ext_instances[idx].name self.log.info(f"exts_filter result for {ext_name}: exit code {res.exit_code}; output: {res.output}") - if res.exit_code == EasyBuildExit.SUCCESS: - print_msg(f"skipping extension {ext_name}", log=self.log) - installed_exts_ids.append(idx) - checked_exts_cnt += 1 - exts_pbar_label = "skipping installed extensions " - exts_pbar_label += "(%d/%d checked)" % (checked_exts_cnt, exts_cnt) - self.update_exts_progress_bar(exts_pbar_label) + if res.exit_code != EasyBuildExit.SUCCESS: + installed_exts[idx] = False + + pending_cmds_per_ext[idx] -= 1 + if pending_cmds_per_ext[idx] == 0: + checked_exts_cnt += 1 + exts_pbar_label = "skipping installed extensions " + exts_pbar_label += "(%d/%d checked)" % (checked_exts_cnt, exts_cnt) + self.update_exts_progress_bar(exts_pbar_label) # compose new list of extensions, skip over the ones that are already installed; # note: original order in extensions list should be preserved! retained_ext_instances = [] for idx, ext in enumerate(self.ext_instances): - if idx not in installed_exts_ids: + if installed_exts[idx]: + print_msg(f"skipping extension {ext.name}", log=self.log) + else: retained_ext_instances.append(ext) self.log.info("Not skipping %s", ext.name) diff --git a/easybuild/framework/extension.py b/easybuild/framework/extension.py index 0005f24428..051f51a218 100644 --- a/easybuild/framework/extension.py +++ b/easybuild/framework/extension.py @@ -40,17 +40,17 @@ from easybuild.framework.easyconfig.easyconfig import resolve_template from easybuild.framework.easyconfig.templates import TEMPLATE_NAMES_EASYBLOCK_RUN_STEP, template_constant_dict -from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, raise_nosupport +from easybuild.tools.build_log import EasyBuildError, EasyBuildExit from easybuild.tools.filetools import change_dir from easybuild.tools.run import run_shell_cmd -def resolve_exts_filter_template(exts_filter, ext): +def construct_exts_filter_cmds(exts_filter, ext): """ Resolve the exts_filter tuple by replacing the template values using the extension :param exts_filter: Tuple of (command, input) using template values (ext_name, ext_version, src) :param ext: Instance of Extension or dictionary like with 'name' and optionally 'options', 'version', 'source' keys - :return: (cmd, input) as a tuple of strings + :return: (cmd, input) as a tuple of strings for each modulename. Might be empty if no filtering is intented """ if isinstance(exts_filter, str) or len(exts_filter) != 2: @@ -61,25 +61,35 @@ def resolve_exts_filter_template(exts_filter, ext): if not isinstance(ext, dict): ext = {'name': ext.name, 'version': ext.version, 'src': ext.src, 'options': ext.options} - name = ext['name'] - if 'options' in ext and 'modulename' in ext['options']: - modname = ext['options']['modulename'] - else: - modname = name - tmpldict = { - 'ext_name': modname, - 'ext_version': ext.get('version'), - 'src': ext.get('src'), - } - try: - cmd = cmd % tmpldict - cmdinput = cmdinput % tmpldict if cmdinput else None - except KeyError as err: - msg = "KeyError occurred on completing extension filter template: %s; " - msg += "'name'/'version' keys are no longer supported, should use 'ext_name'/'ext_version' instead" - raise_nosupport(msg % err, '2.0') - return cmd, cmdinput + modulenames = ext['options']['modulename'] + except KeyError: + modulenames = [ext['name']] + else: + if isinstance(modulenames, list): + if not modulenames: + raise EasyBuildError(f"Empty modulename list for {ext['name']} is not supported." + "Use `False` to skip checking the module!") + elif modulenames is False: + return [] # Skip any checks + elif not isinstance(modulenames, str): + raise EasyBuildError(f"Wrong type of modulename for {ext['name']}: {type(modulenames)}: {modulenames}") + else: + modulenames = [modulenames] + + result = [] + for modulename in modulenames: + tmpldict = { + 'ext_name': modulename, + 'ext_version': ext.get('version'), + 'src': ext.get('src'), + } + try: + result.append((cmd % tmpldict, + cmdinput % tmpldict if cmdinput else None)) + except KeyError as err: + raise EasyBuildError(f"KeyError occurred on completing extension filter template: {err}") + return result class Extension(object): @@ -287,31 +297,30 @@ def sanity_check_step(self): if exts_filter is None: self.log.debug("no exts_filter setting found, skipping sanitycheck") + return res - if 'modulename' in self.options: - modname = self.options['modulename'] - self.log.debug("modulename found in self.options, using it: %s", modname) - else: - modname = self.name - self.log.debug("self.name: %s", modname) - + exts_filer_cmds = construct_exts_filter_cmds(exts_filter, self) # allow skipping of sanity check by setting module name to False - if modname is False: + if exts_filer_cmds is None: self.log.info("modulename set to False for '%s' extension, so skipping sanity check", self.name) - elif exts_filter: - cmd, stdin = resolve_exts_filter_template(exts_filter, self) - cmd_res = run_shell_cmd(cmd, fail_on_error=False, stdin=stdin) - - if cmd_res.exit_code != EasyBuildExit.SUCCESS: - if stdin: - fail_msg = 'command "%s" (stdin: "%s") failed' % (cmd, stdin) - else: - fail_msg = 'command "%s" failed' % cmd - fail_msg += "; output:\n%s" % cmd_res.output.strip() + else: + fail_msgs = [] + for cmd, stdin in exts_filer_cmds: + cmd_res = run_shell_cmd(cmd, fail_on_error=False, stdin=stdin) + + if cmd_res.exit_code != EasyBuildExit.SUCCESS: + if stdin: + fail_msg = 'command "%s" (stdin: "%s") failed' % (cmd, stdin) + else: + fail_msg = 'command "%s" failed' % cmd + fail_msg += "; output:\n%s" % cmd_res.output.strip() + fail_msgs.append(fail_msg) + if fail_msgs: + fail_msg = '\n'.join(fail_msgs) 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) + res = (False, fail_msg) return res diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 1e630a5627..cf69283bef 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -62,7 +62,7 @@ from easybuild.framework.easyconfig.tools import dep_graph, det_copy_ec_specs, find_related_easyconfigs, get_paths_for from easybuild.framework.easyconfig.tools import parse_easyconfigs from easybuild.framework.easyconfig.tweak import obtain_ec_for, tweak, tweak_one -from easybuild.framework.extension import resolve_exts_filter_template +from easybuild.framework.extension import construct_exts_filter_cmds from easybuild.toolchains.system import SystemToolchain from easybuild.tools.build_log import EasyBuildError from easybuild.tools.config import build_option, get_module_syntax, module_classes, update_build_option @@ -4632,8 +4632,8 @@ def test_unexpected_version_keys_caught(self): self.assertRaises(EasyBuildError, EasyConfig, test_ec) - def test_resolve_exts_filter_template(self): - """Test for resolve_exts_filter_template function.""" + def test_construct_exts_filter_cmds(self): + """Test for construct_exts_filter_cmds function.""" class TestExtension(object): def __init__(self, values): self.name = values['name'] @@ -4642,11 +4642,11 @@ def __init__(self, values): self.options = values.get('options', {}) error_msg = 'exts_filter should be a list or tuple' - self.assertErrorRegex(EasyBuildError, error_msg, resolve_exts_filter_template, + self.assertErrorRegex(EasyBuildError, error_msg, construct_exts_filter_cmds, '[ 1 == 1 ]', {}) - self.assertErrorRegex(EasyBuildError, error_msg, resolve_exts_filter_template, + self.assertErrorRegex(EasyBuildError, error_msg, construct_exts_filter_cmds, ['[ 1 == 1 ]'], {}) - self.assertErrorRegex(EasyBuildError, error_msg, resolve_exts_filter_template, + self.assertErrorRegex(EasyBuildError, error_msg, construct_exts_filter_cmds, ['[ 1 == 1 ]', 'true', 'false'], {}) test_cases = [ @@ -4677,10 +4677,16 @@ def __init__(self, values): ), ] for exts_filter, ext, expected_value in test_cases: - value = resolve_exts_filter_template(exts_filter, ext) - self.assertEqual(value, expected_value) - value = resolve_exts_filter_template(exts_filter, TestExtension(ext)) - self.assertEqual(value, expected_value) + value = construct_exts_filter_cmds(exts_filter, ext) + self.assertEqual(value, [expected_value]) + value = construct_exts_filter_cmds(exts_filter, TestExtension(ext)) + self.assertEqual(value, [expected_value]) + + exts_filter = ('run %(ext_name)s', None) + value = construct_exts_filter_cmds(exts_filter, {'name': 'foo', 'options': {'modulename': False}}) + self.assertEqual(value, []) + value = construct_exts_filter_cmds(exts_filter, {'name': 'foo', 'options': {'modulename': ['name', 'alt']}}) + self.assertEqual(value, [('run name', None), ('run alt', None)]) def test_cuda_compute_capabilities(self): """Tests that the cuda_compute_capabilities templates are correct""" From a5f52474b6742d70bc33bda80eb2a523567cb13d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 5 Dec 2024 14:12:26 +0100 Subject: [PATCH 2/3] Enhance the skip-extension test to cover `modulename: False` There shouldn't be a command executed for that. --- test/framework/easyblock.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 2e4953b8bc..52f7c710c4 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -1190,7 +1190,7 @@ def test_skip_extensions_step(self): "ext1", ("EXT-2", "42", {"source_tmpl": "dummy.tgz"}), ("ext3", "1.1", {"source_tmpl": "dummy.tgz", "modulename": "real_ext"}), - "ext4", + ("ext4", "0.2", {"source_tmpl": "dummy.tgz", "modulename": False}), ] exts_filter = ("\ if [ %(ext_name)s == 'ext_2' ] && [ %(ext_version)s == '42' ] && [[ %(src)s == *dummy.tgz ]];\ @@ -1206,20 +1206,24 @@ def test_skip_extensions_step(self): eb.installdir = config.install_path() eb.skip = True - self.mock_stdout(True) - eb.extensions_step(fetch=True) - stdout = self.get_stdout() - self.mock_stdout(False) + with self.mocked_stdout_stderr(): + eb.extensions_step(fetch=True) + stdout = self.get_stdout() + logtxt = read_file(eb.logfile) + self.assertRegex(logtxt, r"Running shell command\W+if \[ ext1") + self.assertRegex(logtxt, r"Running shell command\W+if \[ ext_2") + self.assertRegex(logtxt, r"Running shell command\W+if \[ real_ext") + # modulename: False skips the check + self.assertNotRegex(logtxt, r"Running shell command\W+if \[ (False|ext4)") patterns = [ r"^== skipping extension EXT-2", r"^== skipping extension ext3", r"^== installing extension ext1 \(1/2\)\.\.\.", - r"^== installing extension ext4 \(2/2\)\.\.\.", + r"^== installing extension ext4 0.2 \(2/2\)\.\.\.", ] for pattern in patterns: - regex = re.compile(pattern, re.M) - self.assertTrue(regex.search(stdout), "Pattern '%s' found in: %s" % (regex.pattern, stdout)) + self.assertRegex(stdout, re.compile(pattern, re.M)) # 'ext1' should be in eb.ext_instances eb_exts = [x.name for x in eb.ext_instances] From 7dac45e31fd5fcb0a2509372b049009f39d433b0 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 5 Dec 2024 14:31:00 +0100 Subject: [PATCH 3/3] Factor out method to get modulename(s) This might be useful for easyblocks to avoid duplicating the code for determining the modulename from either the options or the name. --- easybuild/framework/extension.py | 38 +++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/easybuild/framework/extension.py b/easybuild/framework/extension.py index 051f51a218..bfe575710e 100644 --- a/easybuild/framework/extension.py +++ b/easybuild/framework/extension.py @@ -45,21 +45,13 @@ from easybuild.tools.run import run_shell_cmd -def construct_exts_filter_cmds(exts_filter, ext): - """ - Resolve the exts_filter tuple by replacing the template values using the extension - :param exts_filter: Tuple of (command, input) using template values (ext_name, ext_version, src) +def get_modulenames(ext, use_name_for_false): + """Return a list of modulenames for the extension :param ext: Instance of Extension or dictionary like with 'name' and optionally 'options', 'version', 'source' keys - :return: (cmd, input) as a tuple of strings for each modulename. Might be empty if no filtering is intented + :param use_name_for_false: Whether to return a list with the name or an empty list when the modulename is False """ - - if isinstance(exts_filter, str) or len(exts_filter) != 2: - raise EasyBuildError('exts_filter should be a list or tuple of ("command","input")') - - cmd, cmdinput = exts_filter - if not isinstance(ext, dict): - ext = {'name': ext.name, 'version': ext.version, 'src': ext.src, 'options': ext.options} + ext = {'name': ext.name, 'options': ext.options} try: modulenames = ext['options']['modulename'] @@ -71,11 +63,31 @@ def construct_exts_filter_cmds(exts_filter, ext): raise EasyBuildError(f"Empty modulename list for {ext['name']} is not supported." "Use `False` to skip checking the module!") elif modulenames is False: - return [] # Skip any checks + return [ext['name']] if use_name_for_false else [] elif not isinstance(modulenames, str): raise EasyBuildError(f"Wrong type of modulename for {ext['name']}: {type(modulenames)}: {modulenames}") else: modulenames = [modulenames] + return modulenames + + +def construct_exts_filter_cmds(exts_filter, ext): + """ + Resolve the exts_filter tuple by replacing the template values using the extension + :param exts_filter: Tuple of (command, input) using template values (ext_name, ext_version, src) + :param ext: Instance of Extension or dictionary like with 'name' and optionally 'options', 'version', 'source' keys + :return: (cmd, input) as a tuple of strings for each modulename. Might be empty if no filtering is intented + """ + + if isinstance(exts_filter, str) or len(exts_filter) != 2: + raise EasyBuildError('exts_filter should be a list or tuple of ("command","input")') + + cmd, cmdinput = exts_filter + + if not isinstance(ext, dict): + ext = {'name': ext.name, 'version': ext.version, 'src': ext.src, 'options': ext.options} + + modulenames = get_modulenames(ext, use_name_for_false=False) result = [] for modulename in modulenames: