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.

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

Enable module-depends-on by default #4500

Merged
merged 14 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions easybuild/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,8 @@ def make_devel_module(self, create_in_builddir=False):
# these should be all the dependencies and we should load them
recursive_unload = self.cfg['recursive_module_unload']
depends_on = self.cfg['module_depends_on']
if depends_on is not None:
self.log.deprecated("'module_depends_on' easyconfig parameter should not be used anymore", '6.0')
for key in os.environ:
# legacy support
if key.startswith(DEVEL_ENV_VAR_NAME_PREFIX):
Expand Down Expand Up @@ -1346,6 +1348,8 @@ def make_module_dep(self, unload_info=None):
# include load statements for retained dependencies
recursive_unload = self.cfg['recursive_module_unload']
depends_on = self.cfg['module_depends_on']
if depends_on is not None:
self.log.deprecated("'module_depends_on' easyconfig parameter should not be used anymore", '6.0')
loads = []
for dep in deps:
unload_modules = []
Expand Down
4 changes: 2 additions & 2 deletions easybuild/framework/easyconfig/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@
'moduleclass': [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_depends_on': [False, 'Use depends_on (Lmod 7.6.1+) for dependencies in generated module '
'(implies recursive unloading of modules).', MODULES],
'module_depends_on': [None, 'Use depends_on (Lmod 7.6.1+) for dependencies in generated module '
'(implies recursive unloading of modules) [DEPRECATED]', MODULES],
'recursive_module_unload': [None, "Recursive unload of all dependencies when unloading module "
"(True/False to hard enable/disable; None implies honoring "
"the --recursive-module-unload EasyBuild configuration setting",
Expand Down
2 changes: 1 addition & 1 deletion easybuild/tools/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,13 +422,13 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
'command_line',
'external_modules_metadata',
'extra_ec_paths',
'mod_depends_on', # deprecated
'robot_path',
'valid_module_classes',
'valid_stops',
],
False: [
'dry_run',
'mod_depends_on',
'recursive_mod_unload',
'retain_all_deps',
'silent',
Expand Down
22 changes: 14 additions & 8 deletions easybuild/tools/module_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,13 +484,13 @@ def getenv_cmd(self, envvar, default=None):
"""
raise NotImplementedError

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

:param mod_name: name of module to generate load statement for
:param recursive_unload: boolean indicating whether the 'load' statement should be reverted on unload
:param depends_on: use depends_on statements rather than (guarded) load statements
:param depends_on: use depends_on statements rather than (guarded) load statements (DEPRECATED)
:param unload_modules: name(s) of module to unload first
:param multi_dep_mods: list of module names in multi_deps context, to use for guarding load statement
"""
Expand Down Expand Up @@ -879,14 +879,14 @@ def getenv_cmd(self, envvar, default=None):
cmd = '[if { [info exists %(envvar)s] } { concat $%(envvar)s } else { concat "%(default)s" } ]' % values
return cmd

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

:param mod_name: name of module to generate load statement for
:param recursive_unload: boolean indicating whether the 'load' statement should be reverted on unload
(if None: enable if recursive_mod_unload build option or depends_on is True)
:param depends_on: use depends_on statements rather than (guarded) load statements
:param depends_on: use depends_on statements rather than (guarded) load statements (DEPRECATED)
:param unload_modules: name(s) of module to unload first
:param multi_dep_mods: list of module names in multi_deps context, to use for guarding load statement
"""
Expand All @@ -895,7 +895,10 @@ def load_module(self, mod_name, recursive_unload=None, depends_on=False, unload_
body.extend([self.unload_module(m).strip() for m in unload_modules])
load_template = self.LOAD_TEMPLATE
# Lmod 7.6.1+ supports depends-on which does this most nicely:
if build_option('mod_depends_on') or depends_on:
if (build_option('mod_depends_on') and self.modules_tool.supports_depends_on) or depends_on:
if depends_on is not None:
depr_msg = "'depends_on' argument of module generator method 'load_module' should not be used anymore"
self.log.deprecated(depr_msg, '6.0')
if not self.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
Expand Down Expand Up @@ -1304,14 +1307,14 @@ def getenv_cmd(self, envvar, default=None):
cmd = 'os.getenv("%s") or "%s"' % (envvar, default)
return cmd

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

:param mod_name: name of module to generate load statement for
:param recursive_unload: boolean indicating whether the 'load' statement should be reverted on unload
(if None: enable if recursive_mod_unload build option or depends_on is True)
:param depends_on: use depends_on statements rather than (guarded) load statements
:param depends_on: use depends_on statements rather than (guarded) load statements (DEPRECATED)
:param unload_modules: name(s) of module to unload first
:param multi_dep_mods: list of module names in multi_deps context, to use for guarding load statement
"""
Expand All @@ -1321,7 +1324,10 @@ def load_module(self, mod_name, recursive_unload=None, depends_on=False, unload_

load_template = self.LOAD_TEMPLATE
# Lmod 7.6+ supports depends_on which does this most nicely:
if build_option('mod_depends_on') or depends_on:
if (build_option('mod_depends_on') and self.modules_tool.supports_depends_on) or depends_on:
if depends_on is not None:
depr_msg = "'depends_on' argument of module generator method 'load_module' should not be used anymore"
self.log.deprecated(depr_msg, '6.0')
if not self.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
Expand Down
2 changes: 1 addition & 1 deletion easybuild/tools/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ def config_options(self):
'strtuple', 'store', DEFAULT_LOGFILE_FORMAT[:], {'metavar': 'DIR,FORMAT'}),
'module-depends-on': ("Use depends_on (Lmod 7.6.1+) for dependencies in all generated modules "
"(implies recursive unloading of modules).",
None, 'store_true', False),
None, 'store_true', True),
'module-extensions': ("Include 'extensions' statement in generated module file (Lua syntax only)",
None, 'store_true', True),
'module-naming-scheme': ("Module naming scheme to use", None, 'store', DEFAULT_MNS),
Expand Down
16 changes: 8 additions & 8 deletions test/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -873,10 +873,10 @@ def test_make_module_dep_hmns(self):
with self.mocked_stdout_stderr():
mod_dep_txt = eb.make_module_dep()
for mod in ['GCC/6.4.0-2.28', 'OpenMPI/2.1.2']:
regex = re.compile('load.*%s' % mod)
regex = re.compile('(load|depends[-_]on).*%s' % mod)
self.assertFalse(regex.search(mod_dep_txt), "Pattern '%s' found in: %s" % (regex.pattern, mod_dep_txt))

