diff --git a/easybuild/toolchains/compiler/fujitsu.py b/easybuild/toolchains/compiler/fujitsu.py index c5e8ded0e1..1c9ce4aca4 100644 --- a/easybuild/toolchains/compiler/fujitsu.py +++ b/easybuild/toolchains/compiler/fujitsu.py @@ -103,7 +103,7 @@ def prepare(self, *args, **kwargs): libdir = os.path.join(os.getenv(TC_CONSTANT_MODULE_VAR), 'lib64') if libdir not in library_path: self.log.debug("Adding %s to $LIBRARY_PATH" % libdir) - env.setvar('LIBRARY_PATH', os.pathsep.join([library_path, libdir])) + env.setvar('LIBRARY_PATH', env.join_path_var([library_path, libdir])) def _set_compiler_vars(self): super()._set_compiler_vars() diff --git a/easybuild/tools/environment.py b/easybuild/tools/environment.py index b1a371f099..52e5889692 100644 --- a/easybuild/tools/environment.py +++ b/easybuild/tools/environment.py @@ -32,6 +32,8 @@ """ import copy import os +from pathlib import Path +from typing import List, Union from easybuild.base import fancylogger from easybuild.tools.build_log import EasyBuildError, dry_run_msg @@ -76,6 +78,11 @@ def get_changes(): return _changes +def join_path_var(paths: List[Union[str, Path]]) -> str: + """Join a list of paths into a single path variable string, filtering out empty entries.""" + return os.pathsep.join(str(path) for path in paths if path) + + def copy_current_env(): """ Copy current environment, and return it. @@ -222,7 +229,7 @@ def sanitize_env(): entries = val.split(os.pathsep) if '' in entries: _log.info("Found %d empty entries in $%s, filtering them out...", entries.count(''), key) - newval = os.pathsep.join(x for x in entries if x) + newval = join_path_var(entries) if newval: setvar(key, newval) else: diff --git a/easybuild/tools/toolchain/toolchain.py b/easybuild/tools/toolchain/toolchain.py index e07268c7fa..fcad80e318 100644 --- a/easybuild/tools/toolchain/toolchain.py +++ b/easybuild/tools/toolchain/toolchain.py @@ -61,7 +61,7 @@ from easybuild.base import fancylogger from easybuild.tools.build_log import EasyBuildError, dry_run_msg, print_warning from easybuild.tools.config import build_option, install_path -from easybuild.tools.environment import setvar +from easybuild.tools.environment import setvar, join_path_var from easybuild.tools.filetools import adjust_permissions, copy_file, find_eb_script, mkdir, read_file, which, write_file from easybuild.tools.module_generator import dependencies_for from easybuild.tools.modules import get_software_root, get_software_root_env_var_name @@ -431,7 +431,7 @@ def show_variables(self, offset='', sep='\n', verbose=False): if verbose: res.append("# type %s" % (type(self.variables[v]))) res.append("# %s" % (self.variables[v].show_el())) - res.append("# repr %s" % (self.variables[v].__repr__())) + res.append("# repr %s" % (repr(self.variables[v]))) if offset is None: offset = '' @@ -604,7 +604,7 @@ def _check_dependencies(self, dependencies, check_modules=True): return deps - def is_required(self, name): + def is_required(self, _name): """Determine whether this is a required toolchain element.""" # default: assume every element is required return True @@ -1174,8 +1174,9 @@ def handle_sysroot(self): if not any(os.path.exists(x) and os.path.samefile(x, sysroot_pc_path) for x in pkg_config_path): pkg_config_path.append(sysroot_pc_path) + pkg_config_path = join_path_var(pkg_config_path) if pkg_config_path: - setvar('PKG_CONFIG_PATH', os.pathsep.join(pkg_config_path)) + setvar('PKG_CONFIG_PATH', pkg_config_path) def _add_dependency_variables(self, names=None, cpp=None, ld=None): """ diff --git a/test/framework/environment.py b/test/framework/environment.py index 64467a9d3b..1e9a45dd33 100644 --- a/test/framework/environment.py +++ b/test/framework/environment.py @@ -31,6 +31,7 @@ import sys from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, init_config from unittest import TextTestRunner +from pathlib import Path import easybuild.tools.environment as env @@ -38,6 +39,20 @@ class EnvironmentTest(EnhancedTestCase): """ Testcase for run module """ + def test_join_path_var(self): + """Test join_path_var function.""" + # join_path_var is same as os.pathsep.join if there are no empty paths involved + paths = ['/foo', '/bar', '/baz'] + self.assertEqual(env.join_path_var(paths), os.pathsep.join(paths)) + + paths = ['/foo', '', '/bar', None, '/baz'] + self.assertEqual(env.join_path_var(paths), '/foo:/bar:/baz') + self.assertEqual(env.join_path_var([Path(p) if p else p for p in paths]), '/foo:/bar:/baz') + self.assertEqual(env.join_path_var(['foo']), 'foo') + self.assertEqual(env.join_path_var(['foo', '']), 'foo') + self.assertEqual(env.join_path_var(['', 'foo']), 'foo') + self.assertEqual(env.join_path_var(['']), '') + def test_setvar(self): """Test setvar function.""" self.mock_stdout(True) @@ -91,8 +106,8 @@ def test_modify_env(self): # keys in new_env should not be set yet, keys in old_env are expected to be set for key in new_env_vars: os.environ.pop(key, None) - for key in old_env_vars: - os.environ[key] = old_env_vars[key] + for key, value in old_env_vars.items(): + os.environ[key] = value env.modify_env(os.environ, new_env_vars) @@ -147,7 +162,7 @@ def test_sanitize_env(self): env.sanitize_env() - self.assertFalse(any(x for x in os.environ.keys() if x.startswith('PYTHON'))) + self.assertFalse(any(x for x in os.environ if x.startswith('PYTHON'))) expected = { 'CPATH': self.test_prefix, diff --git a/test/framework/modules.py b/test/framework/modules.py index f3272cfa09..09c85977fc 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -44,6 +44,7 @@ from easybuild.framework.easyconfig.easyconfig import EasyConfig from easybuild.tools import LooseVersion from easybuild.tools.build_log import EasyBuildError +from easybuild.tools.environment import join_path_var from easybuild.tools.filetools import adjust_permissions, copy_file, copy_dir, mkdir from easybuild.tools.filetools import read_file, remove_dir, remove_file, symlink, write_file from easybuild.tools.modules import EnvironmentModules, EnvironmentModulesC, EnvironmentModulesTcl, Lmod, NoModulesTool @@ -1324,7 +1325,7 @@ def test_add_and_remove_module_path(self): # 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)) + return join_path_var(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)) diff --git a/test/framework/modulestool.py b/test/framework/modulestool.py index 2daa9e3dc8..6c9cefd95d 100644 --- a/test/framework/modulestool.py +++ b/test/framework/modulestool.py @@ -38,6 +38,7 @@ from easybuild.base import fancylogger from easybuild.tools import modules, LooseVersion from easybuild.tools.build_log import EasyBuildError +from easybuild.tools.environment import join_path_var from easybuild.tools.filetools import read_file, which, write_file from easybuild.tools.modules import EnvironmentModules, Lmod from test.framework.utilities import init_config @@ -162,7 +163,7 @@ def test_lmod_specific(self): spider_cand_path = os.path.join(path, 'spider') if not os.path.isfile(lmod_cand_path) and not os.path.isfile(spider_cand_path): new_paths.append(path) - os.environ['PATH'] = os.pathsep.join(new_paths) + os.environ['PATH'] = join_path_var(new_paths) # make sure $MODULEPATH contains path that provides some modules os.environ['MODULEPATH'] = os.path.abspath(os.path.join(os.path.dirname(__file__), 'modules'))