Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions easybuild/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -937,14 +937,16 @@ def make_devel_module(self, create_in_builddir=False):
# capture all the EBDEVEL vars
# these should be all the dependencies and we should load them
recursive_unload = self.cfg['recursive_module_unload']
use_depends_on = self.cfg['module_use_depends_on']
for key in os.environ:
# legacy support
if key.startswith(DEVEL_ENV_VAR_NAME_PREFIX):
if not key.endswith(convert_name(self.name, upper=True)):
path = os.environ[key]
if os.path.isfile(path):
mod_name = path.rsplit(os.path.sep, 1)[-1]
load_statement = self.module_generator.load_module(mod_name, recursive_unload=recursive_unload)
load_statement = self.module_generator.load_module(mod_name, recursive_unload=recursive_unload,
use_depends_on=use_depends_on)
load_lines.append(load_statement)
elif key.startswith('SOFTDEVEL'):
self.log.nosupport("Environment variable SOFTDEVEL* being relied on", '2.0')
Expand Down Expand Up @@ -1042,12 +1044,15 @@ def make_module_dep(self, unload_info=None):
self.log.debug("List of retained deps to load in generated module: %s", deps)

# include load statements for retained dependencies
recursive_unload = self.cfg['recursive_module_unload']
use_depends_on = self.cfg['module_use_depends_on']
loads = []
for dep in deps:
unload_modules = []
if dep in unload_info:
unload_modules.append(unload_info[dep])
loads.append(self.module_generator.load_module(dep, recursive_unload=self.cfg['recursive_module_unload'],
loads.append(self.module_generator.load_module(dep, recursive_unload=recursive_unload,
use_depends_on=use_depends_on,
unload_modules=unload_modules))

# Force unloading any other modules
Expand Down
2 changes: 2 additions & 0 deletions easybuild/framework/easyconfig/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@
'moduleclass': ['base', 'Module class to be used for this software', MODULES],
'moduleforceunload': [False, 'Force unload of all modules when loading the extension', MODULES],
'moduleloadnoconflict': [False, "Don't check for conflicts, unload other versions instead ", MODULES],
'module_use_depends_on' : [False, 'Use depends_on (Lmod 7.6.1+) for dependencies in generated module '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bartoldeman I would avoid module_use, since that has a particular meaning (not at all related to depends_on).

How about module_depends_on or use_depends_on?

'(implies recursive unloading of modules).', MODULES],
'recursive_module_unload': [False, 'Recursive unload of all dependencies when unloading module', MODULES],

# MODULES documentation easyconfig parameters
Expand Down
2 changes: 2 additions & 0 deletions easybuild/tools/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
False: [
'dry_run',
'recursive_mod_unload',
'mod_use_depends_on',
'retain_all_deps',
'silent',
'try_to_generate',
Expand Down Expand Up @@ -386,6 +387,7 @@ def init_build_options(build_options=None, cmdline_options=None):
'check_osdeps': not cmdline_options.ignore_osdeps,
'dry_run': cmdline_options.dry_run or cmdline_options.dry_run_short,
'recursive_mod_unload': cmdline_options.recursive_module_unload,
'mod_use_depends_on': cmdline_options.module_use_depends_on,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, avoid mod(ule)_use, since it has a very particular meaning

'retain_all_deps': retain_all_deps,
'validate': not cmdline_options.force,
'valid_module_classes': module_classes(),
Expand Down
57 changes: 38 additions & 19 deletions easybuild/tools/module_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,9 @@ class ModuleGeneratorTcl(ModuleGenerator):
MODULE_SHEBANG = '#%Module'
CHARS_TO_ESCAPE = ['$']

LOAD_REGEX = r"^\s*module\s+load\s+(\S+)"
LOAD_REGEX = r"^\s*module\s+(?:load|depends-on)\s+(\S+)"
LOAD_TEMPLATE = "module load %(mod_name)s"
LOAD_TEMPLATE_DEPENDS_ON = "depends-on %(mod_name)s"

def check_group(self, group, error_msg=None):
"""
Expand Down Expand Up @@ -494,7 +495,7 @@ def getenv_cmd(self, envvar):
"""
return '$env(%s)' % envvar

def load_module(self, mod_name, recursive_unload=False, unload_modules=None):
def load_module(self, mod_name, recursive_unload=False, use_depends_on=False, unload_modules=None):
"""
Generate load statement for specified module.

Expand All @@ -505,9 +506,16 @@ def load_module(self, mod_name, recursive_unload=False, unload_modules=None):
body = []
if unload_modules:
body.extend([self.unload_module(m).strip() for m in unload_modules])
body.append(self.LOAD_TEMPLATE)

if build_option('recursive_mod_unload') or recursive_unload:
load_template = self.LOAD_TEMPLATE
# Lmod 7.6.1+ supports depends-on which does this most nicely:
if build_option('mod_use_depends_on') or use_depends_on:
if not modules_tool().supports_depends_on:
raise EasyBuildError("depends_on statements in generated module are not supported by modules tool")
load_template = self.LOAD_TEMPLATE_DEPENDS_ON
body.append(load_template)

if (build_option('recursive_mod_unload') or recursive_unload or
load_template == self.LOAD_TEMPLATE_DEPENDS_ON):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to wrap this across two lines? looks confusing imho...

# not wrapping the 'module load' with an is-loaded guard ensures recursive unloading;
# when "module unload" is called on the module in which the dependency "module load" is present,
# it will get translated to "module unload"
Expand Down Expand Up @@ -668,8 +676,9 @@ class ModuleGeneratorLua(ModuleGenerator):
MODULE_SHEBANG = '' # no 'shebang' in Lua module files
CHARS_TO_ESCAPE = []

LOAD_REGEX = r'^\s*load\("(\S+)"'
LOAD_REGEX = r'^\s*(?:load|depends_on)\("(\S+)"'
LOAD_TEMPLATE = 'load("%(mod_name)s")'
LOAD_TEMPLATE_DEPENDS_ON = 'depends_on("%(mod_name)s")'

PATH_JOIN_TEMPLATE = 'pathJoin(root, "%s")'
UPDATE_PATH_TEMPLATE = '%s_path("%s", %s)'
Expand Down Expand Up @@ -784,7 +793,7 @@ def getenv_cmd(self, envvar):
"""
return 'os.getenv("%s")' % envvar

def load_module(self, mod_name, recursive_unload=False, unload_modules=None):
def load_module(self, mod_name, recursive_unload=False, use_depends_on=False, unload_modules=None):
"""
Generate load statement for specified module.

Expand All @@ -795,19 +804,29 @@ def load_module(self, mod_name, recursive_unload=False, unload_modules=None):
body = []
if unload_modules:
body.extend([self.unload_module(m).strip() for m in unload_modules])
body.append(self.LOAD_TEMPLATE)

if build_option('recursive_mod_unload') or recursive_unload:
# wrapping the 'module load' with an 'is-loaded or mode == unload'
# guard ensures recursive unloading while avoiding load storms,
# when "module unload" is called on the module in which the
# depedency "module load" is present, it will get translated
# to "module unload"
# see also http://lmod.readthedocs.io/en/latest/210_load_storms.html
load_guard = 'isloaded("%(mod_name)s") or mode() == "unload"'

load_template = self.LOAD_TEMPLATE
# Lmod 7.6+ supports depends_on which does this most nicely:
if build_option('mod_use_depends_on') or use_depends_on:
if not modules_tool().supports_depends_on:
raise EasyBuildError("depends_on statements in generated module are not supported by modules tool")
load_template = self.LOAD_TEMPLATE_DEPENDS_ON

body.append(load_template)
if load_template == self.LOAD_TEMPLATE_DEPENDS_ON:
load_statement = body + ['']
else:
load_guard = 'isloaded("%(mod_name)s")'
load_statement = [self.conditional_statement(load_guard, '\n'.join(body), negative=True)]
if build_option('recursive_mod_unload') or recursive_unload:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this elif?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it really needs to be else: because of the common load_statement = statement at the bottom.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I overlooked that indeed, thanks or clarifying

# wrapping the 'module load' with an 'is-loaded or mode == unload'
# guard ensures recursive unloading while avoiding load storms,
# when "module unload" is called on the module in which the
# depedency "module load" is present, it will get translated
# to "module unload"
# see also http://lmod.readthedocs.io/en/latest/210_load_storms.html
load_guard = 'isloaded("%(mod_name)s") or mode() == "unload"'
else:
load_guard = 'isloaded("%(mod_name)s")'
load_statement = [self.conditional_statement(load_guard, '\n'.join(body), negative=True)]

return '\n'.join([''] + load_statement) % {'mod_name': mod_name}

Expand Down
3 changes: 3 additions & 0 deletions easybuild/tools/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ def __init__(self, mod_paths=None, testing=False):
self.check_module_path()
self.check_module_function(allow_mismatch=build_option('allow_modules_tool_mismatch'))
self.set_and_check_version()
self.supports_depends_on = False

def buildstats(self):
"""Return tuple with data to be included in buildstats"""
Expand Down Expand Up @@ -1101,6 +1102,7 @@ class Lmod(ModulesTool):
COMMAND = 'lmod'
COMMAND_ENVIRONMENT = 'LMOD_CMD'
REQ_VERSION = '5.8'
REQ_VERSION_DEPENDS_ON = '7.6.1'
VERSION_REGEXP = r"^Modules\s+based\s+on\s+Lua:\s+Version\s+(?P<version>\d\S*)\s"
USER_CACHE_DIR = os.path.join(os.path.expanduser('~'), '.lmod.d', '.cache')

Expand All @@ -1116,6 +1118,7 @@ def __init__(self, *args, **kwargs):
setvar('LMOD_REDIRECT', 'no', verbose=False)

super(Lmod, self).__init__(*args, **kwargs)
self.supports_depends_on = StrictVersion(self.version) >= StrictVersion(self.REQ_VERSION_DEPENDS_ON)

def check_module_function(self, *args, **kwargs):
"""Check whether selected module tool matches 'module' function definition."""
Expand Down
3 changes: 3 additions & 0 deletions easybuild/tools/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,9 @@ def config_options(self):
'module-naming-scheme': ("Module naming scheme to use", None, 'store', DEFAULT_MNS),
'module-syntax': ("Syntax to be used for module files", 'choice', 'store', DEFAULT_MODULE_SYNTAX,
sorted(avail_module_generators().keys())),
'module-use-depends-on': ("Use depends_on (Lmod 7.6.1+) for dependencies in all generated modules "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, I'd go with either --module-depends-on or --use-depends-on

"(implies recursive unloading of modules).",
None, 'store_true', False),
'moduleclasses': (("Extend supported module classes "
"(For more info on the default classes, use --show-default-moduleclasses)"),
'strlist', 'extend', [x[0] for x in DEFAULT_MODULECLASSES]),
Expand Down
22 changes: 22 additions & 0 deletions test/framework/module_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,17 @@ def test_load(self):

init_config(build_options={'recursive_mod_unload': True})
self.assertEqual(expected, self.modgen.load_module("mod_name"))

# Lmod 7.6+ depends-on
if self.modtool.supports_depends_on:
expected = '\n'.join([
'',
"depends-on mod_name",
'',
])
self.assertEqual(expected, self.modgen.load_module("mod_name", use_depends_on=True))
init_config(build_options={'mod_use_depends_on': 'True'})
self.assertEqual(expected, self.modgen.load_module("mod_name"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also check the other case, with self.assertErrorRegex?

else:
# default: guarded module load (which implies no recursive unloading)
expected = '\n'.join([
Expand All @@ -283,6 +294,17 @@ def test_load(self):
init_config(build_options={'recursive_mod_unload': True})
self.assertEqual(expected, self.modgen.load_module("mod_name"))

# Lmod 7.6+ depends_on
if self.modtool.supports_depends_on:
expected = '\n'.join([
'',
'depends_on("mod_name")',
'',
])
self.assertEqual(expected, self.modgen.load_module("mod_name", use_depends_on=True))
init_config(build_options={'mod_use_depends_on': 'True'})
self.assertEqual(expected, self.modgen.load_module("mod_name"))

def test_unload(self):
"""Test unload part in generated module file."""

Expand Down
38 changes: 21 additions & 17 deletions test/framework/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -1156,23 +1156,27 @@ def test_recursive_module_unload(self):
eb_file = os.path.join(os.path.dirname(__file__), 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0-deps.eb')

# check log message with --skip for existing module
args = [
eb_file,
'--sourcepath=%s' % self.test_sourcepath,
'--buildpath=%s' % self.test_buildpath,
'--installpath=%s' % self.test_installpath,
'--debug',
'--force',
'--recursive-module-unload',
]
self.eb_main(args, do_build=True, verbose=True)

toy_module = os.path.join(self.test_installpath, 'modules', 'all', 'toy', '0.0-deps')
if get_module_syntax() == 'Lua':
toy_module += '.lua'
toy_module_txt = read_file(toy_module)
is_loaded_regex = re.compile(r"if { !\[is-loaded gompi/1.3.12\] }", re.M)
self.assertFalse(is_loaded_regex.search(toy_module_txt), "Recursive unloading is used: %s" % toy_module_txt)
lastargs = ['--recursive-module-unload']
if self.modtool.supports_depends_on:
lastargs.append('--module-use-depends-on')
for lastarg in lastargs:
args = [
eb_file,
'--sourcepath=%s' % self.test_sourcepath,
'--buildpath=%s' % self.test_buildpath,
'--installpath=%s' % self.test_installpath,
'--debug',
'--force',
lastarg,
]
self.eb_main(args, do_build=True, verbose=True)

toy_module = os.path.join(self.test_installpath, 'modules', 'all', 'toy', '0.0-deps')
if get_module_syntax() == 'Lua':
toy_module += '.lua'
toy_module_txt = read_file(toy_module)
is_loaded_regex = re.compile(r"if { !\[is-loaded gompi/1.3.12\] }", re.M)
self.assertFalse(is_loaded_regex.search(toy_module_txt), "Recursive unloading is used: %s" % toy_module_txt)

def test_tmpdir(self):
"""Test setting temporary directory to use by EasyBuild."""
Expand Down