regex = re.compile('load.*FFTW/3.3.7')
regex = re.compile('(load|depends[-_]on).*FFTW/3.3.7')
self.assertTrue(regex.search(mod_dep_txt), "Pattern '%s' found in: %s" % (regex.pattern, mod_dep_txt))

def test_make_module_dep_of_dep_hmns(self):
Expand Down Expand Up @@ -1361,27 +1361,27 @@ def test_make_module_step(self):

for (name, ver) in [('GCC', '6.4.0-2.28')]:
if get_module_syntax() == 'Tcl':
regex = re.compile(r'^\s*module load %s\s*$' % os.path.join(name, ver), re.M)
regex = re.compile(r'^\s*(module load|depends-on) %s\s*$' % os.path.join(name, ver), re.M)
elif get_module_syntax() == 'Lua':
regex = re.compile(r'^\s*load\("%s"\)$' % os.path.join(name, ver), re.M)
regex = re.compile(r'^\s*(load|depends_on)\("%s"\)$' % os.path.join(name, ver), re.M)
else:
self.fail("Unknown module syntax: %s" % get_module_syntax())
self.assertTrue(regex.search(txt), "Pattern %s found in %s" % (regex.pattern, txt))

for (name, ver) in [('test', '1.2.3')]:
if get_module_syntax() == 'Tcl':
regex = re.compile(r'^\s*module load %s/.%s\s*$' % (name, ver), re.M)
regex = re.compile(r'^\s*(module load|depends-on) %s/.%s\s*$' % (name, ver), re.M)
elif get_module_syntax() == 'Lua':
regex = re.compile(r'^\s*load\("%s/.%s"\)$' % (name, ver), re.M)
regex = re.compile(r'^\s*(load|depends_on)\("%s/.%s"\)$' % (name, ver), re.M)
else:
self.fail("Unknown module syntax: %s" % get_module_syntax())
self.assertTrue(regex.search(txt), "Pattern %s found in %s" % (regex.pattern, txt))

