[refactor] Use filetools functions to safely create error log directory

easybuild/framework/easyblock.py

@@ -75,16 +75,17 @@
 from easybuild.tools.config import FORCE_DOWNLOAD_ALL, FORCE_DOWNLOAD_PATCHES, FORCE_DOWNLOAD_SOURCES
 from easybuild.tools.config import EASYBUILD_SOURCES_URL # noqa
 from easybuild.tools.config import build_option, build_path, get_log_filename, get_repository, get_repositorypath
-from easybuild.tools.config import install_path, log_path, package_path, source_paths, error_log_path
+from easybuild.tools.config import install_path, log_path, package_path, source_paths, get_error_log_dir
 from easybuild.tools.environment import restore_env, sanitize_env
 from easybuild.tools.filetools import CHECKSUM_TYPE_MD5, CHECKSUM_TYPE_SHA256
 from easybuild.tools.filetools import adjust_permissions, apply_patch, back_up_file, change_dir, check_lock
-from easybuild.tools.filetools import convert_name, copy_file, copy_dir, create_lock, create_patch_info, is_readable
-from easybuild.tools.filetools import derive_alt_pypi_url, diff_files, dir_contains_files, download_file
-from easybuild.tools.filetools import encode_class_name, extract_file, compute_checksum
+from easybuild.tools.filetools import convert_name, copy_file, copy_dir, create_lock, create_patch_info
+from easybuild.tools.filetools import create_unused_dir, derive_alt_pypi_url, diff_files, dir_contains_files
+from easybuild.tools.filetools import download_file, encode_class_name, extract_file, compute_checksum
 from easybuild.tools.filetools import find_backup_name_candidate, get_source_tarball_from_git, is_alt_pypi_url
-from easybuild.tools.filetools import is_binary, is_sha256_checksum, mkdir, move_file, move_logs, read_file, remove_dir
-from easybuild.tools.filetools import remove_file, remove_lock, verify_checksum, weld_paths, write_file, symlink
+from easybuild.tools.filetools import is_binary, is_readable, is_sha256_checksum, mkdir, move_file, move_logs
+from easybuild.tools.filetools import read_file, remove_dir, remove_file, remove_lock, verify_checksum, weld_paths
+from easybuild.tools.filetools import write_file, symlink
 from easybuild.tools.hooks import BUILD_STEP, CLEANUP_STEP, CONFIGURE_STEP, EXTENSIONS_STEP, FETCH_STEP, INSTALL_STEP
 from easybuild.tools.hooks import MODULE_STEP, MODULE_WRITE, PACKAGE_STEP, PATCH_STEP, PERMISSIONS_STEP, POSTITER_STEP
 from easybuild.tools.hooks import POSTPROC_STEP, PREPARE_STEP, READY_STEP, SANITYCHECK_STEP, SOURCE_STEP
@@ -4205,7 +4206,15 @@ def print_dry_run_note(loc, silent=True):
     dry_run_msg(msg, silent=silent)
 
 
-def persists_failed_compilation_log_and_artifacts(build_successful, application_log, silent, app, err_log_path):
+def persist_failed_compilation_log_and_artifacts(build_successful, application_log, silent, app, easyconfig):
+    err_log_dir = None
+    def get_unique_err_log_dir(err_log_dir_data):
+        nonlocal err_log_dir
+        if err_log_dir is None:
+            err_log_dir_path, err_log_dir_name = err_log_dir_data
+            err_log_dir = create_unused_dir(err_log_dir_path, err_log_dir_name)
+        return err_log_dir
+
     def do_if_paths_distinct(operation, source, destination):
         if not os.path.exists(source):
             return
@@ -4222,13 +4231,16 @@ def do_if_paths_distinct(operation, source, destination):
             silent=silent
         )
 
-        if err_log_path and not build_successful:
+        err_log_dir_data = get_error_log_dir(ec=easyconfig)
+        if not build_successful and err_log_dir_data is not None:
             for log_file in logs:
