From 981e820830f72658204a44129087ac6914b819d7 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 7 Feb 2025 10:03:20 +0100 Subject: [PATCH 01/15] enhance `apply_regex_substitutions` to support multi-line matches It might be required to replace patterns with more context, e.g. content of the next or previous line to disambiguate otherwise too generic matches. Add parameter `single_line` to enable the old behavior (default) of matching per line and otherwise match the whole text. Add parameter `match_all` to require all patterns to match for each file not only at least one. --- easybuild/tools/filetools.py | 81 ++++++++++++++++++++++++------------ test/framework/filetools.py | 48 +++++++++++++++------ 2 files changed, 90 insertions(+), 39 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index c5bf349451..7aac0c7ba4 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -1643,16 +1643,22 @@ def apply_patch(patch_file, dest, fn=None, copy=False, level=None, use_git_am=Fa return True -def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb', on_missing_match=None): +def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb', + on_missing_match=None, match_all=False, single_line=True): """ Apply specified list of regex substitutions. :param paths: list of paths to files to patch (or just a single filepath) - :param regex_subs: list of substitutions to apply, specified as (, ) + :param regex_subs: list of substitutions to apply, + specified as (, ) :param backup: create backup of original file with specified suffix (no backup if value evaluates to False) :param on_missing_match: Define what to do when no match was found in the file. Can be 'error' to raise an error, 'warn' to print a warning or 'ignore' to do nothing Defaults to the value of --strict + :param match_all: Expect to match all patterns in all files + instead of at least one per file for error/warning reporting + :param single_line: Replace first match of each pattern for each line in the order of the patterns. + If False the patterns are applied in order to the full text and may match line breaks. """ if on_missing_match is None: on_missing_match = build_option('strict') @@ -1664,18 +1670,22 @@ def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb', on_missing_m if isinstance(paths, string_type): paths = [paths] + flags = 0 if single_line else re.M + compiled_regex_subs = [(re.compile(regex, flags) if isinstance(regex, str) else regex, subtxt) + for (regex, subtxt) in regex_subs] + # only report when in 'dry run' mode if build_option('extended_dry_run'): paths_str = ', '.join(paths) dry_run_msg("applying regex substitutions to file(s): %s" % paths_str, silent=build_option('silent')) - for regex, subtxt in regex_subs: - dry_run_msg(" * regex pattern '%s', replacement string '%s'" % (regex, subtxt)) + for regex, subtxt in compiled_regex_subs: + dry_run_msg(" * regex pattern '%s', replacement string '%s'" % (regex.pattern, subtxt)) else: - _log.info("Applying following regex substitutions to %s: %s", paths, regex_subs) - - compiled_regex_subs = [(re.compile(regex), subtxt) for (regex, subtxt) in regex_subs] + _log.info("Applying following regex substitutions to %s: %s", + paths, [(regex.pattern, subtxt) for regex, subtxt in compiled_regex_subs]) + replacement_failed_msgs = [] for path in paths: try: # make sure that file can be opened in text mode; @@ -1695,32 +1705,49 @@ def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb', on_missing_m if backup: copy_file(path, path + backup) replacement_msgs = [] + replaced = [False] * len(compiled_regex_subs) with open_file(path, 'w') as out_file: - lines = txt_utf8.split('\n') - del txt_utf8 - for line_id, line in enumerate(lines): - for regex, subtxt in compiled_regex_subs: - match = regex.search(line) - if match: + if single_line: + lines = txt_utf8.split('\n') + del txt_utf8 + for line_id, line in enumerate(lines): + for i, (regex, subtxt) in enumerate(compiled_regex_subs): + match = regex.search(line) + if match: + origtxt = match.group(0) + replacement_msgs.append("Replaced in line %d: '%s' -> '%s'" % + (line_id + 1, origtxt, subtxt)) + replaced[i] = True + line = regex.sub(subtxt, line) + lines[line_id] = line + out_file.write('\n'.join(lines)) + else: + for i, (regex, subtxt) in enumerate(compiled_regex_subs): + def do_replace(match): origtxt = match.group(0) - replacement_msgs.append("Replaced in line %d: '%s' -> '%s'" % - (line_id + 1, origtxt, subtxt)) - line = regex.sub(subtxt, line) - lines[line_id] = line - out_file.write('\n'.join(lines)) + # pylint: disable=cell-var-from-loop + cur_subtxt = match.expand(subtxt) + # pylint: disable=cell-var-from-loop + replacement_msgs.append("Replaced: '%s' -> '%s'" % (origtxt, cur_subtxt)) + return cur_subtxt + txt_utf8, replaced[i] = regex.subn(do_replace, txt_utf8) + out_file.write(txt_utf8) if replacement_msgs: _log.info('Applied the following substitutions to %s:\n%s', path, '\n'.join(replacement_msgs)) - else: - msg = 'Nothing found to replace in %s' % path - if on_missing_match == ERROR: - raise EasyBuildError(msg) - elif on_missing_match == WARN: - _log.warning(msg) - else: - _log.info(msg) - + if (match_all and not all(replaced)) or (not match_all and not any(replaced)): + errors = ["Nothing found to replace '%s'" % regex.pattern + for cur_replaced, (regex, _) in zip(replaced, compiled_regex_subs) if not cur_replaced] + replacement_failed_msgs.append(', '.join(errors) + ' in ' + path) except (IOError, OSError) as err: raise EasyBuildError("Failed to patch %s: %s", path, err) + if replacement_failed_msgs: + msg = '\n'.join(replacement_failed_msgs) + if on_missing_match == ERROR: + raise EasyBuildError(msg) + elif on_missing_match == WARN: + _log.warning(msg) + else: + _log.info(msg) def modify_env(old, new): diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 2e2b030fc8..d0c4c6fc15 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -1443,16 +1443,27 @@ def test_apply_regex_substitutions(self): # Check handling of on_missing_match ft.write_file(testfile, testtxt) regex_subs_no_match = [('Not there', 'Not used')] - error_pat = 'Nothing found to replace in %s' % testfile + error_pat = "Nothing found to replace 'Not there' in %s" % testfile # Error self.assertErrorRegex(EasyBuildError, error_pat, ft.apply_regex_substitutions, testfile, regex_subs_no_match, on_missing_match=run.ERROR) + # First matches, but 2nd not + regex_subs_part_match = [regex_subs[0], ('Not there', 'Not used')] + self.assertErrorRegex(EasyBuildError, error_pat, ft.apply_regex_substitutions, testfile, regex_subs_part_match, + on_missing_match=run.ERROR, match_all=True) + # First matched so OK with match_all + ft.apply_regex_substitutions(testfile, regex_subs_part_match, + on_missing_match=run.ERROR, match_all=False) # Warn with self.log_to_testlogfile(): ft.apply_regex_substitutions(testfile, regex_subs_no_match, on_missing_match=run.WARN) logtxt = ft.read_file(self.logfile) self.assertIn('WARNING ' + error_pat, logtxt) + with self.log_to_testlogfile(): + ft.apply_regex_substitutions(testfile, regex_subs_part_match, on_missing_match=run.WARN, match_all=True) + logtxt = ft.read_file(self.logfile) + self.assertIn('WARNING ' + error_pat, logtxt) # Ignore with self.log_to_testlogfile(): @@ -1465,6 +1476,21 @@ def test_apply_regex_substitutions(self): path = os.path.join(self.test_prefix, 'nosuchfile.txt') self.assertErrorRegex(EasyBuildError, error_pat, ft.apply_regex_substitutions, path, regex_subs) + # Replace multi-line strings + testtxt = "This si wrong\nBut mkae right\nLeave this!" + expected_testtxt = 'This is wrong.\nBut make right\nLeave this!' + ft.write_file(testfile, testtxt) + repl = ('This si( .*)\n(.*)mkae right$', 'This is wrong.\nBut make right') + ft.apply_regex_substitutions(testfile, [repl], backup=False, on_missing_match=ERROR, single_line=False) + new_testtxt = ft.read_file(testfile) + self.assertEqual(new_testtxt, expected_testtxt) + # Supports capture groups + ft.write_file(testfile, testtxt) + repl = ('This si( .*)\n(.*)mkae right$', r'This is\1.\n\2make right') + ft.apply_regex_substitutions(testfile, [repl], backup=False, on_missing_match=ERROR, single_line=False) + new_testtxt = ft.read_file(testfile) + self.assertEqual(new_testtxt, expected_testtxt) + # make sure apply_regex_substitutions can patch files that include UTF-8 characters testtxt = b"foo \xe2\x80\x93 bar" # This is an UTF-8 "-" ft.write_file(testfile, testtxt) @@ -1485,34 +1511,32 @@ def test_apply_regex_substitutions(self): # also test apply_regex_substitutions with a *list* of paths # cfr. https://github.com/easybuilders/easybuild-framework/issues/3493 + # and a compiled regex test_dir = os.path.join(self.test_prefix, 'test_dir') test_file1 = os.path.join(test_dir, 'one.txt') test_file2 = os.path.join(test_dir, 'two.txt') ft.write_file(test_file1, "Donald is an elephant") ft.write_file(test_file2, "2 + 2 = 5") regexs = [ - ('Donald', 'Dumbo'), + (re.compile('donald', re.I), 'Dumbo'), # Only matches if this is used as-is ('= 5', '= 4'), ] ft.apply_regex_substitutions([test_file1, test_file2], regexs) # also check dry run mode init_config(build_options={'extended_dry_run': True}) - self.mock_stderr(True) - self.mock_stdout(True) - ft.apply_regex_substitutions([test_file1, test_file2], regexs) - stderr, stdout = self.get_stderr(), self.get_stdout() - self.mock_stderr(False) - self.mock_stdout(False) + with self.mocked_stdout_stderr(): + ft.apply_regex_substitutions([test_file1, test_file2], regexs) + stderr, stdout = self.get_stderr(), self.get_stdout() self.assertFalse(stderr) - regex = re.compile('\n'.join([ + regex = '\n'.join([ r"applying regex substitutions to file\(s\): .*/test_dir/one.txt, .*/test_dir/two.txt", - r" \* regex pattern 'Donald', replacement string 'Dumbo'", + r" \* regex pattern 'donald', replacement string 'Dumbo'", r" \* regex pattern '= 5', replacement string '= 4'", '', - ])) - self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + ]) + self.assertTrue(re.search(regex, stdout), "Pattern '%s' should be found in: %s" % (regex, stdout)) def test_find_flexlm_license(self): """Test find_flexlm_license function.""" From 7e53406d9e8c1b938cc9f88b318edf2c4576b8cc Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 10 Feb 2025 12:21:30 +0100 Subject: [PATCH 02/15] Verify format of regex_subs --- easybuild/tools/filetools.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 7aac0c7ba4..a9dabe0414 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -1664,11 +1664,14 @@ def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb', on_missing_match = build_option('strict') allowed_values = (ERROR, IGNORE, WARN) if on_missing_match not in allowed_values: - raise EasyBuildError('Invalid value passed to on_missing_match: %s (allowed: %s)', - on_missing_match, ', '.join(allowed_values)) + raise ValueError('Invalid value passed to on_missing_match: %s (allowed: %s)', + on_missing_match, ', '.join(allowed_values)) if isinstance(paths, string_type): paths = [paths] + if (not isinstance(regex_subs, (list, tuple)) or + not all(isinstance(sub, (list, tuple)) and len(sub) == 2 for sub in regex_subs)): + raise ValueError('Parameter regex_subs must be a list of 2-element tuples. Got:', regex_subs) flags = 0 if single_line else re.M compiled_regex_subs = [(re.compile(regex, flags) if isinstance(regex, str) else regex, subtxt) From a34a9c2791af88bf4c8dab99c7082df06009d650 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 10 Feb 2025 15:31:12 +0100 Subject: [PATCH 03/15] Add test for using full group match --- test/framework/filetools.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index d0c4c6fc15..4fe38495a3 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -1486,10 +1486,13 @@ def test_apply_regex_substitutions(self): self.assertEqual(new_testtxt, expected_testtxt) # Supports capture groups ft.write_file(testfile, testtxt) - repl = ('This si( .*)\n(.*)mkae right$', r'This is\1.\n\2make right') - ft.apply_regex_substitutions(testfile, [repl], backup=False, on_missing_match=ERROR, single_line=False) + repls = [ + ('This si( .*)\n(.*)mkae right$', r'This is\1.\n\2make right'), + ('Lea(ve)', r'Do \g<0>\1'), # Reference to full match + ] + ft.apply_regex_substitutions(testfile, repls, backup=False, on_missing_match=ERROR, single_line=False) new_testtxt = ft.read_file(testfile) - self.assertEqual(new_testtxt, expected_testtxt) + self.assertEqual(new_testtxt, expected_testtxt.replace('Leave', 'Do Leaveve')) # make sure apply_regex_substitutions can patch files that include UTF-8 characters testtxt = b"foo \xe2\x80\x93 bar" # This is an UTF-8 "-" From 8e4d419d95ad6f46170d37508dae12e6a0d30cb7 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 21 Feb 2025 14:16:34 +0100 Subject: [PATCH 04/15] Use EASYBLOCK_CLASS_PREFIX consistently --- easybuild/scripts/mk_tmpl_easyblock_for.py | 7 ++++--- easybuild/tools/include.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/easybuild/scripts/mk_tmpl_easyblock_for.py b/easybuild/scripts/mk_tmpl_easyblock_for.py index 4e35442024..da55b66e07 100755 --- a/easybuild/scripts/mk_tmpl_easyblock_for.py +++ b/easybuild/scripts/mk_tmpl_easyblock_for.py @@ -36,7 +36,7 @@ import sys from optparse import OptionParser -from easybuild.tools.filetools import encode_class_name +from easybuild.tools.filetools import encode_class_name, EASYBLOCK_CLASS_PREFIX # parse options parser = OptionParser() @@ -83,8 +83,9 @@ # determine parent easyblock class parent_import = "from easybuild.framework.easyblock import EasyBlock" if not options.parent == "EasyBlock": - if options.parent.startswith('EB_'): - ebmod = options.parent[3:].lower() # FIXME: here we should actually decode the encoded class name + if options.parent.startswith(EASYBLOCK_CLASS_PREFIX): + # FIXME: here we should actually decode the encoded class name + ebmod = options.parent[len(EASYBLOCK_CLASS_PREFIX):].lower() else: ebmod = "generic.%s" % options.parent.lower() parent_import = "from easybuild.easyblocks.%s import %s" % (ebmod, options.parent) diff --git a/easybuild/tools/include.py b/easybuild/tools/include.py index acfc74696d..17be1c713a 100644 --- a/easybuild/tools/include.py +++ b/easybuild/tools/include.py @@ -37,7 +37,7 @@ from easybuild.base import fancylogger from easybuild.tools.build_log import EasyBuildError -from easybuild.tools.filetools import expand_glob_paths, read_file, symlink +from easybuild.tools.filetools import expand_glob_paths, read_file, symlink, EASYBLOCK_CLASS_PREFIX # these are imported just to we can reload them later import easybuild.tools.module_naming_scheme import easybuild.toolchains @@ -157,7 +157,7 @@ def verify_imports(pymods, pypkg, from_path): def is_software_specific_easyblock(module): """Determine whether Python module at specified location is a software-specific easyblock.""" - return bool(re.search(r'^class EB_.*\(.*\):\s*$', read_file(module), re.M)) + return bool(re.search(r'^class %s.*\(.*\):\s*$' % EASYBLOCK_CLASS_PREFIX, read_file(module), re.M)) def include_easyblocks(tmpdir, paths): From 00ace4653b5ae9b429c74efbce278b17cd921ce9 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 21 Feb 2025 14:26:13 +0100 Subject: [PATCH 05/15] Ignore other classes if software specific easyblock class was found If we have other classes next to a software specific easyblock class in an easyblock file they are likely supplemental. So ignore them in `avail_easyblocks`. --- easybuild/framework/easyconfig/tools.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index e3a93d652a..ac6bc88d4f 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -58,7 +58,7 @@ from easybuild.tools.build_log import EasyBuildError, print_error, print_msg, print_warning from easybuild.tools.config import build_option from easybuild.tools.environment import restore_env -from easybuild.tools.filetools import find_easyconfigs, is_patch_file, locate_files +from easybuild.tools.filetools import find_easyconfigs, is_generic_easyblock, is_patch_file, locate_files from easybuild.tools.filetools import read_file, resolve_path, which, write_file from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO from easybuild.tools.github import det_pr_labels, det_pr_title, download_repo, fetch_easyconfigs_from_commit @@ -783,6 +783,12 @@ def avail_easyblocks(): easyblock_loc = os.path.join(path, fn) class_names = class_regex.findall(read_file(easyblock_loc)) + if len(class_names) > 1: + # If there is exactly one software specific easyblock we use that + sw_specific_class_names = [name for name in class_names + if not is_generic_easyblock(name)] + if len(sw_specific_class_names) == 1: + class_names = sw_specific_class_names if len(class_names) == 1: easyblock_class = class_names[0] elif class_names: From 5261d18610c3b57f3290965fdb3a1d64ba78c9ab Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 21 Feb 2025 15:21:10 +0100 Subject: [PATCH 06/15] Test that additional classes in easyblocks don't cause issues --- test/framework/sandbox/easybuild/easyblocks/f/foofoo.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/framework/sandbox/easybuild/easyblocks/f/foofoo.py b/test/framework/sandbox/easybuild/easyblocks/f/foofoo.py index 1555f1a810..86d2a5a934 100644 --- a/test/framework/sandbox/easybuild/easyblocks/f/foofoo.py +++ b/test/framework/sandbox/easybuild/easyblocks/f/foofoo.py @@ -32,6 +32,14 @@ from easybuild.framework.easyconfig import CUSTOM, MANDATORY +class dummy1: + """Only to verify that unrelated classes in software specific easyblocks are ignored""" + + +class dummy2(dummy1): + """Same but with inheritance""" + + class EB_foofoo(EB_foo): """Support for building/installing foofoo.""" From 612f852b0fdab7a8599a590c0b969d580ea46dc7 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 21 Feb 2025 15:21:49 +0100 Subject: [PATCH 07/15] Simplify avail_easyblocks - Reduce indentation - Assign directly to dict --- easybuild/framework/easyconfig/tools.py | 58 ++++++++++++------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index ac6bc88d4f..3a2ad8b9fa 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -768,39 +768,37 @@ def avail_easyblocks(): __import__(pkg) # determine paths for this package - paths = sys.modules[pkg].__path__ + paths = [path for path in sys.modules[pkg].__path__ if os.path.exists(path)] # import all modules in these paths for path in paths: - if os.path.exists(path): - for fn in os.listdir(path): - res = module_regexp.match(fn) - if res: - easyblock_mod_name = '%s.%s' % (pkg, res.group(1)) - - if easyblock_mod_name not in easyblocks: - __import__(easyblock_mod_name) - easyblock_loc = os.path.join(path, fn) - - class_names = class_regex.findall(read_file(easyblock_loc)) - if len(class_names) > 1: - # If there is exactly one software specific easyblock we use that - sw_specific_class_names = [name for name in class_names - if not is_generic_easyblock(name)] - if len(sw_specific_class_names) == 1: - class_names = sw_specific_class_names - if len(class_names) == 1: - easyblock_class = class_names[0] - elif class_names: - raise EasyBuildError("Found multiple class names for easyblock %s: %s", - easyblock_loc, class_names) - else: - raise EasyBuildError("Failed to determine easyblock class name for %s", easyblock_loc) - - easyblocks[easyblock_mod_name] = {'class': easyblock_class, 'loc': easyblock_loc} - else: - _log.debug("%s already imported from %s, ignoring %s", - easyblock_mod_name, easyblocks[easyblock_mod_name]['loc'], path) + for fn in os.listdir(path): + res = module_regexp.match(fn) + if not res: + continue + easyblock_mod_name = '%s.%s' % (pkg, res.group(1)) + + if easyblock_mod_name in easyblocks: + _log.debug("%s already imported from %s, ignoring %s", + easyblock_mod_name, easyblocks[easyblock_mod_name]['loc'], path) + else: + __import__(easyblock_mod_name) + easyblock_loc = os.path.join(path, fn) + + class_names = class_regex.findall(read_file(easyblock_loc)) + if len(class_names) > 1: + # If there is exactly one software specific easyblock we use that + sw_specific_class_names = [name for name in class_names + if not is_generic_easyblock(name)] + if len(sw_specific_class_names) == 1: + class_names = sw_specific_class_names + if len(class_names) == 1: + easyblocks[easyblock_mod_name] = {'class': class_names[0], 'loc': easyblock_loc} + elif class_names: + raise EasyBuildError("Found multiple class names for easyblock %s: %s", + easyblock_loc, class_names) + else: + raise EasyBuildError("Failed to determine easyblock class name for %s", easyblock_loc) return easyblocks From ef10fa72ce0d47b3b6a9730aeed302262d9e0acb Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 24 Feb 2025 09:06:04 +0100 Subject: [PATCH 08/15] Fix class name regex and add test to generic easyblock The class name regex is MULTILINE and hence might start a match at a class name without a super-class and match until another class name with a super-class Restrict by excluding colons in addition to parentheses Also add test for this and the multi-class case to a generic easyblock --- easybuild/framework/easyconfig/tools.py | 2 +- .../sandbox/easybuild/easyblocks/generic/bar.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 3a2ad8b9fa..f57d676bbe 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -758,7 +758,7 @@ def avail_easyblocks(): """Return a list of all available easyblocks.""" module_regexp = re.compile(r"^([^_].*)\.py$") - class_regex = re.compile(r"^class ([^(]*)\(", re.M) + class_regex = re.compile(r"^class ([^(:]*)\(", re.M) # finish initialisation of the toolchain module (ie set the TC_CONSTANT constants) search_toolchain('') diff --git a/test/framework/sandbox/easybuild/easyblocks/generic/bar.py b/test/framework/sandbox/easybuild/easyblocks/generic/bar.py index 0ba3aaceed..2d18a45c5d 100644 --- a/test/framework/sandbox/easybuild/easyblocks/generic/bar.py +++ b/test/framework/sandbox/easybuild/easyblocks/generic/bar.py @@ -32,6 +32,18 @@ from easybuild.framework.easyconfig import CUSTOM, MANDATORY +class dummy1: + """Only to verify that unrelated classes in software specific easyblocks are ignored""" + + +class dummy2(dummy1): + """Same but with inheritance""" + + +class dummy1: + """Class without inheritance before the real easyblock to verify the regex not being too greedy""" + + class bar(EasyBlock): """Generic support for building/installing bar.""" From c9081c47edd7f87a4c6ae576e37f78cd712e9e3b Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 24 Feb 2025 09:08:46 +0100 Subject: [PATCH 09/15] Also exclude unrelated classes in generic easyblocks --- easybuild/framework/easyconfig/tools.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index f57d676bbe..9c43e0d8be 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -776,24 +776,30 @@ def avail_easyblocks(): res = module_regexp.match(fn) if not res: continue - easyblock_mod_name = '%s.%s' % (pkg, res.group(1)) + easyblock_mod_name = res.group(1) + easyblock_full_mod_name = '%s.%s' % (pkg, easyblock_mod_name) - if easyblock_mod_name in easyblocks: + if easyblock_full_mod_name in easyblocks: _log.debug("%s already imported from %s, ignoring %s", - easyblock_mod_name, easyblocks[easyblock_mod_name]['loc'], path) + easyblock_full_mod_name, easyblocks[easyblock_full_mod_name]['loc'], path) else: - __import__(easyblock_mod_name) + __import__(easyblock_full_mod_name) easyblock_loc = os.path.join(path, fn) class_names = class_regex.findall(read_file(easyblock_loc)) if len(class_names) > 1: - # If there is exactly one software specific easyblock we use that - sw_specific_class_names = [name for name in class_names - if not is_generic_easyblock(name)] + if pkg.endswith('.generic'): + # In generic easyblocks we have e.g. ConfigureMake in configuremake.py + sw_specific_class_names = [name for name in class_names + if name.lower() == easyblock_mod_name.lower()] + else: + # If there is exactly one software specific easyblock we use that + sw_specific_class_names = [name for name in class_names + if not is_generic_easyblock(name)] if len(sw_specific_class_names) == 1: class_names = sw_specific_class_names if len(class_names) == 1: - easyblocks[easyblock_mod_name] = {'class': class_names[0], 'loc': easyblock_loc} + easyblocks[easyblock_full_mod_name] = {'class': class_names[0], 'loc': easyblock_loc} elif class_names: raise EasyBuildError("Found multiple class names for easyblock %s: %s", easyblock_loc, class_names) From 002b20a6c470f0863684f5cc452535b43f18b35a Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 24 Feb 2025 10:40:16 +0100 Subject: [PATCH 10/15] Fix `get_easyblock_classes` for non-EasyBlock classes Unrelated class in an easyblock file have the same package name as the easyblock and hence the filter doesn't remove them. Instead of the package name check if the class derives from `EasyBlock` which is a much more reliable check whether the class is an EasyBlock. --- easybuild/tools/docs.py | 6 +++--- test/framework/docs.py | 24 ++++++------------------ 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/easybuild/tools/docs.py b/easybuild/tools/docs.py index 1d1763c01f..fbea27b286 100644 --- a/easybuild/tools/docs.py +++ b/easybuild/tools/docs.py @@ -1313,15 +1313,15 @@ def get_easyblock_classes(package_name): """ Get list of all easyblock classes in specified easyblocks.* package """ - easyblocks = [] + easyblocks = set() modules = import_available_modules(package_name) for mod in modules: for name, _ in inspect.getmembers(mod, inspect.isclass): eb_class = getattr(mod, name) # skip imported classes that are not easyblocks - if eb_class.__module__.startswith(package_name) and eb_class not in easyblocks: - easyblocks.append(eb_class) + if eb_class.__module__.startswith(package_name) and EasyBlock in inspect.getmro(eb_class): + easyblocks.add(eb_class) return easyblocks diff --git a/test/framework/docs.py b/test/framework/docs.py index 93db1ad505..3ceb1a0b64 100644 --- a/test/framework/docs.py +++ b/test/framework/docs.py @@ -520,7 +520,7 @@ def test_get_easyblock_classes(self): def test_gen_easyblocks_overview(self): """ Test gen_easyblocks_overview_* functions """ gen_easyblocks_pkg = 'easybuild.easyblocks.generic' - modules = import_available_modules(gen_easyblocks_pkg) + names = [eb_class.__name__ for eb_class in get_easyblock_classes(gen_easyblocks_pkg)] common_params = { 'ConfigureMake': ['configopts', 'buildopts', 'installopts'], } @@ -564,15 +564,9 @@ def test_gen_easyblocks_overview(self): ]) self.assertIn(check_configuremake, ebdoc) - names = [] - for mod in modules: - for name, _ in inspect.getmembers(mod, inspect.isclass): - eb_class = getattr(mod, name) - # skip imported classes that are not easyblocks - if eb_class.__module__.startswith(gen_easyblocks_pkg): - self.assertIn(name, ebdoc) - names.append(name) + for name in names: + self.assertIn(name, ebdoc) toc = [":ref:`" + n + "`" for n in sorted(set(names))] pattern = " - ".join(toc) @@ -610,17 +604,11 @@ def test_gen_easyblocks_overview(self): ]) self.assertIn(check_configuremake, ebdoc) - names = [] - for mod in modules: - for name, _ in inspect.getmembers(mod, inspect.isclass): - eb_class = getattr(mod, name) - # skip imported classes that are not easyblocks - if eb_class.__module__.startswith(gen_easyblocks_pkg): - self.assertIn(name, ebdoc) - names.append(name) + for name in names: + self.assertIn(name, ebdoc) - toc = ["\\[" + n + "\\]\\(#" + n.lower() + "\\)" for n in sorted(set(names))] + toc = ["\\[" + n + "\\]\\(#" + n.lower() + "\\)" for n in sorted(names)] pattern = " - ".join(toc) regex = re.compile(pattern) self.assertTrue(re.search(regex, ebdoc), "Pattern %s found in %s" % (regex.pattern, ebdoc)) From 94b9a72371bb7f58677b224c9ff601e245e4131c Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 24 Feb 2025 10:45:38 +0100 Subject: [PATCH 11/15] Raise an error if there is an easyblock with no easyblock class found --- easybuild/tools/docs.py | 4 ++++ test/framework/docs.py | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/docs.py b/easybuild/tools/docs.py index fbea27b286..7fb20aeb73 100644 --- a/easybuild/tools/docs.py +++ b/easybuild/tools/docs.py @@ -1317,11 +1317,15 @@ def get_easyblock_classes(package_name): modules = import_available_modules(package_name) for mod in modules: + easyblock_found = False for name, _ in inspect.getmembers(mod, inspect.isclass): eb_class = getattr(mod, name) # skip imported classes that are not easyblocks if eb_class.__module__.startswith(package_name) and EasyBlock in inspect.getmro(eb_class): easyblocks.add(eb_class) + easyblock_found = True + if not easyblock_found: + raise RuntimeError("No easyblocks found in module: %s", mod.__name__) return easyblocks diff --git a/test/framework/docs.py b/test/framework/docs.py index 3ceb1a0b64..d79a563835 100644 --- a/test/framework/docs.py +++ b/test/framework/docs.py @@ -25,7 +25,6 @@ """ Unit tests for docs.py. """ -import inspect import os import re import sys @@ -38,7 +37,7 @@ from easybuild.tools.docs import list_easyblocks, list_software, list_toolchains from easybuild.tools.docs import md_title_and_table, rst_title_and_table from easybuild.tools.options import EasyBuildOptions -from easybuild.tools.utilities import import_available_modules, mk_md_table, mk_rst_table +from easybuild.tools.utilities import mk_md_table, mk_rst_table from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, init_config From 06189626f1dc1314cdcb62db523b84864b32d162 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 24 Feb 2025 14:06:06 +0100 Subject: [PATCH 12/15] Fix C&P mistake in test --- test/framework/sandbox/easybuild/easyblocks/generic/bar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/sandbox/easybuild/easyblocks/generic/bar.py b/test/framework/sandbox/easybuild/easyblocks/generic/bar.py index 2d18a45c5d..d1bd4700d5 100644 --- a/test/framework/sandbox/easybuild/easyblocks/generic/bar.py +++ b/test/framework/sandbox/easybuild/easyblocks/generic/bar.py @@ -40,7 +40,7 @@ class dummy2(dummy1): """Same but with inheritance""" -class dummy1: +class dummy3: """Class without inheritance before the real easyblock to verify the regex not being too greedy""" From 72e18eb278e6f4801700d82dd91adf629fd1cdc3 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 24 Feb 2025 15:24:02 +0100 Subject: [PATCH 13/15] Make software-specific detection regex more strict --- easybuild/tools/include.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/include.py b/easybuild/tools/include.py index 17be1c713a..96e5bfd66c 100644 --- a/easybuild/tools/include.py +++ b/easybuild/tools/include.py @@ -157,7 +157,8 @@ def verify_imports(pymods, pypkg, from_path): def is_software_specific_easyblock(module): """Determine whether Python module at specified location is a software-specific easyblock.""" - return bool(re.search(r'^class %s.*\(.*\):\s*$' % EASYBLOCK_CLASS_PREFIX, read_file(module), re.M)) + # All software-specific easyblocks start with the prefix and derive from another class, at least EasyBlock + return bool(re.search(r"^class %s[^(:]+\([^)]+\):\s*$" % EASYBLOCK_CLASS_PREFIX, read_file(module), re.M)) def include_easyblocks(tmpdir, paths): From 8c0e5003bd922c03f64716b244f5bb2c88af66bc Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 3 Mar 2025 20:41:36 +0100 Subject: [PATCH 14/15] use more direct way to filter out class names for software-specific easyblocks based on 'EB_' prefix --- easybuild/framework/easyconfig/tools.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 9c43e0d8be..158c37a73e 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -58,7 +58,7 @@ from easybuild.tools.build_log import EasyBuildError, print_error, print_msg, print_warning from easybuild.tools.config import build_option from easybuild.tools.environment import restore_env -from easybuild.tools.filetools import find_easyconfigs, is_generic_easyblock, is_patch_file, locate_files +from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX, find_easyconfigs, is_patch_file, locate_files from easybuild.tools.filetools import read_file, resolve_path, which, write_file from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO from easybuild.tools.github import det_pr_labels, det_pr_title, download_repo, fetch_easyconfigs_from_commit @@ -795,7 +795,7 @@ def avail_easyblocks(): else: # If there is exactly one software specific easyblock we use that sw_specific_class_names = [name for name in class_names - if not is_generic_easyblock(name)] + if name.startswith(EASYBLOCK_CLASS_PREFIX)] if len(sw_specific_class_names) == 1: class_names = sw_specific_class_names if len(class_names) == 1: From ade0d8814563235e7815966ff506467a3f4332a0 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Fri, 7 Mar 2025 08:22:18 +0100 Subject: [PATCH 15/15] use WARN rather than run.WARN in test_apply_regex_substitutions --- test/framework/filetools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index e04359990b..fca57c0a4e 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -1578,7 +1578,7 @@ def test_apply_regex_substitutions(self): logtxt = ft.read_file(self.logfile) self.assertIn('WARNING ' + error_pat, logtxt) with self.log_to_testlogfile(): - ft.apply_regex_substitutions(testfile, regex_subs_part_match, on_missing_match=run.WARN, match_all=True) + ft.apply_regex_substitutions(testfile, regex_subs_part_match, on_missing_match=WARN, match_all=True) logtxt = ft.read_file(self.logfile) self.assertIn('WARNING ' + error_pat, logtxt)