for (name, ver) in [('OpenMPI', '2.1.2-GCC-6.4.0-2.28')]:
if get_module_syntax() == 'Tcl':
regex = re.compile(r'^\s*module load %s/.?%s\s*$' % (name, ver), re.M)
regex = re.compile(r'^\s*(module load|depends-on) %s/.?%s\s*$' % (name, ver), re.M)
elif get_module_syntax() == 'Lua':
regex = re.compile(r'^\s*load\("%s/.?%s"\)$' % (name, ver), re.M)
regex = re.compile(r'^\s*(load|depends_on)\("%s/.?%s"\)$' % (name, ver), re.M)
else:
self.fail("Unknown module syntax: %s" % get_module_syntax())
self.assertFalse(regex.search(txt), "Pattern '%s' *not* found in %s" % (regex.pattern, txt))
Expand Down
8 changes: 8 additions & 0 deletions test/framework/easyconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -4785,6 +4785,10 @@ def test_recursive_module_unload(self):
toy_ec = os.path.join(test_ecs_dir, 'f', 'foss', 'foss-2018a.eb')
test_ec = os.path.join(self.test_prefix, 'test.eb')
test_ec_txt = read_file(toy_ec)

# this test only makes sense if depends_on is not used
self.allow_deprecated_behaviour()
test_ec_txt += '\nmodule_depends_on = False'
write_file(test_ec, test_ec_txt)

test_module = os.path.join(self.test_installpath, 'modules', 'all', 'foss', '2018a')
Expand Down Expand Up @@ -4827,6 +4831,8 @@ def test_recursive_module_unload(self):
# recursive_module_unload easyconfig parameter is honored
test_ec_bis = os.path.join(self.test_prefix, 'test_bis.eb')
test_ec_bis_txt = read_file(toy_ec) + '\nrecursive_module_unload = True'
# this test only makes sense if depends_on is not used
test_ec_bis_txt += '\nmodule_depends_on = False'
write_file(test_ec_bis, test_ec_bis_txt)

ec_bis = EasyConfig(test_ec_bis)
Expand Down Expand Up @@ -4871,6 +4877,8 @@ def test_recursive_module_unload(self):
self.assertTrue(build_option('recursive_mod_unload'))
test_ec_bis = os.path.join(self.test_prefix, 'test_bis.eb')
test_ec_bis_txt = read_file(toy_ec) + '\nrecursive_module_unload = False'
# this test only makes sense if depends_on is not used
test_ec_bis_txt += '\nmodule_depends_on = False'
write_file(test_ec_bis, test_ec_bis_txt)
ec_bis = EasyConfig(test_ec_bis)
self.assertEqual(ec_bis['recursive_module_unload'], False)
Expand Down
49 changes: 38 additions & 11 deletions test/framework/module_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,20 +298,25 @@ def test_load(self):
self.assertEqual(expected, self.modgen.load_module("mod_name"))

# Lmod 7.6+ depends-on

self.allow_deprecated_behaviour()

