-
Notifications
You must be signed in to change notification settings - Fork 219
Make generic EasyBlock usable to install software #4531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 25 commits
5bf48e4
9781455
6be1579
95fed87
b51c586
6233cb4
32ca373
5f370cd
7eafeb7
37d0edc
39be30a
ac2fa7a
9655dfb
74bf3a9
3834170
b6d9b3b
cea4bcd
2b39e93
0b73ef5
6add7bc
2564be9
69aa521
164e3c6
041a397
a1a58c1
31f9a93
b879130
22d9648
e05a985
eacd6e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2720,20 +2720,66 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True): | |||||||||||||||||
| if start_dir: | ||||||||||||||||||
| self.guess_start_dir() | ||||||||||||||||||
|
|
||||||||||||||||||
| def _run_command_stack(self, commands, caller): | ||||||||||||||||||
| """Execute stack of commands in a shell""" | ||||||||||||||||||
| self.log.debug(f"Stack of commands to be executed: {commands}") | ||||||||||||||||||
|
|
||||||||||||||||||
| # make sure we have a list of commands | ||||||||||||||||||
| if not isinstance(commands, (list, tuple)): | ||||||||||||||||||
| error_msg = f"Invalid value for '{caller}', should be list or tuple of strings: {commands}" | ||||||||||||||||||
| raise EasyBuildError(error_msg) | ||||||||||||||||||
|
|
||||||||||||||||||
| for cmd in commands: | ||||||||||||||||||
| if not isinstance(cmd, str): | ||||||||||||||||||
| raise EasyBuildError(f"Invalid element in '{caller}', not a string: {cmd}") | ||||||||||||||||||
| run_shell_cmd(cmd) | ||||||||||||||||||
|
|
||||||||||||||||||
| def run_step_main_action(self, action, step_name, pre_cmds=None, pre_opts="", post_opts=""): | ||||||||||||||||||
| """Construct main command of step and execute it""" | ||||||||||||||||||
| if pre_cmds: | ||||||||||||||||||
| self._run_command_stack(pre_cmds, f"pre_{step_name}_cmds") | ||||||||||||||||||
|
|
||||||||||||||||||
| step_cmd = f"{pre_opts} {action} {post_opts}" | ||||||||||||||||||
| return run_shell_cmd(step_cmd) | ||||||||||||||||||
|
|
||||||||||||||||||
| def configure_step(self): | ||||||||||||||||||
| """Configure build (abstract method).""" | ||||||||||||||||||
| raise NotImplementedError | ||||||||||||||||||
| """Configure build.""" | ||||||||||||||||||
| if self.cfg['configure_cmd'] is not None: | ||||||||||||||||||
| res = self.run_step_main_action( | ||||||||||||||||||
| self.cfg['configure_cmd'], | ||||||||||||||||||
| "configure", | ||||||||||||||||||
| self.cfg['pre_configure_cmds'], | ||||||||||||||||||
| self.cfg['preconfigopts'], | ||||||||||||||||||
| self.cfg['configopts'], | ||||||||||||||||||
|
||||||||||||||||||
| "configure", | |
| self.cfg['pre_configure_cmds'], | |
| self.cfg['preconfigopts'], | |
| self.cfg['configopts'], | |
| 'configure', | |
| pre_cmds=self.cfg['pre_configure_cmds'], | |
| pre_opts=self.cfg['preconfigopts'], | |
| post_opts=self.cfg['configopts'], |
Even better would be to use only named options, so also use action='self.cfg['configure_cmd']' and action='configure' (and making those both None by default in run_step_main_action and mandatory with an is None check in there.
Also, maybe rename run_step_main_action to run_core_step, and only pass a string like configure, let run_core_step figure out the rest (to use configure_cmd, preconfigopts, etc.)
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.
good idea, it is indeed easy to construct the commands from just the step name. Done in e05a985
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.
@lexming This implies updating all easyblocks/easyconfigs to use test_cmd instead of runtest before we release EasyBuild 5.0, so please open an issue on that in both repos and tag it for EasyBuild 5.0
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.
Also, what if both test_cmd and runtest are set?
Either we error out, or we let one overrule the other (with logging).
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.
good point, I suggest to not error out and just ignore runtest if both are defined (with a warning ofc). Fixed in 22d9648
lexming marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,9 +88,11 @@ | |
| "to be linked in any installed binary/library", BUILD], | ||
| 'bitbucket_account': ['%(namelower)s', "Bitbucket account name to be used to resolve template values in source" | ||
| " URLs", BUILD], | ||
| 'buildopts': ['', 'Extra options passed to make step (default already has -j X)', BUILD], | ||
| 'buildopts': ['', 'Extra options appended to build command', BUILD], | ||
| 'build_cmd': [None, "Main shell command used in the build step", BUILD], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also mention something like " |
||
| 'checksums': [[], "Checksums for sources and patches", BUILD], | ||
| 'configopts': ['', 'Extra options passed to configure (default already has --prefix)', BUILD], | ||
| 'configopts': ['', 'Extra options appended to configure command', BUILD], | ||
| 'configure_cmd': [None, "Main shell command used in the configure step", BUILD], | ||
| 'cuda_compute_capabilities': [[], "List of CUDA compute capabilities to build with (if supported)", BUILD], | ||
| 'download_instructions': ['', "Specify steps to acquire necessary file, if obtaining it is difficult", BUILD], | ||
| 'easyblock': [None, "EasyBlock to use for building; if set to None, an easyblock is selected " | ||
|
|
@@ -107,18 +109,23 @@ | |
| 'github_account': ['%(namelower)s', "GitHub account name to be used to resolve template values in source URLs", | ||
| BUILD], | ||
| 'hidden': [False, "Install module file as 'hidden' by prefixing its version with '.'", BUILD], | ||
| 'installopts': ['', 'Extra options for installation', BUILD], | ||
| 'installopts': ['', 'Extra options appended to installation command', BUILD], | ||
| 'install_cmd': [None, "Main shell command used in the install step", BUILD], | ||
| 'maxparallel': [None, 'Max degree of parallelism', BUILD], | ||
| 'parallel': [None, ('Degree of parallelism for e.g. make (default: based on the number of ' | ||
| 'cores, active cpuset and restrictions in ulimit)'), BUILD], | ||
| 'patches': [[], "List of patches to apply", BUILD], | ||
| 'prebuildopts': ['', 'Extra options pre-passed to build command.', BUILD], | ||
| 'preconfigopts': ['', 'Extra options pre-passed to configure.', BUILD], | ||
| 'preinstallopts': ['', 'Extra prefix options for installation.', BUILD], | ||
| 'pretestopts': ['', 'Extra prefix options for test.', BUILD], | ||
| 'postinstallcmds': [[], 'Commands to run after the install step.', BUILD], | ||
| 'postinstallpatches': [[], 'Patch files to apply after running the install step.', BUILD], | ||
| 'postinstallmsgs': [[], 'Messages to print after running the install step.', BUILD], | ||
| 'pre_build_cmds': ['', 'Extra commands executed before main build command', BUILD], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mention that these are run in a different subshell than the build command itself (no sharing of environment), and only when there's no generic/custom easyblock is used Same for |
||
| 'prebuildopts': ['', 'Extra options prepended to build command', BUILD], | ||
| 'pre_configure_cmds': ['', 'Extra commands executed before main configure command', BUILD], | ||
| 'preconfigopts': ['', 'Extra options prepended to configure command', BUILD], | ||
| 'pre_install_cmds': ['', 'Extra commands executed before main install command', BUILD], | ||
| 'preinstallopts': ['', 'Extra options prepended to installation command', BUILD], | ||
| 'pre_test_cmds': ['', 'Extra commands executed before main test command', BUILD], | ||
| 'pretestopts': ['', 'Extra options prepended to test command', BUILD], | ||
| 'postinstallcmds': [[], 'Commands to run after the install step', BUILD], | ||
| 'postinstallpatches': [[], 'Patch files to apply after running the install step', BUILD], | ||
| 'postinstallmsgs': [[], 'Messages to print after running the install step', BUILD], | ||
| 'required_linked_shared_libs': [[], "List of shared libraries (names, file names, or paths) which must be " | ||
| "linked in all installed binaries/libraries", BUILD], | ||
| 'runtest': [None, ('Indicates if a test should be run after make; should specify argument ' | ||
|
|
@@ -134,8 +141,9 @@ | |
| 'skipsteps': [[], "Skip these steps", BUILD], | ||
| 'source_urls': [[], "List of URLs for source files", BUILD], | ||
| 'sources': [[], "List of source files", BUILD], | ||
| 'stop': [None, 'Keyword to halt the build process after a certain step.', BUILD], | ||
| 'testopts': ['', 'Extra options for test.', BUILD], | ||
| 'stop': [None, 'Keyword to halt the build process after a certain step', BUILD], | ||
| 'testopts': ['', 'Extra options appended to test command', BUILD], | ||
| 'test_cmd': [None, "Main shell command used in the test step", BUILD], | ||
| 'tests': [[], ("List of test-scripts to run after install. A test script should return a " | ||
| "non-zero exit status to fail"), BUILD], | ||
| 'unpack_options': ['', "Extra options for unpacking source", BUILD], | ||
|
|
@@ -148,9 +156,9 @@ | |
| 'buildininstalldir': [False, ('Boolean to build (True) or not build (False) in the installation directory'), | ||
| FILEMANAGEMENT], | ||
| 'cleanupoldbuild': [True, ('Boolean to remove (True) or backup (False) the previous build ' | ||
| 'directory with identical name or not.'), FILEMANAGEMENT], | ||
| 'directory with identical name or not'), FILEMANAGEMENT], | ||
| 'cleanupoldinstall': [True, ('Boolean to remove (True) or backup (False) the previous install ' | ||
| 'directory with identical name or not.'), FILEMANAGEMENT], | ||
| 'directory with identical name or not'), FILEMANAGEMENT], | ||
| 'dontcreateinstalldir': [False, ('Boolean to create (False) or not create (True) the install directory'), | ||
| FILEMANAGEMENT], | ||
| 'keeppreviousinstall': [False, ('Boolean to keep the previous installation with identical ' | ||
|
|
@@ -159,7 +167,7 @@ | |
| 'or if the content of the files pointed to should be copied'), | ||
| FILEMANAGEMENT], | ||
| 'start_dir': [None, ('Path to start the make in. If the path is absolute, use that path. ' | ||
| 'If not, this is added to the guessed path.'), FILEMANAGEMENT], | ||
| 'If not, this is added to the guessed path'), FILEMANAGEMENT], | ||
|
|
||
| # DEPENDENCIES easyconfig parameters | ||
| 'allow_system_deps': [[], "Allow listed system dependencies (format: (<name>, <version>))", DEPENDENCIES], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,8 +103,10 @@ | |
| try: | ||
| import autopep8 | ||
| HAVE_AUTOPEP8 = True | ||
| except ImportError as err: | ||
| _log.warning("Failed to import autopep8, dumping easyconfigs with reformatting enabled will not work: %s", err) | ||
| 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,72 @@ 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') | ||
|
|
||
|
|
||
| def get_easyblock_class(easyblock, name=None, error_on_failed_import=True, error_on_missing_easyblock=True, **kwargs): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't remove At best deprecate it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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_missing_easyblock=True, **kwargs): | ||
| """ | ||
| Get class for a particular easyblock (or use default) | ||
| """ | ||
| cls = None | ||
| try: | ||
| if easyblock: | ||
| # something was specified, lets parse it | ||
| es = easyblock.split('.') | ||
| class_name = es.pop(-1) | ||
| # figure out if full path was specified or not | ||
| if es: | ||
| modulepath = '.'.join(es) | ||
| _log.info("Assuming that full easyblock module path was specified (class: %s, modulepath: %s)", | ||
| class_name, modulepath) | ||
| cls = get_class_for(modulepath, class_name) | ||
| else: | ||
| modulepath = get_module_path(easyblock) | ||
| cls = get_class_for(modulepath, class_name) | ||
| _log.info("Derived full easyblock module path for %s: %s" % (class_name, modulepath)) | ||
| modulepath, class_name = None, None | ||
|
|
||
| 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: | ||
| # 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) | ||
| modulepath_imported = False | ||
| try: | ||
| __import__(modulepath, globals(), locals(), ['']) | ||
| modulepath_imported = True | ||
| except ImportError as err: | ||
| _log.debug("Failed to import module '%s': %s" % (modulepath, err)) | ||
|
|
||
| # check if determining module path based on software name would have resulted in a different module path | ||
| if modulepath_imported: | ||
| _log.debug("Module path '%s' found" % modulepath) | ||
| else: | ||
| _log.debug("No module path '%s' found" % modulepath) | ||
| modulepath_bis = get_module_path(name, generic=False, decode=False) | ||
| _log.debug("Module path determined based on software name: %s" % modulepath_bis) | ||
| if modulepath_bis != modulepath: | ||
| _log.nosupport("Determining module path based on software name", '2.0') | ||
|
|
||
| # try and find easyblock | ||
| try: | ||
| _log.debug("getting class for %s.%s" % (modulepath, class_name)) | ||
| cls = get_class_for(modulepath, class_name) | ||
| _log.info("Successfully obtained %s class instance from %s" % (class_name, modulepath)) | ||
| 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(r"No module named '?.*/?%s'?" % modname) | ||
| _log.debug("error regexp for ImportError on '%s' easyblock: %s", modname, error_re.pattern) | ||
| if error_re.match(str(err)): | ||
| if error_on_missing_easyblock: | ||
| raise EasyBuildError("No software-specific easyblock '%s' found for %s", class_name, name) | ||
| elif error_on_failed_import: | ||
| raise EasyBuildError("Failed to import %s easyblock: %s", class_name, err) | ||
| else: | ||
| _log.debug("Failed to import easyblock for %s, but ignoring it: %s" % (class_name, err)) | ||
| 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) | ||
|
|
||
| if cls is not None: | ||
| _log.info("Successfully obtained class '%s' for easyblock '%s' (software name '%s')", | ||
| cls.__name__, easyblock, name) | ||
| 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("No class found for easyblock '%s' (software name '%s')", easyblock, name) | ||
|
|
||
| return cls | ||
| _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}") | ||
| else: | ||
| _log.debug(f"Easyblock for {class_name} not found, but ignoring it: {err}") | ||
| else: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keep the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in eacd6e3 |
||
| 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("Failed to obtain class for %s easyblock (not available?): %s", easyblock, 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): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.