Enforce persistence copying restrictions during input option validation

Flags for persisting logs and artifacts are now checked during the
validation of input options. In case of invalid settings the program
terminates early with an error.

Integration tests for internal logging functions are deactivated as the
logging functions are never reached in normal operation.

easybuild/tools/options.py

@@ -86,7 +86,7 @@
 from easybuild.tools.docs import list_easyblocks, list_toolchains
 from easybuild.tools.environment import restore_env, unset_env_vars
 from easybuild.tools.filetools import CHECKSUM_TYPE_SHA256, CHECKSUM_TYPES, expand_glob_paths, get_cwd
-from easybuild.tools.filetools import install_fake_vsc, move_file, which
+from easybuild.tools.filetools import install_fake_vsc, move_file, which, is_parent_path
 from easybuild.tools.github import GITHUB_PR_DIRECTION_DESC, GITHUB_PR_ORDER_CREATED
 from easybuild.tools.github import GITHUB_PR_STATE_OPEN, GITHUB_PR_STATES, GITHUB_PR_ORDERS, GITHUB_PR_DIRECTIONS
 from easybuild.tools.github import HAVE_GITHUB_API, HAVE_KEYRING, VALID_CLOSE_PR_REASONS
@@ -1294,6 +1294,21 @@ def _postprocess_config(self):
         if self.options.inject_checksums or self.options.inject_checksums_to_json:
             self.options.pre_create_installdir = False
 
+        # Prevent permanent storage of logs and artifacts from modifying the build directory
+        if self.options.buildpath and self.options.log_error_path:
+            if is_parent_path(self.options.buildpath, self.options.log_error_path):
+                raise EasyBuildError(
+                    "The --log-error-path ('%s') cannot reside on a subdirectory of the --buildpath ('%s')"
+                    % (self.options.log_error_path, self.options.buildpath)
+                )
+
+        if self.options.buildpath and self.options.artifact_error_path:
+            if is_parent_path(self.options.buildpath, self.options.artifact_error_path):
+                raise EasyBuildError(
+                    "The --artifact-error-path ('%s') cannot reside on a subdirectory of the --buildpath ('%s')"
+                    % (self.options.log_error_path, self.options.buildpath)
+                )
+
     def _postprocess_list_avail(self):
         """Create all the additional info that can be requested (exit at the end)"""
         msg = ''

test/framework/options.py

@@ -1630,6 +1630,52 @@ def test_dry_run(self):
         regex = re.compile(r" \* \[%s\] \S+%s \(module: %s\)" % (mark, ec, mod), re.M)
         self.assertTrue(regex.search(logtxt), "Found match for pattern %s in '%s'" % (regex.pattern, logtxt))
 
+    def test_persistence_copying_restrictions(self):
+        """
+        Test that EasyBuild fails when instructed to move logs or artifacts inside the build directory
+
+        Moving log files or artifacts inside the build directory modifies the build artifacts, and in the case of
+        build artifacts it is also copying directories into themselves.
+        """
+        base_args = [
+            'gzip-1.4-GCC-4.6.3.eb',
+            '--dry-run',
+            '--robot',
+        ]
+
+        def test_eb_with(option_flag, is_valid):
+            with tempfile.TemporaryDirectory() as root_dir:
+                build_dir = os.path.join(root_dir, 'build_dir')
+                if is_valid:
+                    persist_path = os.path.join(root_dir, 'persist_dir')
+                else:
+                    persist_path = os.path.join(root_dir, 'build_dir', 'persist_dir')
+
+                extra_args = [
+                    f"--buildpath={build_dir}",
+                    f"{option_flag}={persist_path}",
+                ]
+
+                pattern = f"The {option_flag} (.*) cannot reside on a subdirectory of the --buildpath (.*)"
+
+                args = base_args
+                args.extend(extra_args)
+
+                if is_valid:
+                    try:
+                        self.eb_main(args, raise_error=True)
+                    except EasyBuildError:
+                        self.fail(
+                            "Should not fail with --buildpath='{build_dir}' and {option_flag}='{persist_path}'."
+                        )
+                else:
+                    self.assertErrorRegex(EasyBuildError, pattern, self.eb_main, args, raise_error=True)
+
+        test_eb_with(option_flag='--log-error-path', is_valid=True)
+        test_eb_with(option_flag='--log-error-path', is_valid=False)
+        test_eb_with(option_flag='--artifact-error-path', is_valid=True)
+        test_eb_with(option_flag='--artifact-error-path', is_valid=False)
+
     def test_missing(self):
         """Test use of --missing/-M."""
 

test/framework/toy_build.py

@@ -378,129 +378,6 @@ def test_toy_broken_compilation(self):
             self.check_errorlog(output_regexs, outtxt, tmp_log_dir,
                                 self.test_buildpath, tmp_log_error_dir, tmp_artifact_error_dir)
 
