Skip to content

Commit

Permalink
modules: use depends-on to autoload module with Lmod on Tcl (spack#38347
Browse files Browse the repository at this point in the history
)

Update Tcl modulefile template to use the `depends-on` command to
autoload modules if Lmod is the current module tool.

Autoloading modules with `module load` command in Tcl modulefile does
not work well for Lmod at some extend. An attempt to unload then load
designated module is performed each time such command is encountered. It
may lead to a load storm that may not end correctly with large number of
module dependencies.

`depends-on` command should be used for Lmod instead of `module load`,
as it checks if module is already loaded, and does not attempt to reload
this module.

Lua modulefile template already uses `depends_on` command to autoload
dependencies. Thus it is already considered that to use Lmod with Spack,
it must support `depends_on` command (version 7.6+).

Environment Modules copes well with `module load` command to autoload
dependencies (version 3.2+). `depends-on` command is supported starting
version 5.1 (as an alias of `prereq-all` command) which was relased last
year.

This change introduces a test to determine if current module tool that
evaluates modulefile is Lmod. If so, autoload dependencies are defined
with `depends-on` command. Otherwise `module load` command is used.

Test is based on `LMOD_VERSION_MAJOR` environment variable, which is set
by Lmod starting version 5.1.

Fixes spack#36764
  • Loading branch information
xdelaruelle authored Jun 14, 2023
1 parent 6979d6a commit 2db09f2
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
27 changes: 27 additions & 0 deletions lib/spack/spack/test/modules/tcl.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ def test_autoload_direct(self, modulefile_content, module_configuration):
module_configuration("autoload_direct")
content = modulefile_content(mpileaks_spec_string)

assert (
len([x for x in content if "if {![info exists ::env(LMOD_VERSION_MAJOR)]} {" in x])
== 1
)
assert len([x for x in content if "depends-on " in x]) == 2
assert len([x for x in content if "module load " in x]) == 2

# dtbuild1 has
Expand All @@ -46,6 +51,11 @@ def test_autoload_direct(self, modulefile_content, module_configuration):
# Just make sure the 'build' dependency is not there
content = modulefile_content("dtbuild1")

assert (
len([x for x in content if "if {![info exists ::env(LMOD_VERSION_MAJOR)]} {" in x])
== 1
)
assert len([x for x in content if "depends-on " in x]) == 2
assert len([x for x in content if "module load " in x]) == 2

# The configuration file sets the verbose keyword to False
Expand All @@ -58,6 +68,11 @@ def test_autoload_all(self, modulefile_content, module_configuration):
module_configuration("autoload_all")
content = modulefile_content(mpileaks_spec_string)

assert (
len([x for x in content if "if {![info exists ::env(LMOD_VERSION_MAJOR)]} {" in x])
== 1
)
assert len([x for x in content if "depends-on " in x]) == 5
assert len([x for x in content if "module load " in x]) == 5

# dtbuild1 has
Expand All @@ -67,6 +82,11 @@ def test_autoload_all(self, modulefile_content, module_configuration):
# Just make sure the 'build' dependency is not there
content = modulefile_content("dtbuild1")

assert (
len([x for x in content if "if {![info exists ::env(LMOD_VERSION_MAJOR)]} {" in x])
== 1
)
assert len([x for x in content if "depends-on " in x]) == 2
assert len([x for x in content if "module load " in x]) == 2

def test_prerequisites_direct(self, modulefile_content, module_configuration):
Expand Down Expand Up @@ -103,6 +123,7 @@ def test_alter_environment(self, modulefile_content, module_configuration):
assert len([x for x in content if x.startswith("prepend-path CMAKE_PREFIX_PATH")]) == 0
assert len([x for x in content if 'setenv FOO "foo"' in x]) == 0
assert len([x for x in content if "unsetenv BAR" in x]) == 0
assert len([x for x in content if "depends-on foo/bar" in x]) == 1
assert len([x for x in content if "module load foo/bar" in x]) == 1
assert len([x for x in content if "setenv LIBDWARF_ROOT" in x]) == 1

Expand Down Expand Up @@ -434,10 +455,16 @@ def test_autoload_with_constraints(self, modulefile_content, module_configuratio

# Test the mpileaks that should have the autoloaded dependencies
content = modulefile_content("mpileaks ^mpich2")
assert len([x for x in content if "depends-on " in x]) == 2
assert len([x for x in content if "module load " in x]) == 2

# Test the mpileaks that should NOT have the autoloaded dependencies
content = modulefile_content("mpileaks ^mpich")
assert (
len([x for x in content if "if {![info exists ::env(LMOD_VERSION_MAJOR)]} {" in x])
== 0
)
assert len([x for x in content if "depends-on " in x]) == 0
assert len([x for x in content if "module load " in x]) == 0

def test_modules_no_arch(self, factory, module_configuration):
Expand Down
10 changes: 9 additions & 1 deletion share/spack/templates/modules/modulefile.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,17 @@ proc ModulesHelp { } {
{% endblock %}

{% block autoloads %}
{% if autoload|length > 0 %}
if {![info exists ::env(LMOD_VERSION_MAJOR)]} {
{% for module in autoload %}
module load {{ module }}
module load {{ module }}
{% endfor %}
} else {
{% for module in autoload %}
depends-on {{ module }}
{% endfor %}
}
{% endif %}
{% endblock %}
{# #}
{% block prerequisite %}
Expand Down

0 comments on commit 2db09f2

Please sign in to comment.