From 93f3a211e76d6ad6b203acc831354cee159466ab Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 13 Dec 2019 08:35:08 -0700 Subject: [PATCH 1/7] Ensure sys.path cleanup in the event of an import exception --- pyutilib/misc/import_file.py | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/pyutilib/misc/import_file.py b/pyutilib/misc/import_file.py index 90db6c0a..eebedfdf 100644 --- a/pyutilib/misc/import_file.py +++ b/pyutilib/misc/import_file.py @@ -92,11 +92,11 @@ def import_file(filename, context=None, name=None, clear_cache=False): else: if clear_cache and modulename in sys.modules: del sys.modules[modulename] - if dirname is not None: - sys.path.insert(0, dirname) - else: - sys.path.insert(0, implied_dirname) try: + if dirname is not None: + sys.path.insert(0, dirname) + else: + sys.path.insert(0, implied_dirname) module = __import__(modulename) except ImportError: pass @@ -129,8 +129,8 @@ def import_file(filename, context=None, name=None, clear_cache=False): [dirname]) fp.close() else: - sys.path.insert(0, implied_dirname) try: + sys.path.insert(0, implied_dirname) # find_module will return the .py file # (never .pyc) fp, pathname, description = imp.find_module(modulename) @@ -202,29 +202,18 @@ def run_file(filename, logfile=None, execdir=None): # Run the module # try: - if not execdir is None: + tmp_path = list(sys.path) + if execdir is not None: tmp = os.getcwd() os.chdir(execdir) - tmp_path = sys.path sys.path = [execdir] + sys.path runpy.run_module(name, None, "__main__") - if not execdir is None: + finally: + # Mandatory cleanup + sys.path = tmp_path + if execdir is not None: os.chdir(tmp) - sys.path = tmp_path - except Exception: #pragma:nocover - if not logfile is None: + if logfile is not None: OUTPUT.close() sys.stdout = save_stdout sys.stderr = save_stderr - raise - if currdir_ in sys.path: - sys.path.remove(currdir_) - if execdir in sys.path: - sys.path.remove(execdir) - # - # Close logfile - # - if not logfile is None: - OUTPUT.close() - sys.stdout = save_stdout - sys.stderr = save_stderr From 18b7062ce7f4c9ef42a4438766dff29834da872e Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 13 Dec 2019 08:35:53 -0700 Subject: [PATCH 2/7] PEP8 style update --- pyutilib/misc/import_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyutilib/misc/import_file.py b/pyutilib/misc/import_file.py index eebedfdf..4a5decc8 100644 --- a/pyutilib/misc/import_file.py +++ b/pyutilib/misc/import_file.py @@ -175,7 +175,7 @@ def run_file(filename, logfile=None, execdir=None): # # Open logfile # - if not logfile is None: + if logfile is not None: sys.stderr.flush() sys.stdout.flush() save_stdout = sys.stdout From 78718b2bf37617301dacae7d9d94407df4230696 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 13 Dec 2019 08:36:02 -0700 Subject: [PATCH 3/7] Improve robustness managing the import file directory --- pyutilib/misc/import_file.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/pyutilib/misc/import_file.py b/pyutilib/misc/import_file.py index 4a5decc8..a52bf1b2 100644 --- a/pyutilib/misc/import_file.py +++ b/pyutilib/misc/import_file.py @@ -187,16 +187,20 @@ def run_file(filename, logfile=None, execdir=None): # Add the file directory to the system path # currdir_ = '' - if '/' in filename: - currdir_ = "/".join((filename).split("/")[:-1]) - tmp_import = (filename).split("/")[-1] - sys.path.append(currdir_) - elif '\\' in filename: - currdir_ = "\\".join((filename).split("\\")[:-1]) - tmp_import = (filename).split("\\")[-1] - sys.path.append(currdir_) - else: - tmp_import = filename + norm_file = os.path.normpath(filename) + assert norm_file[-1] not in '\\/' + split_path = [] + while norm_file: + norm_file, tail = os.path.split(norm_file) + split_path.append(tail) + # Absolute paths can get stuck returning ('/', '') + if not tail: + split_path.append(norm_file) + norm_file = '' + split_path.reverse() + currdir_ = os.path.join(split_path[:-1]) + tmp_import = split_path[-1] + name = ".".join((tmp_import).split(".")[:-1]) # # Run the module @@ -207,6 +211,10 @@ def run_file(filename, logfile=None, execdir=None): tmp = os.getcwd() os.chdir(execdir) sys.path = [execdir] + sys.path + # [JDS 191130] I am not sure why we put the target file's + # directory at the end of sys.path, but I am preserving that + # decision. + sys.path.append(currdir_) runpy.run_module(name, None, "__main__") finally: # Mandatory cleanup From e86c31e9ad14b4c84a860bfe56a2f65456c5c416 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 13 Dec 2019 08:45:03 -0700 Subject: [PATCH 4/7] Fixing argument to join() --- pyutilib/misc/import_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyutilib/misc/import_file.py b/pyutilib/misc/import_file.py index a52bf1b2..54614e92 100644 --- a/pyutilib/misc/import_file.py +++ b/pyutilib/misc/import_file.py @@ -198,7 +198,7 @@ def run_file(filename, logfile=None, execdir=None): split_path.append(norm_file) norm_file = '' split_path.reverse() - currdir_ = os.path.join(split_path[:-1]) + currdir_ = os.path.join(*tuple(split_path[:-1])) tmp_import = split_path[-1] name = ".".join((tmp_import).split(".")[:-1]) From 7a746604092891a4fbfbb50063959a60876ba26e Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 13 Dec 2019 09:09:22 -0700 Subject: [PATCH 5/7] Adding guard for local paths --- pyutilib/misc/import_file.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyutilib/misc/import_file.py b/pyutilib/misc/import_file.py index 54614e92..ab9ea393 100644 --- a/pyutilib/misc/import_file.py +++ b/pyutilib/misc/import_file.py @@ -198,7 +198,10 @@ def run_file(filename, logfile=None, execdir=None): split_path.append(norm_file) norm_file = '' split_path.reverse() - currdir_ = os.path.join(*tuple(split_path[:-1])) + if len(split_path) > 1: + currdir_ = os.path.join(*tuple(split_path[:-1])) + else: + currdir_ = '.' tmp_import = split_path[-1] name = ".".join((tmp_import).split(".")[:-1]) From 859df866e2368c3d47e7794f2d40514fce7ee492 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 13 Dec 2019 09:10:26 -0700 Subject: [PATCH 6/7] Adding import_file tests --- pyutilib/misc/tests/import_exception.py | 3 +++ pyutilib/misc/tests/import_main_exception.py | 5 +++++ pyutilib/misc/tests/import_main_exception.txt | 1 + pyutilib/misc/tests/test_import.py | 17 +++++++++++++++++ 4 files changed, 26 insertions(+) create mode 100644 pyutilib/misc/tests/import_exception.py create mode 100644 pyutilib/misc/tests/import_main_exception.py create mode 100644 pyutilib/misc/tests/import_main_exception.txt diff --git a/pyutilib/misc/tests/import_exception.py b/pyutilib/misc/tests/import_exception.py new file mode 100644 index 00000000..5480bd95 --- /dev/null +++ b/pyutilib/misc/tests/import_exception.py @@ -0,0 +1,3 @@ +import sys + +raise RuntimeError("raised during import") diff --git a/pyutilib/misc/tests/import_main_exception.py b/pyutilib/misc/tests/import_main_exception.py new file mode 100644 index 00000000..a3ef7612 --- /dev/null +++ b/pyutilib/misc/tests/import_main_exception.py @@ -0,0 +1,5 @@ +import sys + +if __name__ == "__main__": + print("import_main_exception - main") + raise RuntimeError("raised from __main__") diff --git a/pyutilib/misc/tests/import_main_exception.txt b/pyutilib/misc/tests/import_main_exception.txt new file mode 100644 index 00000000..588c98e3 --- /dev/null +++ b/pyutilib/misc/tests/import_main_exception.txt @@ -0,0 +1 @@ +import_main_exception - main diff --git a/pyutilib/misc/tests/test_import.py b/pyutilib/misc/tests/test_import.py index a5a57b07..dbd50bf9 100644 --- a/pyutilib/misc/tests/test_import.py +++ b/pyutilib/misc/tests/test_import.py @@ -54,6 +54,18 @@ def test_run_file3(self): currdir + "import2.txt")[0]) os.remove(currdir + "import2.log") + def test_run_file_exception(self): + with self.assertRaisesRegexp(RuntimeError, "raised from __main__"): + pyutilib.misc.run_file( + "import_main_exception.py", + logfile=currdir + "import_main_exception.log", execdir=currdir) + + self.assertFalse( + pyutilib.misc.comparison.compare_file( + currdir + "import_main_exception.log", + currdir + "import_main_exception.txt")[0]) + os.remove(currdir + "import_main_exception.log") + class TestImportFile(unittest.TestCase): @@ -83,6 +95,11 @@ def test_import_file_context3(self): if not "import1" in globals(): self.fail("test_import_file - failed to import the import1.py file") + def test_run_file_exception(self): + with self.assertRaisesRegexp(RuntimeError, "raised during import"): + pyutilib.misc.run_file( + "import_exception.py", execdir=currdir) + def test1(self): try: pyutilib.misc.import_file('tfile.py') From d04eace1dafbfeaa5552a8782bdc6b8af285e1f8 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 13 Dec 2019 09:14:54 -0700 Subject: [PATCH 7/7] Updating tests to confirm sys.path is preserved --- pyutilib/misc/tests/test_import.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pyutilib/misc/tests/test_import.py b/pyutilib/misc/tests/test_import.py index dbd50bf9..af523ad2 100644 --- a/pyutilib/misc/tests/test_import.py +++ b/pyutilib/misc/tests/test_import.py @@ -55,6 +55,7 @@ def test_run_file3(self): os.remove(currdir + "import2.log") def test_run_file_exception(self): + orig_path = list(sys.path) with self.assertRaisesRegexp(RuntimeError, "raised from __main__"): pyutilib.misc.run_file( "import_main_exception.py", @@ -65,6 +66,8 @@ def test_run_file_exception(self): currdir + "import_main_exception.log", currdir + "import_main_exception.txt")[0]) os.remove(currdir + "import_main_exception.log") + self.assertIsNot(orig_path, sys.path) + self.assertEqual(orig_path, sys.path) class TestImportFile(unittest.TestCase): @@ -95,10 +98,13 @@ def test_import_file_context3(self): if not "import1" in globals(): self.fail("test_import_file - failed to import the import1.py file") - def test_run_file_exception(self): + def test_import_exception(self): + orig_path = list(sys.path) with self.assertRaisesRegexp(RuntimeError, "raised during import"): pyutilib.misc.run_file( "import_exception.py", execdir=currdir) + self.assertIsNot(orig_path, sys.path) + self.assertEqual(orig_path, sys.path) def test1(self): try: