Skip to content
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.

Sign up for GitHub

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

drop load storm safe guard for Environment Modules v4.2.4+ #4373

Merged
merged 1 commit into from
Aug 28, 2024
Merged

drop load storm safe guard for Environment Modules v4.2.4+ #4373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/tools/module_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,8 +915,13 @@ def load_module(self, mod_name, recursive_unload=None, depends_on=False, unload_

cond_tmpl = None

# Environment Modules v4+ safely handles automatic module load by not reloading already
# loaded module. No safe guard test is required and it should even be avoided to get the
# module dependency correctly tracked.
safe_auto_load = self.modules_tool.supports_safe_auto_load

if recursive_unload is None:
recursive_unload = build_option('recursive_mod_unload') or depends_on
recursive_unload = build_option('recursive_mod_unload') or depends_on or safe_auto_load

if recursive_unload:
# wrapping the 'module load' statement with an 'is-loaded or mode == unload'
Expand All @@ -927,7 +932,7 @@ def load_module(self, mod_name, recursive_unload=None, depends_on=False, unload_
# see also http://lmod.readthedocs.io/en/latest/210_load_storms.html
cond_tmpl = "[ module-info mode remove ] || %s"

if depends_on:
if depends_on or safe_auto_load:
if multi_dep_mods and len(multi_dep_mods) > 1:
parent_mod_name = os.path.dirname(mod_name)
guard = self.is_loaded(multi_dep_mods[1:])
Expand Down
3 changes: 3 additions & 0 deletions easybuild/tools/modules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ def __init__(self, mod_paths=None, testing=False):
self.supports_depends_on = False
self.supports_tcl_getenv = False
self.supports_tcl_check_group = False
self.supports_safe_auto_load = False

def __str__(self):
"""String representation of this ModulesTool instance."""
Expand Down Expand Up @@ -1325,6 +1326,7 @@ class EnvironmentModules(EnvironmentModulesTcl):
DEPR_VERSION = '4.0.0' # needs to be set as EnvironmentModules inherits from EnvironmentModulesTcl
MAX_VERSION = None
REQ_VERSION_TCL_CHECK_GROUP = '4.6.0'
REQ_VERSION_SAFE_AUTO_LOAD = '4.2.4'
VERSION_REGEXP = r'^Modules\s+Release\s+(?P<version>\d[^+\s]*)(\+\S*)?\s'

SHOW_HIDDEN_OPTION = '--all'
Expand Down Expand Up @@ -1354,6 +1356,7 @@ def __init__(self, *args, **kwargs):
version = LooseVersion(self.version)
self.supports_tcl_getenv = version >= LooseVersion(self.REQ_VERSION_TCL_GETENV)
self.supports_tcl_check_group = version >= LooseVersion(self.REQ_VERSION_TCL_CHECK_GROUP)
self.supports_safe_auto_load = version >= LooseVersion(self.REQ_VERSION_SAFE_AUTO_LOAD)

def check_module_function(self, allow_mismatch=False, regex=None):
"""Check whether selected module tool matches 'module' function definition."""
Expand Down
110 changes: 72 additions & 38 deletions test/framework/module_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,24 +266,32 @@ def test_load(self):
"""Test load part in generated module file."""

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
# default: guarded module load (which implies no recursive unloading)
expected = '\n'.join([
'',
"if { ![ is-loaded mod_name ] } {",
" module load mod_name",
"}",
'',
])
if not self.modtool.supports_safe_auto_load:
# default: guarded module load (which implies no recursive unloading)
expected = '\n'.join([
'',
"if { ![ is-loaded mod_name ] } {",
" module load mod_name",
"}",
'',
])
else:
expected = '\n'.join([
'',
"module load mod_name",
'',
])
self.assertEqual(expected, self.modgen.load_module("mod_name"))

# with recursive unloading: no if is-loaded guard
expected = '\n'.join([
'',
"if { [ module-info mode remove ] || ![ is-loaded mod_name ] } {",
" module load mod_name",
"}",
'',
])
if not self.modtool.supports_safe_auto_load:
# with recursive unloading: no if is-loaded guard
expected = '\n'.join([
'',
"if { [ module-info mode remove ] || ![ is-loaded mod_name ] } {",
" module load mod_name",
"}",
'',
])
self.assertEqual(expected, self.modgen.load_module("mod_name", recursive_unload=True))

init_config(build_options={'recursive_mod_unload': True})
Expand Down Expand Up @@ -353,13 +361,24 @@ def test_load_multi_deps(self):
res = self.modgen.load_module('Python/3.7.4', multi_dep_mods=multi_dep_mods)

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
'',
"if { ![ is-loaded Python/3.7.4 ] && ![ is-loaded Python/2.7.16 ] } {",
" module load Python/3.7.4",
'}',
'',
])
if not self.modtool.supports_safe_auto_load:
expected = '\n'.join([
'',
"if { ![ is-loaded Python/3.7.4 ] && ![ is-loaded Python/2.7.16 ] } {",
" module load Python/3.7.4",
'}',
'',
])
else:
expected = '\n'.join([
'',
"if { [ module-info mode remove ] || [ is-loaded Python/2.7.16 ] } {",
" module load Python",
'} else {',
" module load Python/3.7.4",
'}',
'',
])
else: # Lua syntax
expected = '\n'.join([
'',
Expand Down Expand Up @@ -401,14 +420,26 @@ def test_load_multi_deps(self):
res = self.modgen.load_module('foo/1.2.3', multi_dep_mods=multi_dep_mods)

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
'',
"if { ![ is-loaded foo/1.2.3 ] && ![ is-loaded foo/2.3.4 ] && " +
"![ is-loaded foo/3.4.5 ] && ![ is-loaded foo/4.5.6 ] } {",
" module load foo/1.2.3",
'}',
'',
])
if not self.modtool.supports_safe_auto_load:
expected = '\n'.join([
'',
"if { ![ is-loaded foo/1.2.3 ] && ![ is-loaded foo/2.3.4 ] && " +
"![ is-loaded foo/3.4.5 ] && ![ is-loaded foo/4.5.6 ] } {",
" module load foo/1.2.3",
'}',
'',
])
else:
expected = '\n'.join([
'',
"if { [ module-info mode remove ] || [ is-loaded foo/2.3.4 ] || [ is-loaded foo/3.4.5 ] " +
"|| [ is-loaded foo/4.5.6 ] } {",
" module load foo",
"} else {",
" module load foo/1.2.3",
'}',
'',
])
else: # Lua syntax
expected = '\n'.join([
'',
Expand Down Expand Up @@ -453,13 +484,16 @@ def test_load_multi_deps(self):
res = self.modgen.load_module('one/1.0', multi_dep_mods=['one/1.0'])

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
'',
"if { ![ is-loaded one/1.0 ] } {",
" module load one/1.0",
'}',
'',
])
if not self.modtool.supports_safe_auto_load:
expected = '\n'.join([
'',
"if { ![ is-loaded one/1.0 ] } {",
" module load one/1.0",
'}',
'',
])
else:
expected = '\nmodule load one/1.0\n'
else: # Lua syntax
expected = '\n'.join([
'',
Expand Down
Loading