Fix UTF-8 encoding errors when running EasyBuild with Python 3.0.x-3.6.x

.github/workflows/unit_tests.yml

@@ -9,6 +9,7 @@ jobs:
         python: [2.7, 3.5, 3.6, 3.7, 3.8, 3.9]
         modules_tool: [Lmod-7.8.22, Lmod-8.2.9, modules-tcl-1.147, modules-3.2.10, modules-4.1.4]
         module_syntax: [Lua, Tcl]
+        lc_all: [""]
         # exclude some configuration for non-Lmod modules tool:
         # - don't test with Lua module syntax (only supported in Lmod)
         # - exclude Python 3.x versions other than 3.6, to limit test configurations
@@ -51,6 +52,13 @@ jobs:
             python: 3.8
           - modules_tool: Lmod-7.8.22
             python: 3.9
+        # There may be encoding errors in Python 3 which are hidden when an UTF-8 encoding is set
+        # Hence run the tests (again) with LC_ALL=C and Python 3.6 (or any < 3.7)
+        include:
+          - python: 3.6
+            modules_tool: Lmod-8.2.9
+            module_syntax: Lua
+            lc_all: C
       fail-fast: false
     steps:
     - uses: actions/checkout@v2
@@ -129,6 +137,7 @@ jobs:
         EB_VERBOSE: 1
         EASYBUILD_MODULE_SYNTAX: ${{matrix.module_syntax}}
         TEST_EASYBUILD_MODULE_SYNTAX: ${{matrix.module_syntax}}
+        LC_ALL: ${{matrix.lc_all}}
       run: |
           # run tests *outside* of checked out easybuild-framework directory,
           # to ensure we're testing installed version (see previous step)

easybuild/base/fancylogger.py

@@ -580,6 +580,8 @@ def logToFile(filename, enable=True, filehandler=None, name=None, max_bytes=MAX_
         'maxBytes': max_bytes,
         'backupCount': backup_count,
     }
+    if sys.version_info[0] >= 3:
+        handleropts['encoding'] = 'utf-8'
     # logging to a file is going to create the file later on, so let's try to be helpful and create the path if needed
     directory = os.path.dirname(filename)
     if not os.path.exists(directory):

easybuild/base/optcomplete.py