+                err_log_path = get_unique_err_log_dir(err_log_dir_data)
                 target_file = os.path.join(err_log_path, os.path.basename(log_file))
                 do_if_paths_distinct(copy_file, log_file, target_file)
 
             builddir = app.builddir
             if is_readable(builddir):
+                err_log_path = get_unique_err_log_dir(err_log_dir_data)
                 build_artifact_log_path = os.path.join(err_log_path, app.get_relative_builddir_base_path())
                 do_if_paths_distinct(copy_dir, builddir, build_artifact_log_path)
 
@@ -4497,8 +4509,7 @@ def ensure_writable_log_dir(log_dir):
         else:
             dry_run_msg("(no ignored errors during dry run)\n", silent=silent)
 
-    err_log_path = error_log_path(ec=ecdict['ec'])
-    persists_failed_compilation_log_and_artifacts(success, application_log, silent, app, err_log_path)
+    persist_failed_compilation_log_and_artifacts(success, application_log, silent, app, ecdict['ec'])
 
     del app
 

easybuild/tools/config.py

@@ -845,31 +845,29 @@ def log_path(ec=None):
     return log_file_format(return_directory=True, ec=ec, date=date, timestamp=timestamp)
 
 
-def error_log_path(ec=None):
+def get_error_log_dir(ec=None):
     """
-    Return the default error log path
+    Return a pair with the parent directory and directory name of the error log path
 
-    This is a path where files from the build_log_path can be stored permanently
-    :param ec: dict-like value that provides values for %(name)s and %(version)s template values
+    The error log path is a path where files from the build_log_path can be stored permanently
+    :param ec:  dict-like value with at least the keys 'name' and 'version' defined
     """
     errorlogpath = ConfigurationVariables()['errorlogpath']
 
+    if errorlogpath == "":
+        return None
+
     if ec is None:
         ec = {}
 
-    name, version = ec.get('name', '%(name)s'), ec.get('version', '%(version)s')
+    name, version = ec.get('name'), ec.get('version')
     date = time.strftime("%Y%m%d")
     timestamp = time.strftime("%H%M%S")
 
-    base_path = '/'.join([errorlogpath, name + '-' + version, date + '-' + timestamp])
-
-    path = base_path
-    inc_no = 1
-    while os.path.exists(path):
-        path = base_path + '_' + str(inc_no)
-        inc_no += 1
+    parent_dir = '/'.join([errorlogpath, name + '-' + version])
+    name = date + '-' + timestamp
 
-    return path
+    return (parent_dir, name)
 
 
 def get_build_log_path():

test/framework/toy_build.py

@@ -240,13 +240,13 @@ def run_test_toy_build_with_output(self, *args, **kwargs):
 
         self.mock_stderr(True)
         self.mock_stdout(True)
-        outtxt = self.test_toy_build(*args, **kwargs)
+        self.test_toy_build(*args, **kwargs)
         stderr = self.get_stderr()
         stdout = self.get_stdout()
         self.mock_stderr(False)
         self.mock_stdout(False)
 
-        return stdout, stderr, outtxt
+        return stdout, stderr
 
     def test_toy_broken(self):
         """Test deliberately broken toy build."""
@@ -330,10 +330,14 @@ def test_toy_broken_compilation(self):
             broken_compilation_ec = os.path.join(os.path.dirname(__file__),
                                                  'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0-buggy.eb')
 
-            _, _, outtxt = self.run_test_toy_build_with_output(
+            self.mock_stderr(True)
+            self.mock_stdout(True)
+            outtxt = self.test_toy_build(
                 ec_file=broken_compilation_ec, tmpdir=tmpdir, tmp_logdir=tmp_logdir,
                 verify=False, fails=True, verbose=False, raise_error=False,
                 name='toy', versionsuffix='-buggy')
+            self.mock_stderr(False)
+            self.mock_stdout(False)
 
             output_regexs = [r"^\s+toy\.c:5:44: error: expected (;|';') before"]