Merge pull request easybuilders#4390 from lexming/openfoam-workdir

check presence of CWD at the end of `run_shell_cmd` and try to return to original working directory if non-existent

easybuild/tools/run.py

@@ -361,11 +361,14 @@ def to_cmd_str(cmd):
     if qa_wait_patterns is None:
         qa_wait_patterns = []
 
+    # keep path to current working dir in case we need to come back to it
+    try:
+        initial_work_dir = os.getcwd()
+    except FileNotFoundError:
+        raise EasyBuildError(CWD_NOTFOUND_ERROR)
+
     if work_dir is None:
-        try:
-            work_dir = os.getcwd()
-        except FileNotFoundError:
-            raise EasyBuildError(CWD_NOTFOUND_ERROR)
+        work_dir = initial_work_dir
 
     if with_hooks:
         hooks = load_hooks(build_option('hooks'))
@@ -558,6 +561,20 @@ def to_cmd_str(cmd):
         if fail_on_error:
             raise_run_shell_cmd_error(res)
 
+    # check that we still are in a sane environment after command execution
+    # safeguard against commands that deleted the work dir or missbehaving filesystems
+    try:
+        os.getcwd()
+    except FileNotFoundError:
+        _log.warning(
+            f"Shell command `{cmd_str}` completed successfully but left the system in an unknown working directory. "
+            f"Changing back to initial working directory: {initial_work_dir}"
+        )
+        try:
+            os.chdir(initial_work_dir)
+        except OSError as err:
+            raise EasyBuildError(f"Failed to return to {initial_work_dir} after executing command `{cmd_str}`: {err}")
+
     if with_hooks:
         run_hook_kwargs = {
             'exit_code': res.exit_code,

test/framework/run.py

@@ -24,7 +24,7 @@
 # along with EasyBuild.  If not, see <http://www.gnu.org/licenses/>.
 # #
 """
-Unit tests for filetools.py
+Unit tests for run.py
 
 @author: Toon Willems (Ghent University)
 @author: Kenneth Hoste (Ghent University)
@@ -44,7 +44,7 @@
 import time
 from concurrent.futures import ThreadPoolExecutor
 from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, init_config
-from unittest import TextTestRunner
+from unittest import TextTestRunner, mock
 from easybuild.base.fancylogger import setLogLevelDebug
 
 import easybuild.tools.asyncprocess as asyncprocess
@@ -580,22 +580,38 @@ def test_run_shell_cmd_work_dir(self):
         """
         Test running shell command in specific directory with run_shell_cmd function.
         """
-        orig_wd = os.getcwd()
-        self.assertFalse(os.path.samefile(orig_wd, self.test_prefix))
-
         test_dir = os.path.join(self.test_prefix, 'test')
+        test_workdir = os.path.join(self.test_prefix, 'test', 'workdir')
         for fn in ('foo.txt', 'bar.txt'):
-            write_file(os.path.join(test_dir, fn), 'test')
+            write_file(os.path.join(test_workdir, fn), 'test')
+
+        os.chdir(test_dir)
+        orig_wd = os.getcwd()
+        self.assertFalse(os.path.samefile(orig_wd, self.test_prefix))
 
         cmd = "ls | sort"
+
+        # working directory is not explicitly defined
         with self.mocked_stdout_stderr():
-            res = run_shell_cmd(cmd, work_dir=test_dir)
+            res = run_shell_cmd(cmd)
+
+        self.assertEqual(res.cmd, cmd)
+        self.assertEqual(res.exit_code, 0)
+        self.assertEqual(res.output, 'workdir\n')
+        self.assertEqual(res.stderr, None)
+        self.assertEqual(res.work_dir, orig_wd)
+
+        self.assertTrue(os.path.samefile(orig_wd, os.getcwd()))
+
+        # working directory is explicitly defined
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(cmd, work_dir=test_workdir)
 
         self.assertEqual(res.cmd, cmd)
         self.assertEqual(res.exit_code, 0)
         self.assertEqual(res.output, 'bar.txt\nfoo.txt\n')
         self.assertEqual(res.stderr, None)
-        self.assertEqual(res.work_dir, test_dir)
+        self.assertEqual(res.work_dir, test_workdir)
 
         self.assertTrue(os.path.samefile(orig_wd, os.getcwd()))
 
@@ -1928,7 +1944,7 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs):
             stdout = self.get_stdout()
 
         expected_stdout = '\n'.join([
-            "pre-run hook 'make' in %s" % cwd,
+            f"pre-run hook 'make' in {cwd}",
             "post-run hook 'echo make' (exit code: 0, output: 'make\n')",
             '',
         ])
@@ -1960,6 +1976,74 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs):
         regex = re.compile('>> running shell command:\n\techo make', re.M)
         self.assertTrue(regex.search(stdout), "Pattern '%s' found in: %s" % (regex.pattern, stdout))
 
+    def test_run_shell_cmd_delete_cwd(self):
+        """
+        Test commands that destroy directories inside initial working directory
+        """
+        workdir = os.path.join(self.test_prefix, 'workdir')
+        sub_workdir = os.path.join(workdir, 'subworkdir')
+
+        # 1. test destruction of CWD which is a subdirectory inside original working directory
+        cmd_subworkdir_rm = (
+            "echo 'Command that jumps to subdir and removes it' && "
+            f"cd {sub_workdir} && pwd && rm -rf {sub_workdir} && "
+            "echo 'Working sub-directory removed.'"
+        )
+
+        # 1.a. in a robust system
+        expected_output = (
+            "Command that jumps to subdir and removes it\n"
+            f"{sub_workdir}\n"
+            "Working sub-directory removed.\n"
+        )
+
+        mkdir(sub_workdir, parents=True)
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(cmd_subworkdir_rm, work_dir=workdir)
+
+        self.assertEqual(res.cmd, cmd_subworkdir_rm)
+        self.assertEqual(res.exit_code, 0)
+        self.assertEqual(res.output, expected_output)
+        self.assertEqual(res.stderr, None)
+        self.assertEqual(res.work_dir, workdir)
+
+        # 1.b. in a flaky system that ends up in an unknown CWD after execution
+        mkdir(sub_workdir, parents=True)
+        fd, logfile = tempfile.mkstemp(suffix='.log', prefix='eb-test-')
+        os.close(fd)
+
+        with self.mocked_stdout_stderr():
+            with mock.patch('os.getcwd') as mock_getcwd:
+                mock_getcwd.side_effect = [
+                    workdir,
+                    FileNotFoundError(),
+                ]
+                init_logging(logfile, silent=True)
+                res = run_shell_cmd(cmd_subworkdir_rm, work_dir=workdir)
+                stop_logging(logfile)
+
+        self.assertEqual(res.cmd, cmd_subworkdir_rm)
+        self.assertEqual(res.exit_code, 0)
+        self.assertEqual(res.output, expected_output)
+        self.assertEqual(res.stderr, None)
+        self.assertEqual(res.work_dir, workdir)
+
+        expected_warning = f"Changing back to initial working directory: {workdir}\n"
+        logtxt = read_file(logfile)
+        self.assertTrue(logtxt.endswith(expected_warning))
+
+        # 2. test destruction of CWD which is main working directory passed to run_shell_cmd
+        cmd_workdir_rm = (
+            "echo 'Command that removes working directory' && pwd && "
+            f"rm -rf {workdir} && echo 'Working directory removed.'"
+        )
+
+        error_pattern = rf"Failed to return to {workdir} after executing command"
+
+        mkdir(workdir, parents=True)
+        with self.mocked_stdout_stderr():
+            self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir)
+
 
 def suite():
     """ returns all the testcases in this module """