diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 77e0df72a5..274e60f874 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -2155,7 +2155,7 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True): curr_modpaths = curr_module_paths() for init_modpath in init_modpaths: full_mod_path = os.path.join(self.installdir_mod, init_modpath) - if full_mod_path not in curr_modpaths: + if os.path.exists(full_mod_path) and full_mod_path not in curr_modpaths: self.modules_tool.prepend_module_path(full_mod_path) # prepare toolchain: load toolchain module and dependencies, set up build environment diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index a2e2709917..f6058e9e42 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -508,6 +508,24 @@ def det_common_path_prefix(paths): return None +def normalize_path(path): + """Normalize path removing empty and dot components. + + Similar to os.path.normpath but does not resolve '..' which may return a wrong path when symlinks are used + """ + # In POSIX 3 or more leading slashes are equivalent to 1 + if path.startswith(os.path.sep): + if path.startswith(os.path.sep * 2) and not path.startswith(os.path.sep * 3): + start_slashes = os.path.sep * 2 + else: + start_slashes = os.path.sep + else: + start_slashes = '' + + filtered_comps = (comp for comp in path.split(os.path.sep) if comp and comp != '.') + return start_slashes + os.path.sep.join(filtered_comps) + + def is_alt_pypi_url(url): """Determine whether specified URL is already an alternate PyPI URL, i.e. whether it contains a hash.""" # example: .../packages/5b/03/e135b19fadeb9b1ccb45eac9f60ca2dc3afe72d099f6bd84e03cb131f9bf/easybuild-2.7.0.tar.gz diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 3e3f004dc5..a93b16290b 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -46,7 +46,7 @@ from easybuild.tools.config import EBROOT_ENV_VAR_ACTIONS, LOADED_MODULES_ACTIONS from easybuild.tools.config import build_option, get_modules_tool, install_path from easybuild.tools.environment import ORIG_OS_ENVIRON, restore_env, setvar, unset_env_vars -from easybuild.tools.filetools import convert_name, mkdir, path_matches, read_file, which, write_file +from easybuild.tools.filetools import convert_name, mkdir, normalize_path, path_matches, read_file, which, write_file from easybuild.tools.module_naming_scheme.mns import DEVEL_MODULE_SUFFIX from easybuild.tools.py2vs3 import subprocess_popen_text from easybuild.tools.run import run_cmd @@ -362,8 +362,12 @@ def use(self, path, priority=None): self.log.info("Ignoring specified priority '%s' when running 'module use %s' (Lmod-specific)", priority, path) - # make sure path exists before we add it - mkdir(path, parents=True) + if not path: + raise EasyBuildError("Cannot add empty path to $MODULEPATH") + if not os.path.exists(path): + self.log.deprecated("Path '%s' for module.use should exist" % path, '5.0') + # make sure path exists before we add it + mkdir(path, parents=True) self.run_module(['use', path]) def unuse(self, path): @@ -377,7 +381,8 @@ def add_module_path(self, path, set_mod_paths=True): :param path: path to add to $MODULEPATH via 'use' :param set_mod_paths: (re)set self.mod_paths """ - if path not in curr_module_paths(): + path = normalize_path(path) + if path not in curr_module_paths(normalize=True): # add module path via 'module use' and make sure self.mod_paths is synced self.use(path) if set_mod_paths: @@ -391,8 +396,14 @@ def remove_module_path(self, path, set_mod_paths=True): :param set_mod_paths: (re)set self.mod_paths """ # remove module path via 'module unuse' and make sure self.mod_paths is synced - if path in curr_module_paths(): - self.unuse(path) + path = normalize_path(path) + try: + # Unuse the path that is actually present in the environment + module_path = next(p for p in curr_module_paths() if normalize_path(p) == path) + except StopIteration: + pass + else: + self.unuse(module_path) if set_mod_paths: self.set_mod_paths() @@ -431,6 +442,7 @@ def check_module_path(self): eb_modpath = os.path.join(install_path(typ='modules'), build_option('suffix_modules_path')) # make sure EasyBuild module path is in 1st place + mkdir(eb_modpath, parents=True) self.prepend_module_path(eb_modpath) self.log.info("Prepended list of module paths with path used by EasyBuild: %s" % eb_modpath) @@ -665,7 +677,8 @@ def load(self, modules, mod_paths=None, purge=False, init_env=None, allow_reload # extend $MODULEPATH if needed for mod_path in mod_paths: full_mod_path = os.path.join(install_path('mod'), build_option('suffix_modules_path'), mod_path) - self.prepend_module_path(full_mod_path) + if os.path.exists(full_mod_path): + self.prepend_module_path(full_mod_path) loaded_modules = self.loaded_modules() for mod in modules: @@ -1286,8 +1299,14 @@ def remove_module_path(self, path, set_mod_paths=True): # remove module path via 'module use' and make sure self.mod_paths is synced # modulecmd.tcl keeps track of how often a path was added via 'module use', # so we need to check to make sure it's really removed - while path in curr_module_paths(): - self.unuse(path) + path = normalize_path(path) + while True: + try: + # Unuse the path that is actually present in the environment + module_path = next(p for p in curr_module_paths() if normalize_path(p) == path) + except StopIteration: + break + self.unuse(module_path) if set_mod_paths: self.set_mod_paths() @@ -1422,8 +1441,12 @@ def use(self, path, priority=None): :param path: path to add to $MODULEPATH :param priority: priority for this path in $MODULEPATH (Lmod-specific) """ - # make sure path exists before we add it - mkdir(path, parents=True) + if not path: + raise EasyBuildError("Cannot add empty path to $MODULEPATH") + if not os.path.exists(path): + self.log.deprecated("Path '%s' for module.use should exist" % path, '5.0') + # make sure path exists before we add it + mkdir(path, parents=True) if priority: self.run_module(['use', '--priority', str(priority), path]) @@ -1433,11 +1456,12 @@ def use(self, path, priority=None): if os.environ.get('__LMOD_Priority_MODULEPATH'): self.run_module(['use', path]) else: + path = normalize_path(path) cur_mod_path = os.environ.get('MODULEPATH') if cur_mod_path is None: new_mod_path = path else: - new_mod_path = [path] + [p for p in cur_mod_path.split(':') if p != path] + new_mod_path = [path] + [p for p in cur_mod_path.split(':') if normalize_path(p) != path] new_mod_path = ':'.join(new_mod_path) self.log.debug('Changing MODULEPATH from %s to %s' % ('' if cur_mod_path is None else cur_mod_path, new_mod_path)) @@ -1453,7 +1477,8 @@ def unuse(self, 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) + path = normalize_path(path) + new_mod_path = ':'.join(p for p in cur_mod_path.split(':') if normalize_path(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 @@ -1603,12 +1628,17 @@ def get_software_version(name): return version -def curr_module_paths(): +def curr_module_paths(normalize=False): """ Return a list of current module paths. + + :param normalize: Normalize the paths """ # avoid empty or nonexistent paths, which don't make any sense - return [p for p in os.environ.get('MODULEPATH', '').split(':') if p and os.path.exists(p)] + module_paths = (p for p in os.environ.get('MODULEPATH', '').split(':') if p and os.path.exists(p)) + if normalize: + module_paths = (normalize_path(p) for p in module_paths) + return list(module_paths) def mk_module_path(paths): diff --git a/easybuild/tools/toolchain/toolchain.py b/easybuild/tools/toolchain/toolchain.py index ca2f732824..985db8733e 100644 --- a/easybuild/tools/toolchain/toolchain.py +++ b/easybuild/tools/toolchain/toolchain.py @@ -634,7 +634,9 @@ def _load_toolchain_module(self, silent=False): if self.init_modpaths: mod_path_suffix = build_option('suffix_modules_path') for modpath in self.init_modpaths: - self.modules_tool.prepend_module_path(os.path.join(install_path('mod'), mod_path_suffix, modpath)) + modpath = os.path.join(install_path('mod'), mod_path_suffix, modpath) + if os.path.exists(modpath): + self.modules_tool.prepend_module_path(modpath) # load modules for all dependencies self.log.debug("Loading module for toolchain: %s", tc_mod) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index f1a3b62efc..21247f2ce1 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -1655,7 +1655,6 @@ def test_external_dependencies(self): # by adding a couple of matching module files with some useful data in them # (use Tcl syntax, so it works with all varieties of module tools) mod_dir = os.path.join(self.test_prefix, 'modules') - self.modtool.use(mod_dir) pi_mod_txt = '\n'.join([ "#%Module", @@ -1680,6 +1679,8 @@ def test_external_dependencies(self): ]) write_file(os.path.join(mod_dir, 'foobar/2.3.4'), foobar_mod_txt) + self.modtool.use(mod_dir) + ec = EasyConfig(toy_ec) deps = ec.dependencies() diff --git a/test/framework/filetools.py b/test/framework/filetools.py index fb9c2440e4..7b176f2d83 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -351,6 +351,21 @@ def test_common_path_prefix(self): self.assertEqual(ft.det_common_path_prefix(['foo']), None) self.assertEqual(ft.det_common_path_prefix([]), None) + def test_normalize_path(self): + """Test normalize_path""" + self.assertEqual(ft.normalize_path(''), '') + self.assertEqual(ft.normalize_path('/'), '/') + self.assertEqual(ft.normalize_path('//'), '//') + self.assertEqual(ft.normalize_path('///'), '/') + self.assertEqual(ft.normalize_path('/foo/bar/baz'), '/foo/bar/baz') + self.assertEqual(ft.normalize_path('/foo//bar/././baz/'), '/foo/bar/baz') + self.assertEqual(ft.normalize_path('foo//bar/././baz/'), 'foo/bar/baz') + self.assertEqual(ft.normalize_path('//foo//bar/././baz/'), '//foo/bar/baz') + self.assertEqual(ft.normalize_path('///foo//bar/././baz/'), '/foo/bar/baz') + self.assertEqual(ft.normalize_path('////foo//bar/././baz/'), '/foo/bar/baz') + self.assertEqual(ft.normalize_path('/././foo//bar/././baz/'), '/foo/bar/baz') + self.assertEqual(ft.normalize_path('//././foo//bar/././baz/'), '//foo/bar/baz') + def test_download_file(self): """Test download_file function.""" fn = 'toy-0.0.tar.gz' diff --git a/test/framework/modules.py b/test/framework/modules.py index a2f4633787..432570c385 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -531,14 +531,15 @@ def test_check_module_path(self): os.environ['MODULEPATH'] = test2 modtool.check_module_path() self.assertEqual(modtool.mod_paths, [mod_install_dir, test1, test2]) - self.assertEqual(os.environ['MODULEPATH'], mod_install_dir + ':' + test1 + ':' + test2) + self.assertEqual(os.environ['MODULEPATH'], os.pathsep.join([mod_install_dir, test1, test2])) # check behaviour if non-existing directories are included in $MODULEPATH os.environ['MODULEPATH'] = '%s:/does/not/exist:%s' % (test3, test2) modtool.check_module_path() # non-existing dir is filtered from mod_paths, but stays in $MODULEPATH self.assertEqual(modtool.mod_paths, [mod_install_dir, test1, test3, test2]) - self.assertEqual(os.environ['MODULEPATH'], ':'.join([mod_install_dir, test1, test3, '/does/not/exist', test2])) + self.assertEqual(os.environ['MODULEPATH'], + os.pathsep.join([mod_install_dir, test1, test3, '/does/not/exist', test2])) def test_check_module_path_hmns(self): """Test behaviour of check_module_path with HierarchicalMNS.""" @@ -1089,8 +1090,9 @@ def test_module_caches(self): """Test module caches and invalidate_module_caches_for function.""" self.assertEqual(mod.MODULE_AVAIL_CACHE, {}) - # purposely extending $MODULEPATH with non-existing path, should be handled fine + # purposely extending $MODULEPATH with an empty path, should be handled fine nonpath = os.path.join(self.test_prefix, 'nosuchfileordirectory') + mkdir(nonpath) self.modtool.use(nonpath) modulepaths = [p for p in os.environ.get('MODULEPATH', '').split(os.pathsep) if p] self.assertTrue(any([os.path.samefile(nonpath, mp) for mp in modulepaths])) @@ -1163,6 +1165,11 @@ def test_module_use_unuse(self): self.modtool.use(test_dir3) self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir3)) + # Adding an empty modulepath is not possible + modulepath = os.environ.get('MODULEPATH', '') + self.assertErrorRegex(EasyBuildError, "Cannot add empty path", self.modtool.use, '') + self.assertEqual(os.environ.get('MODULEPATH', ''), modulepath) + # make sure the right test module is loaded self.modtool.load(['test']) self.assertEqual(os.getenv('TEST123'), 'three') @@ -1223,17 +1230,67 @@ 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_add_and_remove_module_path(self): + """Test add_module_path and whether remove_module_path undoes changes of add_module_path""" + test_dir1 = tempfile.mkdtemp(suffix="_dir1") + test_dir2 = tempfile.mkdtemp(suffix="_dir2") + old_module_path = os.environ.get('MODULEPATH') + del os.environ['MODULEPATH'] + self.modtool.add_module_path(test_dir1) + self.assertEqual(os.environ['MODULEPATH'], test_dir1) + self.modtool.add_module_path(test_dir2) + test_dir_2_and_1 = os.pathsep.join([test_dir2, test_dir1]) + self.assertEqual(os.environ['MODULEPATH'], test_dir_2_and_1) + # Adding the same path does not change the path + self.modtool.add_module_path(test_dir1) + self.assertEqual(os.environ['MODULEPATH'], test_dir_2_and_1) + self.modtool.add_module_path(test_dir2) + self.assertEqual(os.environ['MODULEPATH'], test_dir_2_and_1) + # Even when a (meaningless) slash is added + # This occurs when using an empty modules directory name + self.modtool.add_module_path(os.path.join(test_dir1, '')) + self.assertEqual(os.environ['MODULEPATH'], test_dir_2_and_1) + + # Similar tests for remove_module_path + self.modtool.remove_module_path(test_dir2) + self.assertEqual(os.environ['MODULEPATH'], test_dir1) + # Same again -> no-op + self.modtool.remove_module_path(test_dir2) + self.assertEqual(os.environ['MODULEPATH'], test_dir1) + # And with empty last part + self.modtool.remove_module_path(os.path.join(test_dir1, '')) + self.assertEqual(os.environ.get('MODULEPATH', ''), '') + + # And with some more trickery + # Lmod seems to remove empty paths: /foo//bar/. -> /foo/bar + # Environment-Modules 4.x seems to resolve relative paths: /foo/../foo -> /foo + # Hence we can only check the real paths + def get_resolved_module_path(): + return os.pathsep.join(os.path.realpath(p) for p in os.environ['MODULEPATH'].split(os.pathsep)) + + test_dir1_relative = os.path.join(test_dir1, '..', os.path.basename(test_dir1)) + test_dir2_dot = os.path.join(os.path.dirname(test_dir2), '.', os.path.basename(test_dir2)) + self.modtool.add_module_path(test_dir1_relative) + self.assertEqual(get_resolved_module_path(), test_dir1) + # Adding the same path, but in a different form may be possible, but may also be ignored, e.g. in EnvModules + self.modtool.add_module_path(test_dir1) + if get_resolved_module_path() != test_dir1: + self.assertEqual(get_resolved_module_path(), os.pathsep.join([test_dir1, test_dir1])) + self.modtool.remove_module_path(test_dir1) + self.assertEqual(get_resolved_module_path(), test_dir1) + self.modtool.add_module_path(test_dir2_dot) + self.assertEqual(get_resolved_module_path(), test_dir_2_and_1) + self.modtool.remove_module_path(test_dir2_dot) + self.assertEqual(get_resolved_module_path(), test_dir1) + # Force adding such a dot path which can be removed with either variant + os.environ['MODULEPATH'] = os.pathsep.join([test_dir2_dot, test_dir1_relative]) + self.modtool.remove_module_path(test_dir2_dot) + self.assertEqual(get_resolved_module_path(), test_dir1) + os.environ['MODULEPATH'] = os.pathsep.join([test_dir2_dot, test_dir1_relative]) + self.modtool.remove_module_path(test_dir2) + self.assertEqual(get_resolved_module_path(), test_dir1) + + 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."""