if self.modtool.supports_depends_on:
expected = '\n'.join([
'',
"depends-on mod_name",
'',
])
self.assertEqual(expected, self.modgen.load_module("mod_name", depends_on=True))
with self.mocked_stdout_stderr():
txt = self.modgen.load_module("mod_name", depends_on=True)
self.assertEqual(expected, txt)
init_config(build_options={'mod_depends_on': 'True'})
self.assertEqual(expected, self.modgen.load_module("mod_name"))
else:
expected = "depends-on statements in generated module are not supported by modules tool"
self.assertErrorRegex(EasyBuildError, expected, self.modgen.load_module, "mod_name", depends_on=True)
init_config(build_options={'mod_depends_on': 'True'})
self.assertErrorRegex(EasyBuildError, expected, self.modgen.load_module, "mod_name")
with self.mocked_stdout_stderr():
self.assertErrorRegex(EasyBuildError, expected,
self.modgen.load_module, "mod_name", depends_on=True)
else:
# default: guarded module load (which implies no recursive unloading)
expected = '\n'.join([
Expand All @@ -338,20 +343,26 @@ def test_load(self):
self.assertEqual(expected, self.modgen.load_module("mod_name"))

# Lmod 7.6+ depends_on

self.allow_deprecated_behaviour()

if self.modtool.supports_depends_on:
expected = '\n'.join([
'',
'depends_on("mod_name")',
'',
])
self.assertEqual(expected, self.modgen.load_module("mod_name", depends_on=True))
with self.mocked_stdout_stderr():
txt = self.modgen.load_module("mod_name", depends_on=True)

self.assertEqual(expected, txt)
init_config(build_options={'mod_depends_on': 'True'})
self.assertEqual(expected, self.modgen.load_module("mod_name"))
else:
expected = "depends_on statements in generated module are not supported by modules tool"
self.assertErrorRegex(EasyBuildError, expected, self.modgen.load_module, "mod_name", depends_on=True)
init_config(build_options={'mod_depends_on': 'True'})
self.assertErrorRegex(EasyBuildError, expected, self.modgen.load_module, "mod_name")
with self.mocked_stdout_stderr():
self.assertErrorRegex(EasyBuildError, expected,
self.modgen.load_module, "mod_name", depends_on=True)

def test_load_multi_deps(self):
"""Test generated load statement when multi_deps is involved."""
Expand Down Expand Up @@ -390,8 +401,12 @@ def test_load_multi_deps(self):
self.assertEqual(expected, res)

if self.modtool.supports_depends_on:

self.allow_deprecated_behaviour()

# two versions with depends_on
res = self.modgen.load_module('Python/3.7.4', multi_dep_mods=multi_dep_mods, depends_on=True)
with self.mocked_stdout_stderr():
res = self.modgen.load_module('Python/3.7.4', multi_dep_mods=multi_dep_mods, depends_on=True)

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
Expand All @@ -415,6 +430,8 @@ def test_load_multi_deps(self):
])
self.assertEqual(expected, res)

self.disallow_deprecated_behaviour()

# now test with more than two versions...
multi_dep_mods = ['foo/1.2.3', 'foo/2.3.4', 'foo/3.4.5', 'foo/4.5.6']
res = self.modgen.load_module('foo/1.2.3', multi_dep_mods=multi_dep_mods)
Expand Down Expand Up @@ -452,8 +469,12 @@ def test_load_multi_deps(self):
self.assertEqual(expected, res)

if self.modtool.supports_depends_on:

self.allow_deprecated_behaviour()

# more than two versions, with depends_on
res = self.modgen.load_module('foo/1.2.3', multi_dep_mods=multi_dep_mods, depends_on=True)
with self.mocked_stdout_stderr():
res = self.modgen.load_module('foo/1.2.3', multi_dep_mods=multi_dep_mods, depends_on=True)

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
Expand All @@ -479,6 +500,8 @@ def test_load_multi_deps(self):
])
self.assertEqual(expected, res)

self.disallow_deprecated_behaviour()

# what if we only list a single version?
# see https://github.com/easybuilders/easybuild-framework/issues/3080
res = self.modgen.load_module('one/1.0', multi_dep_mods=['one/1.0'])
Expand All @@ -505,7 +528,11 @@ def test_load_multi_deps(self):
self.assertEqual(expected, res)

if self.modtool.supports_depends_on:
res = self.modgen.load_module('one/1.0', multi_dep_mods=['one/1.0'], depends_on=True)

self.allow_deprecated_behaviour()

with self.mocked_stdout_stderr():
res = self.modgen.load_module('one/1.0', multi_dep_mods=['one/1.0'], depends_on=True)

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\ndepends-on one/1.0\n'
Expand Down
Loading
Loading