Make generic EasyBlock usable to install software #4531
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2720,20 +2720,72 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True): | ||
| if start_dir: | ||
| self.guess_start_dir() | ||
| def _run_cmds(self, cmds): | ||
| """Execute list of shell commands""" | ||
| self.log.debug(f"List of commands to be executed: {cmds}") | ||
| # make sure we have a list of commands | ||
| if not isinstance(cmds, (list, tuple)): | ||
| raise EasyBuildError( | ||
| f"Invalid argument for EasyBlock._run_cmds, expected list or tuple of strings: {cmds}" | ||
| ) | ||
| for cmd in cmds: | ||
| if not isinstance(cmd, str): | ||
| raise EasyBuildError(f"Invalid element in command list, not a string: {cmd}") | ||
| run_shell_cmd(cmd) | ||
| def run_core_step_cmd(self, step): | ||
| """Construct main command of core step and execute it""" | ||
| core_steps = ["configure", "build", "test", "install"] | ||
| if step not in core_steps: | ||
| raise EasyBuildError( | ||
| f"Cannot construct command for step {step}. Supported steps are: {', '.join(core_steps)}." | ||
| ) | ||
| step_cmd_cfgs = { | ||
| "configure": ('pre_configure_cmds', 'preconfigopts', 'configure_cmd', 'configopts'), | ||
| "build": ('pre_build_cmds', 'prebuildopts', 'build_cmd', 'buildopts'), | ||
| "test": ('pre_test_cmds', 'pretestopts', 'test_cmd', 'testopts'), | ||
| "install": ('pre_install_cmds', 'preinstallopts', 'install_cmd', 'installopts'), | ||
| } | ||
| pre_step_cmds = self.cfg[step_cmd_cfgs[step][0]] | ||
| if pre_step_cmds: | ||
| try: | ||
| self._run_cmds(pre_step_cmds) | ||
| except EasyBuildError as err: | ||
| raise EasyBuildError(f"Failed to execute pre-step commands for step '{step}'. {err}") | ||
| step_cmd = ' '.join(str(self.cfg[cfg_name]) for cfg_name in step_cmd_cfgs[step][1:4]) | ||
| return run_shell_cmd(step_cmd) | ||
| def configure_step(self): | ||
| """Configure build.""" | ||
| if self.cfg['configure_cmd'] is not None: | ||
| res = self.run_core_step_cmd("configure") | ||
| return res.output | ||
| def build_step(self): | ||
| """Build software.""" | ||
| if self.cfg['build_cmd'] is not None: | ||
| res = self.run_core_step_cmd("build") | ||
| return res.output | ||
| def test_step(self): | ||
| """Run unit tests provided by software (if any).""" | ||
| if self.cfg['runtest'] is not None: | ||
| self.log.deprecated("Easyconfig parameter 'runtest' is deprecated, use 'test_cmd' instead.", "6.0") | ||
There was a problem hiding this comment. @lexming This implies updating all easyblocks/easyconfigs to use There was a problem hiding this comment. Also, what if both Either we error out, or we let one overrule the other (with logging). There was a problem hiding this comment. good point, I suggest to not error out and just ignore | ||
| if self.cfg['test_cmd'] is None: | ||
| self.cfg['test_cmd'] = self.cfg['runtest'] | ||
| else: | ||
| self.log.warning( | ||
There was a problem hiding this comment. Shouldn't we print a warning here (too), to avoid that this is overlooked easily? | ||
| "Both 'runtest' and 'test_cmd' easyconfig parameters are defined. Ignoring deprecated 'runtest'." | ||
| ) | ||
| if self.cfg['test_cmd'] is not None: | ||
| res = self.run_core_step_cmd("test") | ||
| return res.output | ||
| def _test_step(self): | ||
| @@ -2750,8 +2802,10 @@ def stage_install_step(self): | ||
| """Install in a stage directory before actual installation.""" | ||
| def install_step(self): | ||
| """Install built software.""" | ||
| if self.cfg['install_cmd'] is not None: | ||
| res = self.run_core_step_cmd("install") | ||
| return res.output | ||
| def init_ext_instances(self): | ||
| """ | ||
| @@ -2810,8 +2864,8 @@ def init_ext_instances(self): | ||
| error_on_missing_easyblock=False) | ||
| self.log.debug("Obtained class %s for extension %s", cls, ext_name) | ||
| if cls not in (None, EasyBlock): | ||
| # extensions need a non-default EasyBlock capable of installing extensions | ||
| if not issubclass(cls, Extension): | ||
| raise EasyBuildError("%s easyblock can not be used to install extensions!", cls.__name__) | ||
| @@ -2992,18 +3046,10 @@ def run_post_install_commands(self, commands=None): | ||
| if commands is None: | ||
| commands = self.cfg['postinstallcmds'] | ||
| try: | ||
| self._run_cmds(commands) | ||
| except EasyBuildError as err: | ||
| raise EasyBuildError(f"Failed to execute post-install commands. {err}") | ||
| def apply_post_install_es(self, es=None): | ||
| """ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -103,8 +103,10 @@ | ||
| try: | ||
| import autopep8 | ||
| HAVE_AUTOPEP8 = True | ||
| except ImportError as import_err: | ||
| _log.warning( | ||
| "Failed to import autopep8, dumping easyconfigs with reformatting enabled will not work: %s", import_err | ||
| ) | ||
| HAVE_AUTOPEP8 = False | ||
| @@ -1858,83 +1860,81 @@ def det_installversion(version, toolchain_name, toolchain_version, prefix, suffi | ||
| _log.nosupport('Use det_full_ec_version from easybuild.tools.module_generator instead of %s' % old_fn, '2.0') | ||
There was a problem hiding this comment. Don't remove At best deprecate it There was a problem hiding this comment. Well, we can actually break the API between major versions 😉 I re-added it with a deprecation warning nonetheless. Done in eacd6e3 | ||
| def get_easyblock_class(easyblock, name=None, error_on_failed_import=False, error_on_missing_easyblock=True, **kwargs): | ||
| """ | ||
| Get class for a particular easyblock (or use default) | ||
| """ | ||
| modulepath, class_name = None, None | ||
| if error_on_failed_import: | ||
| _log.deprecated( | ||
| "Use of error_on_failed_import argument in Easyconfig.get_easyblock_class is deprecated, " | ||
| "use 'error_on_missing_easyblock' instead", | ||
| "6.0" | ||
| ) | ||
| if easyblock: | ||
| # something was specified, lets parse it | ||
| easyblock_spec = easyblock.split('.') | ||
| class_name = easyblock_spec.pop(-1) | ||
| # figure out if full path was specified or not | ||
| if easyblock_spec: | ||
| modulepath = '.'.join(easyblock_spec) | ||
| _log.info( | ||
| f"Assuming a full easyblock module path specification (class: {class_name}, modulepath: {modulepath})" | ||
| ) | ||
| else: | ||
| modulepath = get_module_path(easyblock) | ||
| _log.info(f"Derived full easyblock module path for {class_name}: {modulepath}") | ||
| else: | ||
| # if no easyblock specified, try to find if one exists | ||
| if name is None: | ||
| name = "UNKNOWN" | ||
| # The following is a generic way to calculate unique class names for any funny software title | ||
| class_name = encode_class_name(name) | ||
| # modulepath will be the namespace + encoded modulename (from the classname) | ||
| modulepath = get_module_path(class_name, generic=False) | ||
| try: | ||
| __import__(modulepath, globals(), locals(), ['']) | ||
| except ImportError as err: | ||
| _log.debug(f"Failed to import easyblock module '{modulepath}' (derived from easyconfig name): {err}") | ||
| # fallback to generic EasyBlock for easyconfigs | ||
| # without any easyblock specification or no specific easyblock | ||
| class_name = "EasyBlock" | ||
| modulepath = "easybuild.framework.easyblock" | ||
| _log.info(f"Using generic EasyBlock module for software '{name}'") | ||
| else: | ||
| _log.debug(f"Module path '{modulepath}' found (derived from easyconfig name)") | ||
| try: | ||
| _log.debug(f"Importing easyblock class {class_name} from {modulepath}") | ||
| cls = get_class_for(modulepath, class_name) | ||
| except ImportError as err: | ||
| # when an ImportError occurs, make sure that it's caused by not finding the easyblock module, | ||
| # and not because of a broken import statement in the easyblock module | ||
| modname = modulepath.replace('easybuild.easyblocks.', '') | ||
| error_re = re.compile(rf"No module named '?.*/?{modname}'?") | ||
| _log.debug(f"Error regexp for ImportError on '{modname}' easyblock: {error_re.pattern}") | ||
| if error_re.match(str(err)): | ||
| if error_on_missing_easyblock: | ||
| raise EasyBuildError(f"Software-specific easyblock '{class_name}' not found for {name}") | ||
| elif error_on_failed_import: | ||
| raise EasyBuildError(f"Failed to import '{class_name}' easyblock: {err}") | ||
Comment on lines +1922 to +1923 There was a problem hiding this comment. This doesn't belong here, in this case the easyblock doesn't exist? | ||
| else: | ||
| _log.debug(f"Easyblock for {class_name} not found, but ignoring it: {err}") | ||
| else: | ||
There was a problem hiding this comment. keep the | ||
| raise EasyBuildError(f"Failed to import {class_name} easyblock: {err}") | ||
| except EasyBuildError as err: | ||
| # simply reraise rather than wrapping it into another error | ||
| raise err | ||
| except Exception as err: | ||
| raise EasyBuildError(f"Failed to obtain class for {easyblock} easyblock (not available?): {err}") | ||
| else: | ||
| _log.info( | ||
| f"Successfully obtained class '{cls.__name__}' for easyblock '{easyblock}' (software name '{name}')" | ||
| ) | ||
| return cls | ||
| def get_module_path(name, generic=None, decode=True): | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we limit the usefulness of
pre_configure_cmdsby running those commands separately?Something like
export FOO=barinpre_configure_cmdswon't work as expected because it's run in a different shell thanconfigure_cmd, which limits the usefulness ofpre_configure_cmdsquite a bit I think?I can see the value though (better errors when one of the commands in
pre_configure_cmdsfails, for example), but we should try and make it very clear that these commands are running in a different subshell?