Merge pull request #4500 from Micket/moduledependson

Enable `module-depends-on` by default

Author: boegel

easybuild/framework/easyblock.py

@@ -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):
@@ -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 = []

easybuild/framework/easyconfig/default.py

@@ -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",

easybuild/tools/config.py

@@ -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',

easybuild/tools/module_generator.py

@@ -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
         """
@@ -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
         """
@@ -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
@@ -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
         """
@@ -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

easybuild/tools/options.py

@@ -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),

test/framework/easyblock.py

@@ -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):
@@ -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))

test/framework/easyconfig.py

@@ -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')
@@ -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)
@@ -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)

test/framework/module_generator.py

@@ -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([
@@ -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."""
@@ -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([
@@ -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)
@@ -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([
@@ -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'])
@@ -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'