-    def assert_persistence_not_overwriting_build_directory(self, outtxt, patterns_presence):
-        for pattern_presence in patterns_presence:
-            regex_pattern, present = pattern_presence
-            regex = re.compile(regex_pattern, re.M)
-            msg_stdout = "Pattern '%s' found in full test report:\n%s" % (regex.pattern, outtxt)
-            if present:
-                self.assertTrue(regex.search(outtxt), msg_stdout)
-            else:
-                self.assertFalse(regex.search(outtxt), msg_stdout)
-
-    def test_toy_persistence_copying_restrictions(self):
-        """
-        Test that EasyBuild refuses to move logs and artifacts inside the build directory in case of failed
-        compilations.
-
-        Moving log files or artifacts inside the build directory modifies the build artifacts, and in the case of
-        build is also copying directories into themselves.
-        """
-        with tempfile.TemporaryDirectory() as base_tmp_dir:
-            base_tmp_path = pathlib.Path(base_tmp_dir)
-
-            tmpdir = base_tmp_path / 'tmp'
-            tmp_log_dir = base_tmp_path / 'log_dir'
-            tmp_easyconfig_dir = base_tmp_path / 'easyconfig_dir'
-
-            base_ec = os.path.join(
-                os.path.dirname(__file__),
-                'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb')
-
-            base_ec_txt = read_file(base_ec)
-            broken_compilation_ec_txt = re.sub(
-                r'toy-0\.0_fix-silly-typo-in-printf-statement\.patch',
-                r'toy-0.0_add-bug.patch',
-                base_ec_txt)
-            broken_compilation_ec = os.path.join(tmp_easyconfig_dir, 'toy-0.0-buggy.eb')
-            write_file(broken_compilation_ec, broken_compilation_ec_txt)
-
-            def check_overwriting_of_build_directory(tmp_log_error_dir,
-                                                     tmp_artifact_error_dir,
-                                                     copied_log_error_dir,
-                                                     copied_artifact_error_dir):
-                self.mock_stderr(True)
-                self.mock_stdout(True)
-
-                outtxt = self.test_toy_build(
-                    ec_file=broken_compilation_ec, tmpdir=tmpdir,
-                    tmp_logdir=tmp_log_dir, log_error_dir=tmp_log_error_dir, artifact_error_dir=tmp_artifact_error_dir,
-                    verify=False, fails=True, verbose=False, raise_error=False,
-                    name='toy', versionsuffix='-buggy')
-
-                self.assert_persistence_not_overwriting_build_directory(
-                    outtxt=outtxt,
-                    patterns_presence=[
-                        (
-                            "Logs of failed build copied to permanent storage",
-                            copied_log_error_dir
-                        ),
-                        (
-                            "Artifacts of failed build copied to permanent storage",
-                            copied_artifact_error_dir
-                        ),
-                        (
-                            "Persistent log directory is subdirectory of build directory; not copying logs.",
-                            not copied_log_error_dir
-                        ),
-                        (
-                            "Persistent artifact directory is subdirectory of build directory; not copying artifacts.",
-                            not copied_artifact_error_dir
-                        ),
-                    ]
-                )
-
-                shutil.rmtree(tmpdir)
-                shutil.rmtree(tmp_log_dir)
-                shutil.rmtree(self.test_buildpath)
-                if copied_log_error_dir:
-                    shutil.rmtree(tmp_log_error_dir)
-                if copied_artifact_error_dir:
-                    shutil.rmtree(tmp_artifact_error_dir)
-
-                self.mock_stderr(False)
-                self.mock_stdout(False)
-
-            old_buildpath = self.test_buildpath
-            self.test_buildpath = str(base_tmp_path / 'build_dir')
-
-            log_path_outside_build_directory = os.path.join(str(base_tmp_path), 'log_error_dir')
-            log_path_inside_build_directory = os.path.join(self.test_buildpath,
-                                                           'toy', '0.0', 'system-system', 'log_error_dir')
-            artifact_path_outside_build_directory = os.path.join(str(base_tmp_path), 'artifact_error_dir')
-            artifact_path_inside_build_directory = os.path.join(self.test_buildpath,
-                                                                'toy', '0.0', 'system-system', 'artifact_error_dir')
-
-            check_overwriting_of_build_directory(
-                tmp_log_error_dir=log_path_outside_build_directory,
-                tmp_artifact_error_dir=artifact_path_outside_build_directory,
-                copied_log_error_dir=True,
-                copied_artifact_error_dir=True
-            )
-
-            check_overwriting_of_build_directory(
-                tmp_log_error_dir=log_path_inside_build_directory,
-                tmp_artifact_error_dir=artifact_path_outside_build_directory,
-                copied_log_error_dir=False,
-                copied_artifact_error_dir=True
-            )
-
-            check_overwriting_of_build_directory(
-                tmp_log_error_dir=log_path_outside_build_directory,
-                tmp_artifact_error_dir=artifact_path_inside_build_directory,
-                copied_log_error_dir=True,
-                copied_artifact_error_dir=False
-            )
-
-            check_overwriting_of_build_directory(
-                tmp_log_error_dir=log_path_inside_build_directory,
-                tmp_artifact_error_dir=artifact_path_inside_build_directory,
-                copied_log_error_dir=False,
-                copied_artifact_error_dir=False
-            )
-
-            self.test_buildpath = old_buildpath
-
     def test_toy_tweaked(self):
         """Test toy build with tweaked easyconfig, for testing extra easyconfig parameters."""
         test_ecs_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'easyconfigs')