From 17f21849d3f5336649505ead011a1d8207ee27ab Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 11:11:08 +0100 Subject: [PATCH 01/14] Use get_ref instead of enable/disable templating --- easybuild/framework/extension.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/easybuild/framework/extension.py b/easybuild/framework/extension.py index 297a736bad..aaee9c82f3 100644 --- a/easybuild/framework/extension.py +++ b/easybuild/framework/extension.py @@ -144,10 +144,8 @@ def sanity_check_step(self): if os.path.isdir(self.installdir): change_dir(self.installdir) - # disabling templating is required here to support legacy string templates like name/version - self.cfg.enable_templating = False - exts_filter = self.cfg['exts_filter'] - self.cfg.enable_templating = True + # Get raw value to translate ext_name, ext_version, src + exts_filter = self.cfg.get_ref('exts_filter') if exts_filter is not None: cmd, inp = exts_filter From a90749cae004ec34f2a5a2ce64a614c201d07dc4 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 11:11:15 +0100 Subject: [PATCH 02/14] Actually remove deprecated name/version in exts_filter --- easybuild/framework/extension.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/easybuild/framework/extension.py b/easybuild/framework/extension.py index aaee9c82f3..d4f3c298be 100644 --- a/easybuild/framework/extension.py +++ b/easybuild/framework/extension.py @@ -168,11 +168,7 @@ def sanity_check_step(self): 'ext_name': modname, 'ext_version': self.version, 'src': self.src, - # the ones below are only there for legacy purposes - # TODO deprecated, remove in v2.0 # TODO same dict is used in easyblock.py skip_extensions, resolve this - 'name': modname, - 'version': self.version, } cmd = cmd % template From fbbd370caf447f5a2b6e2ea5caff0a08a0077185 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 15:32:38 +0100 Subject: [PATCH 03/14] Add free function raise_nosupport --- easybuild/tools/build_log.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/build_log.py b/easybuild/tools/build_log.py index a6d6777d65..71a9d389bc 100644 --- a/easybuild/tools/build_log.py +++ b/easybuild/tools/build_log.py @@ -90,6 +90,11 @@ def raise_easybuilderror(msg, *args): raise EasyBuildError(msg, *args) +def raise_nosupport(msg, ver): + """Construct error message for no longer supported behaviour, and raise an EasyBuildError.""" + nosupport_msg = "NO LONGER SUPPORTED since v%s: %s; see %s for more information" + raise_easybuilderror(nosupport_msg, ver, msg, DEPRECATED_DOC_URL) + class EasyBuildLog(fancylogger.FancyLogger): """ The EasyBuild logger, with its own error and exception functions. @@ -154,9 +159,8 @@ def log_callback_warning_and_print(msg): fancylogger.FancyLogger.deprecated(self, msg, ver, max_ver, *args, **kwargs) def nosupport(self, msg, ver): - """Print error message for no longer supported behaviour, and raise an EasyBuildError.""" - nosupport_msg = "NO LONGER SUPPORTED since v%s: %s; see %s for more information" - raise EasyBuildError(nosupport_msg, ver, msg, DEPRECATED_DOC_URL) + """Raise error message for no longer supported behaviour, and raise an EasyBuildError.""" + raise_nosupport(msg, ver) def error(self, msg, *args, **kwargs): """Print error message and raise an EasyBuildError.""" From be6d1f3c736db3f2dec53b4f0da4eb26cd86a06a Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 15:33:05 +0100 Subject: [PATCH 04/14] Fix docu of exts_filter --- easybuild/framework/easyconfig/default.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/default.py b/easybuild/framework/easyconfig/default.py index 12417766a5..920e27d1bd 100644 --- a/easybuild/framework/easyconfig/default.py +++ b/easybuild/framework/easyconfig/default.py @@ -167,7 +167,7 @@ 'exts_defaultclass': [None, "List of module for and name of the default extension class", EXTENSIONS], 'exts_default_options': [{}, "List of default options for extensions", EXTENSIONS], 'exts_filter': [None, ("Extension filter details: template for cmd and input to cmd " - "(templates for name, version and src)."), EXTENSIONS], + "(templates for ext_name, ext_version and src)."), EXTENSIONS], 'exts_list': [[], 'List with extensions added to the base installation', EXTENSIONS], # MODULES easyconfig parameters From 5aff4cc3189665b861c661a927cd8b3fd8ed2c68 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 15:33:36 +0100 Subject: [PATCH 05/14] Avoid template warnings when accessing exts_list Just get the value w/o template replacements for logging Adresses #3124 --- easybuild/framework/extensioneasyblock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index 230592f171..35b1bf4407 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -116,9 +116,9 @@ def sanity_check_step(self, exts_filter=None, custom_paths=None, custom_commands """ Custom sanity check for extensions, whether installed as stand-alone module or not """ - if not self.cfg['exts_filter']: + if not self.cfg.get_ref('exts_filter'): self.cfg['exts_filter'] = exts_filter - self.log.debug("starting sanity check for extension with filter %s", self.cfg['exts_filter']) + self.log.debug("starting sanity check for extension with filter %s", self.cfg.get_ref('exts_filter')) # for stand-alone installations that were done for multiple dependency versions (via multi_deps), # we need to perform the extension sanity check for each of them, by loading the corresponding modules first From caa59448bb250be80aaa9d2c8a92ff3dfca823f8 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 15:34:40 +0100 Subject: [PATCH 06/14] Introduce fixed function resolve_exts_filter_template - Avoid duplicated code in EasyBlock and Extension - Fix wrong attribute 'source' accessed instead of 'src' - Enhance testcase to detect above issue - Remove deprecated (and supposedly in v2.0 removed) template args Fixes #3124 --- easybuild/framework/easyblock.py | 34 ++--------- easybuild/framework/extension.py | 56 +++++++++++++------ test/framework/easyblock.py | 38 ++++++++----- test/framework/sandbox/sources/p/pi/dummy.tgz | 0 4 files changed, 69 insertions(+), 59 deletions(-) create mode 100644 test/framework/sandbox/sources/p/pi/dummy.tgz diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 256a9eeb18..c019ec502c 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -61,6 +61,7 @@ from easybuild.framework.easyconfig.style import MAX_LINE_LENGTH from easybuild.framework.easyconfig.tools import get_paths_for from easybuild.framework.easyconfig.templates import TEMPLATE_NAMES_EASYBLOCK_RUN_STEP, template_constant_dict +from easybuild.framework.extension import resolve_exts_filter_template from easybuild.tools import config, filetools from easybuild.tools.build_details import get_build_stats from easybuild.tools.build_log import EasyBuildError, dry_run_msg, dry_run_warning, dry_run_set_dirs @@ -1446,43 +1447,18 @@ def skip_extensions(self): if not exts_filter or len(exts_filter) == 0: raise EasyBuildError("Skipping of extensions, but no exts_filter set in easyconfig") - elif isinstance(exts_filter, string_type) or len(exts_filter) != 2: - raise EasyBuildError('exts_filter should be a list or tuple of ("command","input")') - cmdtmpl = exts_filter[0] - cmdinputtmpl = exts_filter[1] res = [] for ext in self.exts: - 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('source'), - } - - try: - cmd = cmdtmpl % tmpldict - 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" - self.log.nosupport(msg % err, '2.0') - - if cmdinputtmpl: - stdin = cmdinputtmpl % tmpldict - (cmdstdouterr, ec) = run_cmd(cmd, log_all=False, log_ok=False, simple=False, inp=stdin, regexp=False) - else: - (cmdstdouterr, ec) = run_cmd(cmd, log_all=False, log_ok=False, simple=False, regexp=False) + cmd, stdin = resolve_exts_filter_template(exts_filter, ext) + (cmdstdouterr, ec) = run_cmd(cmd, log_all=False, log_ok=False, simple=False, inp=stdin, regexp=False) self.log.info("exts_filter result %s %s", cmdstdouterr, ec) if ec: - self.log.info("Not skipping %s" % name) + self.log.info("Not skipping %s" % ext['name']) self.log.debug("exit code: %s, stdout/err: %s" % (ec, cmdstdouterr)) res.append(ext) else: - self.log.info("Skipping %s" % name) + self.log.info("Skipping %s" % ext['name']) self.exts = res # diff --git a/easybuild/framework/extension.py b/easybuild/framework/extension.py index d4f3c298be..f516511125 100644 --- a/easybuild/framework/extension.py +++ b/easybuild/framework/extension.py @@ -38,11 +38,46 @@ from easybuild.framework.easyconfig.easyconfig import resolve_template from easybuild.framework.easyconfig.templates import template_constant_dict -from easybuild.tools.build_log import EasyBuildError +from easybuild.tools.build_log import EasyBuildError, raise_nosupport from easybuild.tools.filetools import change_dir from easybuild.tools.run import run_cmd +def resolve_exts_filter_template(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 + """ + + 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} + + 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: + return (cmd % tmpldict, 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') + + class Extension(object): """ Support for installing extensions. @@ -147,11 +182,8 @@ def sanity_check_step(self): # Get raw value to translate ext_name, ext_version, src exts_filter = self.cfg.get_ref('exts_filter') - if exts_filter is not None: - cmd, inp = exts_filter - else: + if exts_filter is None: self.log.debug("no exts_filter setting found, skipping sanitycheck") - cmd = None if 'modulename' in self.options: modname = self.options['modulename'] @@ -163,18 +195,8 @@ def sanity_check_step(self): # allow skipping of sanity check by setting module name to False if modname is False: self.log.info("modulename set to False for '%s' extension, so skipping sanity check", self.name) - elif cmd: - template = { - 'ext_name': modname, - 'ext_version': self.version, - 'src': self.src, - # TODO same dict is used in easyblock.py skip_extensions, resolve this - } - cmd = cmd % template - - stdin = None - if inp: - stdin = inp % template + elif exts_filter: + cmd, stdin = resolve_exts_filter_template(exts_filter, self) # set log_ok to False so we can catch the error instead of run_cmd (output, ec) = run_cmd(cmd, log_ok=False, simple=False, regexp=False, inp=stdin) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index d1bd7dde35..00033c106a 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -33,6 +33,7 @@ import shutil import sys import tempfile +from inspect import cleandoc from datetime import datetime from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, init_config from unittest import TextTestRunner @@ -787,17 +788,25 @@ def test_skip_extensions_step(self): """Test the skip_extensions_step""" init_config(build_options={'silent': True}) - self.contents = '\n'.join([ - 'easyblock = "ConfigureMake"', - 'name = "pi"', - 'version = "3.14"', - 'homepage = "http://example.com"', - 'description = "test easyconfig"', - 'toolchain = SYSTEM', - 'exts_list = ["ext1", "ext2"]', - 'exts_filter = ("if [ %(ext_name)s == \'ext2\' ]; then exit 0; else exit 1; fi", "")', - 'exts_defaultclass = "DummyExtension"', - ]) + self.contents = cleandoc(""" + easyblock = "ConfigureMake" + name = "pi" + version = "3.14" + homepage = "http://example.com" + description = "test easyconfig" + toolchain = SYSTEM + exts_list = [ + "ext1", + ("ext2", "42", {"source_tmpl": "dummy.tgz"}), + ("ext3", "1.1", {"source_tmpl": "dummy.tgz", "modulename": "real_ext"}), + ] + exts_filter = ("\ + if [ %(ext_name)s == 'ext2' ] && [ %(ext_version)s == '42' ] && [[ %(src)s == *dummy.tgz ]];\ + then exit 0;\ + elif [ %(ext_name)s == 'real_ext' ]; then exit 0;\ + else exit 1; fi", "") + exts_defaultclass = "DummyExtension" + """) # check if skip skips correct extensions self.writeEC() eb = EasyBlock(EasyConfig(self.eb_file)) @@ -806,9 +815,12 @@ def test_skip_extensions_step(self): eb.skip = True eb.extensions_step(fetch=True) # 'ext1' should be in eb.exts - self.assertTrue('ext1' in [y for x in eb.exts for y in x.values()]) + eb_exts = [y for x in eb.exts for y in x.values()] + self.assertTrue('ext1' in eb_exts) # 'ext2' should not - self.assertFalse('ext2' in [y for x in eb.exts for y in x.values()]) + self.assertFalse('ext2' in eb_exts) + # 'ext3' should not + self.assertFalse('ext3' in eb_exts) # cleanup eb.close_log() diff --git a/test/framework/sandbox/sources/p/pi/dummy.tgz b/test/framework/sandbox/sources/p/pi/dummy.tgz new file mode 100644 index 0000000000..e69de29bb2 From c82a2d4a6fbb23db33f834ac8f4770dbd4084e7e Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 17:13:08 +0100 Subject: [PATCH 07/14] Make search for module command less verbose Removes duplicate "Command found" message in check_cmd_avail Removes expected warning message when checking for cmd before using COMMAND_ENVIRONMENT Reasoning: Usually commands 'ml'/'module' are used which internally use LMOD_CMD so lmod does not need to be in path (and rarely is) --- easybuild/tools/filetools.py | 9 ++++++--- easybuild/tools/modules.py | 16 +++++++++------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 5edffe1e8d..e414ed68a7 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -384,12 +384,14 @@ def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced return find_base_dir() -def which(cmd, retain_all=False, check_perms=True): +def which(cmd, retain_all=False, check_perms=True, log_ok=True, log_error=True): """ Return (first) path in $PATH for specified command, or None if command is not found :param retain_all: returns *all* locations to the specified command in $PATH, not just the first one :param check_perms: check whether candidate path has read/exec permissions before accepting it as a match + :param log_ok: Log an info message where the command has been found (if any) + :param log_error: Log a warning message when command hasn't been found """ if retain_all: res = [] @@ -401,7 +403,8 @@ def which(cmd, retain_all=False, check_perms=True): cmd_path = os.path.join(path, cmd) # only accept path if command is there if os.path.isfile(cmd_path): - _log.info("Command %s found at %s", cmd, cmd_path) + if log_ok: + _log.info("Command %s found at %s", cmd, cmd_path) if check_perms: # check if read/executable permissions are available if not os.access(cmd_path, os.R_OK | os.X_OK): @@ -413,7 +416,7 @@ def which(cmd, retain_all=False, check_perms=True): res = cmd_path break - if not res: + if not res and log_error: _log.warning("Could not find command '%s' (with permissions to read/execute it) in $PATH (%s)" % (cmd, paths)) return res diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 7cd65586c0..a39f70d7d1 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -180,16 +180,18 @@ def __init__(self, mod_paths=None, testing=False): if mod_paths is not None: self.set_mod_paths(mod_paths) + cmd_path = which(self.cmd, log_ok=False, log_error=False) # only use command path in environment variable if command in not available in $PATH - if which(self.cmd) is None and env_cmd_path is not None: - self.log.debug("Set %s command via environment variable %s: %s", - self.NAME, self.COMMAND_ENVIRONMENT, self.cmd) - self.cmd = env_cmd_path + if which(self.cmd) is None: + if env_cmd_path is not None: + self.log.debug("Set %s command via environment variable %s: %s", + self.NAME, self.COMMAND_ENVIRONMENT, self.cmd) + self.cmd = env_cmd_path # check whether paths obtained via $PATH and $LMOD_CMD are different - elif which(self.cmd) != env_cmd_path: + elif env_cmd_path and cmd_path != env_cmd_path: self.log.debug("Different paths found for %s command '%s' via which/$PATH and $%s: %s vs %s", - self.NAME, self.COMMAND, self.COMMAND_ENVIRONMENT, self.cmd, env_cmd_path) + self.NAME, self.COMMAND, self.COMMAND_ENVIRONMENT, cmd_path, env_cmd_path) # make sure the module command was found if self.cmd is None: @@ -284,7 +286,7 @@ def set_and_check_version(self): def check_cmd_avail(self): """Check whether modules tool command is available.""" - cmd_path = which(self.cmd) + cmd_path = which(self.cmd, log_ok=False) if cmd_path is not None: self.cmd = cmd_path self.log.info("Full path for %s command is %s, so using it", self.NAME, self.cmd) From 719c79a06222a111fbb7fdb2fe95fa751dc817ec Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 17:39:17 +0100 Subject: [PATCH 08/14] Avoid warnings about missing template values in validate_iterate_opts_lists Some values are not available at this stage, e.g. installdir As we are not interested in the resolved values, simply disable templating --- easybuild/framework/easyconfig/easyconfig.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 1be7049a60..625d355e0d 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -809,6 +809,10 @@ def validate_iterate_opts_lists(self): Configure/build/install options specified as lists should have same length. """ + # Disable templating as only the existance is of interest, the actual resolved value is not + prev_enable_templating = self.enable_templating + self.enable_templating = False + # configure/build/install options may be lists, in case of an iterated build # when lists are used, they should be all of same length # list of length 1 are treated as if it were strings in EasyBlock @@ -821,6 +825,7 @@ def validate_iterate_opts_lists(self): # anticipate changes in available easyconfig parameters (e.g. makeopts -> buildopts?) if self.get(opt, None) is None: + self.enable_templating = prev_enable_templating raise EasyBuildError("%s not available in self.cfg (anymore)?!", opt) # keep track of list, supply first element as first option to handle @@ -830,8 +835,11 @@ def validate_iterate_opts_lists(self): # make sure that options that specify lists have the same length list_opt_lengths = [length for (opt, length) in opt_counts if length > 1] if len(nub(list_opt_lengths)) > 1: + self.enable_templating = prev_enable_templating raise EasyBuildError("Build option lists for iterated build should have same length: %s", opt_counts) + self.enable_templating = prev_enable_templating + return True def start_iterating(self): From 278bf057587a3d0f1e0fcd6ab65071104b2faf14 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 17:45:11 +0100 Subject: [PATCH 09/14] Add IBM POWER9 processor IDs to VENDOR_IDS Taken from https://www.redbooks.ibm.com/redbooks/pdfs/sg248422.pdf --- easybuild/tools/systemtools.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/systemtools.py b/easybuild/tools/systemtools.py index e871dfa24b..1b285e191d 100644 --- a/easybuild/tools/systemtools.py +++ b/easybuild/tools/systemtools.py @@ -114,6 +114,9 @@ 'AuthenticAMD': AMD, 'GenuineIntel': INTEL, 'IBM': IBM, + # IBM POWER9 + '8335-GTH': IBM, + '8335-GTX': IBM, } # ARM Cortex part numbers from the corresponding ARM Processor Technical Reference Manuals, # see http://infocenter.arm.com - Cortex-A series processors, Section "Main ID Register" @@ -276,7 +279,7 @@ def get_cpu_vendor(): if arch == X86_64: vendor_regex = re.compile(r"vendor_id\s+:\s*(\S+)") elif arch == POWER: - vendor_regex = re.compile(r"model\s+:\s*(\w+)") + vendor_regex = re.compile(r"model\s+:\s*((\w|-)+)") elif arch in [AARCH32, AARCH64]: vendor_regex = re.compile(r"CPU implementer\s+:\s*(\S+)") From ed6b726cb87a3c22e19225f1c5db765d981c3095 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2019 17:48:55 +0100 Subject: [PATCH 10/14] Avoid warnings about missing template values in det_iter_cnt We just need to number of elements, not their values --- easybuild/framework/easyblock.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index c019ec502c..f970465e09 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1576,8 +1576,9 @@ def post_iter_step(self): def det_iter_cnt(self): """Determine iteration count based on configure/build/install options that may be lists.""" - iter_opt_counts = [len(self.cfg[opt]) for opt in ITERATE_OPTIONS - if opt not in ['builddependencies'] and isinstance(self.cfg[opt], (list, tuple))] + # Using get_ref to avoid template substitution (and possible failures) + iter_opt_counts = [len(self.cfg.get_ref(opt)) for opt in ITERATE_OPTIONS + if opt not in ['builddependencies'] and isinstance(self.cfg.get_ref(opt), (list, tuple))] # we need to take into account that builddependencies is always a list # we're only iterating over it if it's a list of lists From f40844255ada50ec41f83766033b4800b386ed47 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 13 Dec 2019 10:18:56 +0100 Subject: [PATCH 11/14] Don't warn when not finding an option starting with _opt_ This happens for e.g. _opt_CC and friends and is allowed to be empty/unset according to the code in the setter function. This avoids half a dozen warnings per software part installed --- easybuild/tools/toolchain/options.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/toolchain/options.py b/easybuild/tools/toolchain/options.py index 3049643f62..42da8e4980 100644 --- a/easybuild/tools/toolchain/options.py +++ b/easybuild/tools/toolchain/options.py @@ -86,7 +86,12 @@ def option(self, name, templatedict=None): """Return option value""" value = self.get(name, None) if value is None and name not in self.options_map: - self.log.warning("option: option with name %s returns None" % name) + msg = "option: option with name %s returns None" % name + # Empty options starting with _opt_ are allowed, so don't warn + if name.startswith('_opt_'): + self.log.devel(msg) + else: + self.log.warning(msg) res = None elif name in self.options_map: res = self.options_map[name] From 7553dbddd8402e3fdb8ca40e0a5fa2f78ae62c5d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 16 Dec 2019 10:02:45 +0100 Subject: [PATCH 12/14] Review fixes --- easybuild/framework/easyconfig/easyconfig.py | 22 ++---- easybuild/framework/extension.py | 7 +- easybuild/tools/build_log.py | 1 + easybuild/tools/modules.py | 25 +++--- test/framework/build_log.py | 16 ++-- test/framework/easyconfig.py | 80 ++++++++++++++++---- 6 files changed, 102 insertions(+), 49 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 625d355e0d..b3e8af1cb8 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -809,10 +809,6 @@ def validate_iterate_opts_lists(self): Configure/build/install options specified as lists should have same length. """ - # Disable templating as only the existance is of interest, the actual resolved value is not - prev_enable_templating = self.enable_templating - self.enable_templating = False - # configure/build/install options may be lists, in case of an iterated build # when lists are used, they should be all of same length # list of length 1 are treated as if it were strings in EasyBlock @@ -820,26 +816,23 @@ def validate_iterate_opts_lists(self): for opt in ITERATE_OPTIONS: # only when builddependencies is a list of lists are we iterating over them - if opt == 'builddependencies' and not all(isinstance(e, list) for e in self[opt]): + if opt == 'builddependencies' and not all(isinstance(e, list) for e in self.get_ref(opt)): continue + opt_value = self.get(opt, None, resolve=False) # anticipate changes in available easyconfig parameters (e.g. makeopts -> buildopts?) - if self.get(opt, None) is None: - self.enable_templating = prev_enable_templating + if opt_value is None: raise EasyBuildError("%s not available in self.cfg (anymore)?!", opt) # keep track of list, supply first element as first option to handle - if isinstance(self[opt], (list, tuple)): - opt_counts.append((opt, len(self[opt]))) + if isinstance(opt_value, (list, tuple)): + opt_counts.append((opt, len(opt_value))) # make sure that options that specify lists have the same length list_opt_lengths = [length for (opt, length) in opt_counts if length > 1] if len(nub(list_opt_lengths)) > 1: - self.enable_templating = prev_enable_templating raise EasyBuildError("Build option lists for iterated build should have same length: %s", opt_counts) - self.enable_templating = prev_enable_templating - return True def start_iterating(self): @@ -1531,12 +1524,13 @@ def __setitem__(self, key, value): key, value) @handle_deprecated_or_replaced_easyconfig_parameters - def get(self, key, default=None): + def get(self, key, default=None, resolve=True): """ Gets the value of a key in the config, with 'default' as fallback. + :param resolve: if False, disables templating via calling get_ref, else resolves template values """ if key in self: - return self[key] + return self[key] if resolve else self.get_ref(key) else: return default diff --git a/easybuild/framework/extension.py b/easybuild/framework/extension.py index f516511125..a27f81dd47 100644 --- a/easybuild/framework/extension.py +++ b/easybuild/framework/extension.py @@ -41,6 +41,7 @@ from easybuild.tools.build_log import EasyBuildError, raise_nosupport from easybuild.tools.filetools import change_dir from easybuild.tools.run import run_cmd +from easybuild.tools.py2vs3 import string_type def resolve_exts_filter_template(exts_filter, ext): @@ -51,7 +52,7 @@ def resolve_exts_filter_template(exts_filter, ext): :return (cmd, input) as a tuple of strings """ - if isinstance(exts_filter, str) or len(exts_filter) != 2: + if isinstance(exts_filter, string_type) or len(exts_filter) != 2: raise EasyBuildError('exts_filter should be a list or tuple of ("command","input")') cmd, cmdinput = exts_filter @@ -71,11 +72,13 @@ def resolve_exts_filter_template(exts_filter, ext): } try: - return (cmd % tmpldict, cmdinput % tmpldict if cmdinput else None) + 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 class Extension(object): diff --git a/easybuild/tools/build_log.py b/easybuild/tools/build_log.py index 71a9d389bc..616c839531 100644 --- a/easybuild/tools/build_log.py +++ b/easybuild/tools/build_log.py @@ -95,6 +95,7 @@ def raise_nosupport(msg, ver): nosupport_msg = "NO LONGER SUPPORTED since v%s: %s; see %s for more information" raise_easybuilderror(nosupport_msg, ver, msg, DEPRECATED_DOC_URL) + class EasyBuildLog(fancylogger.FancyLogger): """ The EasyBuild logger, with its own error and exception functions. diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index a39f70d7d1..038c689c14 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -180,18 +180,17 @@ def __init__(self, mod_paths=None, testing=False): if mod_paths is not None: self.set_mod_paths(mod_paths) - cmd_path = which(self.cmd, log_ok=False, log_error=False) - # only use command path in environment variable if command in not available in $PATH - if which(self.cmd) is None: - if env_cmd_path is not None: + if env_cmd_path: + cmd_path = which(self.cmd, log_ok=False, log_error=False) + # only use command path in environment variable if command in not available in $PATH + if cmd_path is None: self.log.debug("Set %s command via environment variable %s: %s", self.NAME, self.COMMAND_ENVIRONMENT, self.cmd) self.cmd = env_cmd_path - - # check whether paths obtained via $PATH and $LMOD_CMD are different - elif env_cmd_path and cmd_path != env_cmd_path: - self.log.debug("Different paths found for %s command '%s' via which/$PATH and $%s: %s vs %s", - self.NAME, self.COMMAND, self.COMMAND_ENVIRONMENT, cmd_path, env_cmd_path) + # check whether paths obtained via $PATH and $LMOD_CMD are different + elif cmd_path != env_cmd_path: + self.log.debug("Different paths found for %s command '%s' via which/$PATH and $%s: %s vs %s", + self.NAME, self.COMMAND, self.COMMAND_ENVIRONMENT, cmd_path, env_cmd_path) # make sure the module command was found if self.cmd is None: @@ -676,7 +675,7 @@ def modulefile_path(self, mod_name, strip_ext=False): :param strip_ext: strip (.lua) extension from module fileame (if present)""" # (possible relative) path is always followed by a ':', and may be prepended by whitespace # this works for both environment modules and Lmod - modpath_re = re.compile('^\s*(?P[^/\n]*/[^\s]+):$', re.M) + modpath_re = re.compile(r'^\s*(?P[^/\n]*/[^\s]+):$', re.M) modpath = self.get_value_from_modulefile(mod_name, modpath_re) if strip_ext and modpath.endswith('.lua'): @@ -941,7 +940,7 @@ def file_join(res): """Helper function to compose joined path.""" return os.path.join(*[x.strip('"') for x in res.groups()]) - res = re.sub('\[\s+file\s+join\s+(.*)\s+(.*)\s+\]', file_join, res) + res = re.sub(r'\[\s+file\s+join\s+(.*)\s+(.*)\s+\]', file_join, res) # also interpret all $env(...) parts res = re.sub(r'\$env\((?P[^)]*)\)', lambda res: os.getenv(res.group('key'), ''), res) @@ -1160,7 +1159,7 @@ def run_module(self, *args, **kwargs): # this is required for the DEISA variant of modulecmd.tcl which is commonly used def tweak_stdout(txt): """Tweak stdout before it's exec'ed as Python code.""" - modulescript_regex = "^exec\s+[\"'](?P/tmp/modulescript_[0-9_]+)[\"']$" + modulescript_regex = r"^exec\s+[\"'](?P/tmp/modulescript_[0-9_]+)[\"']$" return re.sub(modulescript_regex, r"execfile('\1')", txt) tweak_stdout_fn = None @@ -1368,7 +1367,7 @@ def module_wrapper_exists(self, mod_name): # first consider .modulerc.lua with Lmod 7.8 (or newer) if StrictVersion(self.version) >= StrictVersion('7.8'): - mod_wrapper_regex_template = '^module_version\("(?P.*)", "%s"\)$' + mod_wrapper_regex_template = r'^module_version\("(?P.*)", "%s"\)$' res = super(Lmod, self).module_wrapper_exists(mod_name, modulerc_fn='.modulerc.lua', mod_wrapper_regex_template=mod_wrapper_regex_template) diff --git a/test/framework/build_log.py b/test/framework/build_log.py index 5fd925ecd1..7af9e623f1 100644 --- a/test/framework/build_log.py +++ b/test/framework/build_log.py @@ -36,8 +36,9 @@ from unittest import TextTestRunner from easybuild.base.fancylogger import getLogger, logToFile, setLogFormat -from easybuild.tools.build_log import LOGGING_FORMAT, EasyBuildError, EasyBuildLog, dry_run_msg, dry_run_warning -from easybuild.tools.build_log import init_logging, print_error, print_msg, print_warning, stop_logging, time_str_since +from easybuild.tools.build_log import ( + LOGGING_FORMAT, EasyBuildError, EasyBuildLog, dry_run_msg, dry_run_warning, init_logging, print_error, print_msg, + print_warning, stop_logging, time_str_since, raise_nosupport) from easybuild.tools.filetools import read_file, write_file @@ -146,9 +147,10 @@ def test_easybuildlog(self): logtxt_regex = re.compile(r'^%s' % expected_logtxt, re.M) self.assertTrue(logtxt_regex.search(logtxt), "Pattern '%s' found in %s" % (logtxt_regex.pattern, logtxt)) - self.assertErrorRegex(EasyBuildError, "DEPRECATED \(since .*: kaput", log.deprecated, "kaput", older_ver) - self.assertErrorRegex(EasyBuildError, "DEPRECATED \(since .*: 2>1", log.deprecated, "2>1", '2.0', '1.0') - self.assertErrorRegex(EasyBuildError, "DEPRECATED \(since .*: 2>1", log.deprecated, "2>1", '2.0', max_ver='1.0') + self.assertErrorRegex(EasyBuildError, r"DEPRECATED \(since .*: kaput", log.deprecated, "kaput", older_ver) + self.assertErrorRegex(EasyBuildError, r"DEPRECATED \(since .*: 2>1", log.deprecated, "2>1", '2.0', '1.0') + self.assertErrorRegex(EasyBuildError, r"DEPRECATED \(since .*: 2>1", log.deprecated, "2>1", '2.0', + max_ver='1.0') # wipe log so we can reuse it write_file(tmplog, '') @@ -422,6 +424,10 @@ def test_init_logging(self): stop_logging(logfile, logtostdout=True) + def test_raise_nosupport(self): + self.assertErrorRegex(EasyBuildError, 'NO LONGER SUPPORTED since v42: foobar;', + raise_nosupport, 'foobar', 42) + def suite(): """ returns all the testcases in this module """ diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index f362632e36..f956900f59 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -58,6 +58,7 @@ from easybuild.framework.easyconfig.tools import categorize_files_by_type, check_sha256_checksums, dep_graph from easybuild.framework.easyconfig.tools import find_related_easyconfigs, get_paths_for, parse_easyconfigs from easybuild.framework.easyconfig.tweak import obtain_ec_for, tweak_one +from easybuild.framework.extension import resolve_exts_filter_template from easybuild.toolchains.system import SystemToolchain from easybuild.tools.build_log import EasyBuildError from easybuild.tools.config import module_classes @@ -198,7 +199,7 @@ def test_validation(self): # introduce "TypeError: format requires mapping" issue" self.contents = self.contents.replace("syntax_error'", "foo = '%(name)s %s' % version") self.prep() - error_pattern = "Parsing easyconfig file failed: format requires a mapping \(line 8\)" + error_pattern = r"Parsing easyconfig file failed: format requires a mapping \(line 8\)" self.assertErrorRegex(EasyBuildError, error_pattern, EasyConfig, self.eb_file) def test_system_toolchain_constant(self): @@ -854,7 +855,7 @@ def test_obtain_easyconfig(self): # hidden dependencies must be included in list of dependencies res = obtain_ec_for(specs, [self.test_prefix], None) self.assertEqual(res[0], True) - error_pattern = "Hidden deps with visible module names .* not in list of \(build\)dependencies: .*" + error_pattern = r"Hidden deps with visible module names .* not in list of \(build\)dependencies: .*" self.assertErrorRegex(EasyBuildError, error_pattern, EasyConfig, res[1]) remove_file(res[1]) @@ -1559,7 +1560,6 @@ def test_quote_str(self): 'foo': '"foo"', 'foo\'bar': '"foo\'bar"', 'foo\'bar"baz': '"""foo\'bar"baz"""', - "foo'bar\"baz": '"""foo\'bar"baz"""', "foo\nbar": '"foo\nbar"', 'foo bar': '"foo bar"' } @@ -2499,9 +2499,9 @@ def test_copy_easyconfigs(self): # verify whether copied easyconfig gets cleaned up (stripping out 'Built with' comment + build stats) txt = read_file(copied_toy_ec) regexs = [ - "# Built with EasyBuild", - "# Build statistics", - "buildstats\s*=", + r"# Built with EasyBuild", + r"# Build statistics", + r"buildstats\s*=", ] for regex in regexs: regex = re.compile(regex, re.M) @@ -2708,8 +2708,8 @@ def test_hidden_toolchain(self): '--dry-run', ] outtxt = self.eb_main(args, raise_error=True) - self.assertTrue(re.search('module: GCC/\.4\.9\.2', outtxt)) - self.assertTrue(re.search('module: gzip/1\.6-GCC-4\.9\.2', outtxt)) + self.assertTrue(re.search(r'module: GCC/\.4\.9\.2', outtxt)) + self.assertTrue(re.search(r'module: gzip/1\.6-GCC-4\.9\.2', outtxt)) def test_categorize_files_by_type(self): """Test categorize_files_by_type""" @@ -2853,8 +2853,8 @@ def test_verify_easyconfig_filename(self): # incorrect spec specs['versionsuffix'] = '' error_pattern = "filename '%s' does not match with expected filename 'toy-0.0-gompi-2018a.eb' " % toy_ec_name - error_pattern += "\(specs: name: 'toy'; version: '0.0'; versionsuffix: ''; " - error_pattern += "toolchain name, version: 'gompi', '2018a'\)" + error_pattern += r"\(specs: name: 'toy'; version: '0.0'; versionsuffix: ''; " + error_pattern += r"toolchain name, version: 'gompi', '2018a'\)" self.assertErrorRegex(EasyBuildError, error_pattern, verify_easyconfig_filename, toy_ec, specs) specs['versionsuffix'] = '-test' @@ -2863,8 +2863,8 @@ def test_verify_easyconfig_filename(self): toy_ec = os.path.join(self.test_prefix, 'toy.eb') write_file(toy_ec, toy_txt) error_pattern = "filename 'toy.eb' does not match with expected filename 'toy-0.0-gompi-2018a-test.eb' " - error_pattern += "\(specs: name: 'toy'; version: '0.0'; versionsuffix: '-test'; " - error_pattern += "toolchain name, version: 'gompi', '2018a'\)" + error_pattern += r"\(specs: name: 'toy'; version: '0.0'; versionsuffix: '-test'; " + error_pattern += r"toolchain name, version: 'gompi', '2018a'\)" self.assertErrorRegex(EasyBuildError, error_pattern, verify_easyconfig_filename, toy_ec, specs) # incorrect file contents @@ -2998,7 +2998,7 @@ def test_check_sha256_checksums(self): toy_ec = os.path.join(test_ecs_dir, 't', 'toy', 'toy-0.0.eb') toy_ec_txt = read_file(toy_ec) - checksums_regex = re.compile('^checksums = \[\[(.|\n)*\]\]', re.M) + checksums_regex = re.compile(r'^checksums = \[\[(.|\n)*\]\]', re.M) # wipe out specified checksums, to make check fail test_ec = os.path.join(self.test_prefix, 'toy-0.0-fail.eb') @@ -3013,7 +3013,7 @@ def test_check_sha256_checksums(self): self.assertTrue(res[0].startswith('Checksums missing for one or more sources/patches in toy-0.0-fail.eb')) # test use of whitelist regex patterns: check passes because easyconfig is whitelisted by filename - for regex in ['toy-.*', '.*-0\.0-fail\.eb']: + for regex in [r'toy-.*', r'.*-0\.0-fail\.eb']: res = check_sha256_checksums(ecs, whitelist=[regex]) self.assertFalse(res) @@ -3033,7 +3033,7 @@ def test_check_sha256_checksums(self): # re-test with right checksum in place toy_sha256 = '44332000aa33b99ad1e00cbd1a7da769220d74647060a10e807b916d73ea27bc' test_ec_txt = checksums_regex.sub('checksums = ["%s"]' % toy_sha256, toy_ec_txt) - test_ec_txt = re.sub('patches = \[(.|\n)*\]', '', test_ec_txt) + test_ec_txt = re.sub(r'patches = \[(.|\n)*\]', '', test_ec_txt) test_ec = os.path.join(self.test_prefix, 'toy-0.0-ok.eb') write_file(test_ec, test_ec_txt) @@ -3557,6 +3557,56 @@ 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.""" + class TestExtension(object): + def __init__(self, values): + self.name = values['name'] + self.version = values.get('version') + self.src = values.get('src') + self.options = values.get('options', {}) + + error_msg = 'exts_filter should be a list or tuple' + self.assertErrorRegex(EasyBuildError, error_msg, resolve_exts_filter_template, + '[ 1 == 1 ]', {}) + self.assertErrorRegex(EasyBuildError, error_msg, resolve_exts_filter_template, + ['[ 1 == 1 ]'], {}) + self.assertErrorRegex(EasyBuildError, error_msg, resolve_exts_filter_template, + ['[ 1 == 1 ]', 'true', 'false'], {}) + + test_cases = [ + # Minimal case: just name + (['%(ext_name)s', None], + {'name': 'foo'}, + ('foo', None), + ), + # Minimal case with input + (['%(ext_name)s', '>%(ext_name)s'], + {'name': 'foo'}, + ('foo', '>foo'), + ), + # All values + (['%(ext_name)s-%(ext_version)s-%(src)s', '>%(ext_name)s-%(ext_version)s-%(src)s'], + {'name': 'foo', 'version': 42, 'src': 'bar.tgz'}, + ('foo-42-bar.tgz', '>foo-42-bar.tgz'), + ), + # options dict is accepted + (['%(ext_name)s-%(ext_version)s-%(src)s', '>%(ext_name)s-%(ext_version)s-%(src)s'], + {'name': 'foo', 'version': 42, 'src': 'bar.tgz', 'options': {'dummy': 'value'}}, + ('foo-42-bar.tgz', '>foo-42-bar.tgz'), + ), + # modulename overwrites name + (['%(ext_name)s-%(ext_version)s-%(src)s', '>%(ext_name)s-%(ext_version)s-%(src)s'], + {'name': 'foo', 'version': 42, 'src': 'bar.tgz', 'options': {'modulename': 'baz'}}, + ('baz-42-bar.tgz', '>baz-42-bar.tgz'), + ), + ] + 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) + def suite(): """ returns all the testcases in this module """ From 5c4d4b791f635860be684c411848e869a9099ca1 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 14 Jan 2020 14:17:38 +0100 Subject: [PATCH 13/14] Fix log message of cmd set from env --- easybuild/tools/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 038c689c14..8bfc737bc3 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -184,9 +184,9 @@ def __init__(self, mod_paths=None, testing=False): cmd_path = which(self.cmd, log_ok=False, log_error=False) # only use command path in environment variable if command in not available in $PATH if cmd_path is None: + self.cmd = env_cmd_path self.log.debug("Set %s command via environment variable %s: %s", self.NAME, self.COMMAND_ENVIRONMENT, self.cmd) - self.cmd = env_cmd_path # check whether paths obtained via $PATH and $LMOD_CMD are different elif cmd_path != env_cmd_path: self.log.debug("Different paths found for %s command '%s' via which/$PATH and $%s: %s vs %s", From 1e13f9c0db29d9f8d24c561b2f2c45deb9d4193b Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 14 Jan 2020 15:22:00 +0100 Subject: [PATCH 14/14] Update comment --- easybuild/framework/easyblock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index f970465e09..300f474217 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1576,7 +1576,7 @@ def post_iter_step(self): def det_iter_cnt(self): """Determine iteration count based on configure/build/install options that may be lists.""" - # Using get_ref to avoid template substitution (and possible failures) + # Using get_ref to avoid resolving templates as their required attributes may not be available yet iter_opt_counts = [len(self.cfg.get_ref(opt)) for opt in ITERATE_OPTIONS if opt not in ['builddependencies'] and isinstance(self.cfg.get_ref(opt), (list, tuple))]