@@ -594,9 +594,8 @@ def autocomplete(parser, arg_completer=None, opt_completer=None, subcmd_complete
         if isinstance(debugfn, logging.Logger):
             debugfn.debug(txt)
         else:
-            f = open(debugfn, 'a')
-            f.write(txt)
-            f.close()
+            with open(debugfn, 'a') as fh:
+                fh.write(txt)
 
     # Exit with error code (we do not let the caller continue on purpose, this
     # is a run for completions only.)

easybuild/scripts/fix_docs.py

@@ -55,15 +55,14 @@
     # exclude self
     if os.path.basename(tmp) == os.path.basename(__file__):
         continue
-    with open(tmp) as f:
+    with open(tmp) as fh:
         temp = "tmp_file.py"
-        out = open(temp, 'w')
-        for line in f:
-            if "@author" in line:
-                out.write(re.sub(r"@author: (.*)", r":author: \1", line))
-            elif "@param" in line:
-                out.write(re.sub(r"@param ([^:]*):", r":param \1:", line))
-            else:
-                out.write(line)
-        out.close()
+        with open(temp, 'w') as out:
+            for line in fh:
+                if "@author" in line:
+                    out.write(re.sub(r"@author: (.*)", r":author: \1", line))
+                elif "@param" in line:
+                    out.write(re.sub(r"@param ([^:]*):", r":param \1:", line))
+                else:
+                    out.write(line)
         os.rename(temp, tmp)

easybuild/scripts/mk_tmpl_easyblock_for.py

@@ -225,9 +225,8 @@ def make_module_extra(self):
     dirpath = os.path.dirname(easyblock_path)
     if not os.path.exists(dirpath):
         os.makedirs(dirpath)
-    f = open(easyblock_path, "w")
-    f.write(txt)
-    f.close()
+    with open(easyblock_path, "w") as fh:
+        fh.write(txt)
 except (IOError, OSError) as err:
     sys.stderr.write("ERROR! Writing template easyblock for %s to %s failed: %s" % (name, easyblock_path, err))
     sys.exit(1)

easybuild/tools/configobj.py

@@ -1213,9 +1213,8 @@ def _load(self, infile, configspec):
         if isinstance(infile, string_type):
             self.filename = infile
             if os.path.isfile(infile):
-                h = open(infile, 'r')
-                infile = h.read() or []
-                h.close()
+                with open(infile, 'r') as fh:
+                    infile = fh.read() or []
             elif self.file_error:
                 # raise an error if the file doesn't exist
                 raise IOError('Config file not found: "%s".' % self.filename)
@@ -1224,9 +1223,8 @@ def _load(self, infile, configspec):
                 if self.create_empty:
                     # this is a good test that the filename specified
                     # isn't impossible - like on a non-existent device
-                    h = open(infile, 'w')
-                    h.write('')
-                    h.close()
+                    with open(infile, 'w') as fh:
+                        fh.write('')
                 infile = []
 
         elif isinstance(infile, (list, tuple)):
@@ -2052,9 +2050,8 @@ def write(self, outfile=None, section=None):
         if outfile is not None:
             outfile.write(output)
         else:
-            h = open(self.filename, 'wb')
-            h.write(output)
-            h.close()
+            with open(self.filename, 'wb') as fh:
+                fh.write(output)
 
     def validate(self, validator, preserve_errors=False, copy=False,
                  section=None):

easybuild/tools/environment.py

@@ -50,17 +50,11 @@ def write_changes(filename):
     """
     Write current changes to filename and reset environment afterwards
     """
-    script = None
     try:
-        script = open(filename, 'w')
-
-        for key in _changes:
-            script.write('export %s=%s\n' % (key, shell_quote(_changes[key])))
-
-        script.close()
+        with open(filename, 'w') as script:
+            for key in _changes:
+                script.write('export %s=%s\n' % (key, shell_quote(_changes[key])))
     except IOError as err:
-        if script is not None:
-            script.close()
         raise EasyBuildError("Failed to write to %s: %s", filename, err)
     reset_changes()
 

easybuild/tools/filetools.py

@@ -40,7 +40,6 @@
 """
 import datetime
 import difflib
-import fileinput
 import glob
 import hashlib
 import imp
@@ -189,11 +188,21 @@ def is_readable(path):
         raise EasyBuildError("Failed to check whether %s is readable: %s", path, err)
 
 
+def open_file(path, mode):
+    """Open a (usually) text file. If mode is not binary, then utf-8 encoding will be used for Python 3.x"""
+    # This is required for text files in Python 3, especially until Python 3.7 which implements PEP 540.
+    # This PEP opens files in UTF-8 mode if the C locale is used, see https://www.python.org/dev/peps/pep-0540
+    if sys.version_info[0] >= 3 and 'b' not in mode:
+        return open(path, mode, encoding='utf-8')
+    else:
+        return open(path, mode)
+
+
 def read_file(path, log_error=True, mode='r'):
     """Read contents of file at given path, in a robust way."""
     txt = None
     try:
-        with open(path, mode) as handle:
+        with open_file(path, mode) as handle:
             txt = handle.read()
     except IOError as err:
         if log_error:
@@ -244,8 +253,8 @@ def write_file(path, data, append=False, forced=False, backup=False, always_over
     # note: we can't use try-except-finally, because Python 2.4 doesn't support it as a single block
     try:
         mkdir(os.path.dirname(path), parents=True)
-        with open(path, mode) as handle:
-            handle.write(data)
+        with open_file(path, mode) as fh:
+            fh.write(data)
     except IOError as err:
         raise EasyBuildError("Failed to write to %s: %s", path, err)
 
@@ -1011,10 +1020,9 @@ def calc_block_checksum(path, algorithm):
     _log.debug("Using blocksize %s for calculating the checksum" % blocksize)
 
     try:
-        f = open(path, 'rb')
-        for block in iter(lambda: f.read(blocksize), b''):
-            algorithm.update(block)
-        f.close()
+        with open(path, 'rb') as fh:
+            for block in iter(lambda: fh.read(blocksize), b''):
+                algorithm.update(block)
     except IOError as err:
         raise EasyBuildError("Failed to read %s: %s", path, err)
 
@@ -1360,35 +1368,37 @@ def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb'):
         for regex, subtxt in regex_subs:
             compiled_regex_subs.append((re.compile(regex), subtxt))
 
-        if backup:
-            backup_ext = backup
-        else:
-            # no (persistent) backup file is created if empty string value is passed to 'backup' in fileinput.input
-            backup_ext = ''
-
         for path in paths:
             try:
                 # make sure that file can be opened in text mode;
                 # it's possible this fails with UnicodeDecodeError when running EasyBuild with Python 3
                 try:
-                    with open(path, 'r') as fp:
-                        _ = fp.read()
+                    with open_file(path, 'r') as fp:
+                        txt_utf8 = fp.read()
                 except UnicodeDecodeError as err:
                     _log.info("Encountered UnicodeDecodeError when opening %s in text mode: %s", path, err)
                     path_backup = back_up_file(path)
                     _log.info("Editing %s to strip out non-UTF-8 characters (backup at %s)", path, path_backup)
                     txt = read_file(path, mode='rb')
                     txt_utf8 = txt.decode(encoding='utf-8', errors='replace')
+                    del txt
                     write_file(path, txt_utf8)
 
-                for line_id, line in enumerate(fileinput.input(path, inplace=1, backup=backup_ext)):
-                    for regex, subtxt in compiled_regex_subs:
-                        match = regex.search(line)
-                        if match:
-                            origtxt = match.group(0)
-                            _log.info("Replacing line %d in %s: '%s' -> '%s'", (line_id + 1), path, origtxt, subtxt)
-                        line = regex.sub(subtxt, line)
-                    sys.stdout.write(line)
+                if backup:
+                    copy_file(path, path + backup)
+                with open_file(path, 'w') as out_file:
+                    lines = txt_utf8.split('\n')
+                    del txt_utf8
+                    for line_id, line in enumerate(lines):
+                        for regex, subtxt in compiled_regex_subs:
+                            match = regex.search(line)
+                            if match:
+                                origtxt = match.group(0)
+                                _log.info("Replacing line %d in %s: '%s' -> '%s'",
+                                          (line_id + 1), path, origtxt, subtxt)
+                                line = regex.sub(subtxt, line)
+                                lines[line_id] = line
+                    out_file.write('\n'.join(lines))
 
             except (IOError, OSError) as err:
                 raise EasyBuildError("Failed to patch %s: %s", path, err)
@@ -1563,16 +1573,16 @@ def mkdir(path, parents=False, set_gid=None, sticky=None):
     :param sticky: set the sticky bit on this directory (a.k.a. the restricted deletion flag),
                    to avoid users can removing/renaming files in this directory
     """
-    if set_gid is None:
-        set_gid = build_option('set_gid_bit')
-    if sticky is None:
-        sticky = build_option('sticky_bit')
-
     if not os.path.isabs(path):
         path = os.path.abspath(path)
 
     # exit early if path already exists
     if not os.path.exists(path):
+        if set_gid is None:
+            set_gid = build_option('set_gid_bit')
+        if sticky is None:
+            sticky = build_option('sticky_bit')
+
         _log.info("Creating directory %s (parents: %s, set_gid: %s, sticky: %s)", path, parents, set_gid, sticky)
         # set_gid and sticky bits are only set on new directories, so we need to determine the existing parent path
         existing_parent_path = os.path.dirname(path)
@@ -2042,7 +2052,8 @@ def find_flexlm_license(custom_env_vars=None, lic_specs=None):
             if lic_files:
                 for lic_file in lic_files:
                     try:
-                        open(lic_file, 'r')
+                        # just try to open file for reading, no need to actually read it
+                        open(lic_file, 'rb').close()
                         valid_lic_specs.append(lic_file)
                     except IOError as err:
                         _log.warning("License file %s found, but failed to open it for reading: %s", lic_file, err)
@@ -2376,7 +2387,7 @@ def install_fake_vsc():
     fake_vsc_init_path = os.path.join(fake_vsc_path, 'vsc', '__init__.py')
     if not os.path.exists(os.path.dirname(fake_vsc_init_path)):
         os.makedirs(os.path.dirname(fake_vsc_init_path))
-    with open(fake_vsc_init_path, 'w') as fp:
+    with open_file(fake_vsc_init_path, 'w') as fp:
         fp.write(fake_vsc_init)
 
     sys.path.insert(0, fake_vsc_path)

easybuild/tools/jenkins.py

@@ -105,9 +105,8 @@ def create_success(name, stats):
         root.firstChild.appendChild(el)
 
     try:
-        output_file = open(filename, "w")
-        root.writexml(output_file)
-        output_file.close()
+        with open(filename, "w") as output_file:
+            root.writexml(output_file)
     except IOError as err:
         raise EasyBuildError("Failed to write out XML file %s: %s", filename, err)
 
@@ -162,9 +161,8 @@ def aggregate_xml_in_dirs(base_dir, output_filename):
     comment = root.createComment("%s out of %s builds succeeded" % (succes, total))
     root.firstChild.insertBefore(comment, properties)
     try:
-        output_file = open(output_filename, "w")
-        root.writexml(output_file, addindent="\t", newl="\n")
-        output_file.close()
+        with open(output_filename, "w") as output_file:
+            root.writexml(output_file, addindent="\t", newl="\n")
     except IOError as err:
         raise EasyBuildError("Failed to write out XML file %s: %s", output_filename, err)
 

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
+from easybuild.tools.filetools import convert_name, mkdir, 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
@@ -1403,17 +1403,12 @@ def update(self):
                 # don't actually update local cache when testing, just return the cache contents
                 return stdout
             else:
-                try:
-                    cache_fp = os.path.join(self.USER_CACHE_DIR, 'moduleT.lua')
-                    self.log.debug("Updating Lmod spider cache %s with output from '%s'" % (cache_fp, ' '.join(cmd)))
-                    cache_dir = os.path.dirname(cache_fp)
-                    if not os.path.exists(cache_dir):
-                        mkdir(cache_dir, parents=True)
-                    cache_file = open(cache_fp, 'w')
-                    cache_file.write(stdout)
-                    cache_file.close()
-                except (IOError, OSError) as err:
-                    raise EasyBuildError("Failed to update Lmod spider cache %s: %s", cache_fp, err)
+                cache_fp = os.path.join(self.USER_CACHE_DIR, 'moduleT.lua')
+                self.log.debug("Updating Lmod spider cache %s with output from '%s'" % (cache_fp, ' '.join(cmd)))
+                cache_dir = os.path.dirname(cache_fp)
+                if not os.path.exists(cache_dir):
+                    mkdir(cache_dir, parents=True)
+                write_file(cache_fp, stdout)
 
     def use(self, path, priority=None):
         """

test/framework/build_log.py

@@ -70,7 +70,7 @@ def test_easybuilderror(self):
         logToFile(tmplog, enable=False)
 
         log_re = re.compile(r"^fancyroot ::.* BOOM \(at .*:[0-9]+ in [a-z_]+\)$", re.M)
-        logtxt = open(tmplog, 'r').read()
+        logtxt = read_file(tmplog, 'r')
         self.assertTrue(log_re.match(logtxt), "%s matches %s" % (log_re.pattern, logtxt))
 
         # test formatting of message