Add join_path_var function to join list of paths into a colon-separated string value

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()

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:

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

test/framework/environment.py

@@ -31,13 +31,28 @@
 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
 
 
 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,

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

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