From b5e154c95be5dfc709f6738a1fbca0fb10805a1c Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 6 Apr 2021 12:40:17 +0200 Subject: [PATCH 1/3] Avoid module call for unuse() for LMod and set $MODULEPATH directly Followup to #3557 No check for priorities is required as LMod correctly handles that Also adds a check for an empty path which is technically possible and handled by LMod like any other path --- easybuild/tools/modules.py | 15 +++++++++++++++ test/framework/modules.py | 18 +++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 14509e79d8..3e3f004dc5 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1443,6 +1443,21 @@ def use(self, path, priority=None): ('' if cur_mod_path is None else cur_mod_path, new_mod_path)) os.environ['MODULEPATH'] = new_mod_path + def unuse(self, path): + """Remove a module path""" + # We can simply remove the path from MODULEPATH to avoid the costly module call + cur_mod_path = os.environ.get('MODULEPATH') + if cur_mod_path is not None: + # Removing the last entry unsets the variable + if cur_mod_path == path: + self.log.debug('Changing MODULEPATH from %s to ' % cur_mod_path) + del os.environ['MODULEPATH'] + else: + new_mod_path = ':'.join(p for p in cur_mod_path.split(':') if p != path) + if new_mod_path != cur_mod_path: + self.log.debug('Changing MODULEPATH from %s to %s' % (cur_mod_path, new_mod_path)) + os.environ['MODULEPATH'] = new_mod_path + def prepend_module_path(self, path, set_mod_paths=True, priority=None): """ Prepend given module path to list of module paths, or bump it to 1st place. diff --git a/test/framework/modules.py b/test/framework/modules.py index c71b5f4360..33ef840e8e 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1157,11 +1157,23 @@ def test_module_use_unuse(self): self.assertFalse(test_dir1 in os.environ.get('MODULEPATH', '')) self.modtool.use(test_dir1) - self.assertTrue(os.environ.get('MODULEPATH', '').startswith('%s:' % test_dir1)) + self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir1)) self.modtool.use(test_dir2) - self.assertTrue(os.environ.get('MODULEPATH', '').startswith('%s:' % test_dir2)) + self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir2)) self.modtool.use(test_dir3) - self.assertTrue(os.environ.get('MODULEPATH', '').startswith('%s:' % test_dir3)) + self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir3)) + + # Using an empty path still works (technically) + old_module_path = os.environ['MODULEPATH'] + self.modtool.use('') + self.assertEqual(os.environ['MODULEPATH'], ':' + old_module_path) + self.modtool.unuse('') + self.assertEqual(os.environ['MODULEPATH'], old_module_path) + # Even works when the whole path is empty + os.environ['MODULEPATH'] = '' + self.modtool.unuse('') + self.assertFalse('MODULEPATH' in os.environ) + os.environ['MODULEPATH'] = old_module_path # Restore # make sure the right test module is loaded self.modtool.load(['test']) From 3262fc69b93387deb8f5f6ff08449ef27589b5a5 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 6 Apr 2021 12:45:04 +0200 Subject: [PATCH 2/3] Add test for adding to non-existing MODULEPATH and removing it --- test/framework/modules.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/framework/modules.py b/test/framework/modules.py index 33ef840e8e..a9111620b9 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1224,6 +1224,16 @@ def test_module_use_unuse(self): self.assertEqual(os.getenv('TEST123'), 'three') self.modtool.unload(['test']) + # Also test that load and unload a single path works when it is the only one + # Only for LMod as we have some shortcuts for avoiding the module call there + old_module_path = os.environ['MODULEPATH'] + del os.environ['MODULEPATH'] + self.modtool.use(test_dir1) + self.assertEqual(os.environ['MODULEPATH'], test_dir1) + self.modtool.unuse(test_dir1) + self.assertFalse('MODULEPATH' in os.environ) + os.environ['MODULEPATH'] = old_module_path # Restore + def test_module_use_bash(self): """Test whether effect of 'module use' is preserved when a new bash session is started.""" # this test is here as check for a nasty bug in how the modules tool is deployed From 21083d3b156e92ac69b8eddb20ab4525f22c3d18 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 6 Apr 2021 14:49:55 +0200 Subject: [PATCH 3/3] Test special cases for LMod only --- test/framework/modules.py | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/test/framework/modules.py b/test/framework/modules.py index a9111620b9..a2f4633787 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1163,18 +1163,6 @@ def test_module_use_unuse(self): self.modtool.use(test_dir3) self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir3)) - # Using an empty path still works (technically) - old_module_path = os.environ['MODULEPATH'] - self.modtool.use('') - self.assertEqual(os.environ['MODULEPATH'], ':' + old_module_path) - self.modtool.unuse('') - self.assertEqual(os.environ['MODULEPATH'], old_module_path) - # Even works when the whole path is empty - os.environ['MODULEPATH'] = '' - self.modtool.unuse('') - self.assertFalse('MODULEPATH' in os.environ) - os.environ['MODULEPATH'] = old_module_path # Restore - # make sure the right test module is loaded self.modtool.load(['test']) self.assertEqual(os.getenv('TEST123'), 'three') @@ -1205,8 +1193,9 @@ def test_module_use_unuse(self): self.assertEqual(os.getenv('TEST123'), 'two') self.modtool.unload(['test']) - # check whether prepend with priority actually works (only for Lmod) + # Tests for Lmod only if isinstance(self.modtool, Lmod): + # check whether prepend with priority actually works (priority is specific to Lmod) self.modtool.use(test_dir1, priority=100) self.modtool.use(test_dir3) self.assertTrue(os.environ['MODULEPATH'].startswith('%s:%s:%s:' % (test_dir2, test_dir1, test_dir3))) @@ -1224,8 +1213,8 @@ def test_module_use_unuse(self): self.assertEqual(os.getenv('TEST123'), 'three') self.modtool.unload(['test']) - # Also test that load and unload a single path works when it is the only one - # Only for LMod as we have some shortcuts for avoiding the module call there + # Check load and unload for a single path when it is the only one + # Only for Lmod as we have some shortcuts for avoiding the module call there old_module_path = os.environ['MODULEPATH'] del os.environ['MODULEPATH'] self.modtool.use(test_dir1) @@ -1234,6 +1223,18 @@ def test_module_use_unuse(self): self.assertFalse('MODULEPATH' in os.environ) os.environ['MODULEPATH'] = old_module_path # Restore + # Using an empty path still works (technically) (Lmod only, ignored by Tcl) + old_module_path = os.environ['MODULEPATH'] + self.modtool.use('') + self.assertEqual(os.environ['MODULEPATH'], ':' + old_module_path) + self.modtool.unuse('') + self.assertEqual(os.environ['MODULEPATH'], old_module_path) + # Even works when the whole path is empty + os.environ['MODULEPATH'] = '' + self.modtool.unuse('') + self.assertFalse('MODULEPATH' in os.environ) + os.environ['MODULEPATH'] = old_module_path # Restore + def test_module_use_bash(self): """Test whether effect of 'module use' is preserved when a new bash session is started.""" # this test is here as check for a nasty bug in how the modules tool is deployed