From 9c8aa9bffe755fe6126dc72dfd037c6b20e65906 Mon Sep 17 00:00:00 2001 From: Martin Panter Date: Sun, 21 Aug 2016 04:07:58 +0000 Subject: Issue #27487: Warn if submodule already imported before runpy execution Also try to clarify the find_spec() error message. --- Lib/runpy.py | 11 ++++++++++- Lib/test/test_cmd_line_script.py | 8 +++++--- Lib/test/test_runpy.py | 40 +++++++++++++++++++++++++++++++++------- Misc/NEWS | 4 ++++ 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/Lib/runpy.py b/Lib/runpy.py index af6205d..e290c21 100644 --- a/Lib/runpy.py +++ b/Lib/runpy.py @@ -114,6 +114,15 @@ def _get_module_details(mod_name, error=ImportError): if e.name is None or (e.name != pkg_name and not pkg_name.startswith(e.name + ".")): raise + # Warn if the module has already been imported under its normal name + existing = sys.modules.get(mod_name) + if existing is not None and not hasattr(existing, "__path__"): + from warnings import warn + msg = "{mod_name!r} found in sys.modules after import of " \ + "package {pkg_name!r}, but prior to execution of " \ + "{mod_name!r}; this may result in unpredictable " \ + "behaviour".format(mod_name=mod_name, pkg_name=pkg_name) + warn(RuntimeWarning(msg)) try: spec = importlib.util.find_spec(mod_name) @@ -121,7 +130,7 @@ def _get_module_details(mod_name, error=ImportError): # This hack fixes an impedance mismatch between pkgutil and # importlib, where the latter raises other errors for cases where # pkgutil previously raised ImportError - msg = "Error while finding spec for {!r} ({}: {})" + msg = "Error while finding module specification for {!r} ({}: {})" raise error(msg.format(mod_name, type(ex).__name__, ex)) from ex if spec is None: raise error("No module named %s" % mod_name) diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py index b2be9b1..befe0e4 100644 --- a/Lib/test/test_cmd_line_script.py +++ b/Lib/test/test_cmd_line_script.py @@ -426,8 +426,9 @@ class CmdLineTest(unittest.TestCase): # Exercise error reporting for various invalid package executions tests = ( ('builtins', br'No code object available'), - ('builtins.x', br'Error while finding spec.*AttributeError'), - ('builtins.x.y', br'Error while finding spec.*' + ('builtins.x', br'Error while finding module specification.*' + br'AttributeError'), + ('builtins.x.y', br'Error while finding module specification.*' br'ImportError.*No module named.*not a package'), ('os.path', br'loader.*cannot handle'), ('importlib', br'No module named.*' @@ -450,7 +451,8 @@ class CmdLineTest(unittest.TestCase): with open('test_pkg/__init__.pyc', 'wb'): pass err = self.check_dash_m_failure('test_pkg') - self.assertRegex(err, br'Error while finding spec.*' + self.assertRegex(err, + br'Error while finding module specification.*' br'ImportError.*bad magic number') self.assertNotIn(b'is a package', err) self.assertNotIn(b'Traceback', err) diff --git a/Lib/test/test_runpy.py b/Lib/test/test_runpy.py index 87c83ec..db55db7 100644 --- a/Lib/test/test_runpy.py +++ b/Lib/test/test_runpy.py @@ -7,6 +7,7 @@ import re import tempfile import importlib, importlib.machinery, importlib.util import py_compile +import warnings from test.support import ( forget, make_legacy_pyc, unload, verbose, no_tracing, create_empty_file, temp_dir) @@ -246,7 +247,7 @@ class RunModuleTestCase(unittest.TestCase, CodeExecutionMixin): mod_fname) return pkg_dir, mod_fname, mod_name, mod_spec - def _del_pkg(self, top, depth, mod_name): + def _del_pkg(self, top): for entry in list(sys.modules): if entry.startswith("__runpy_pkg__"): del sys.modules[entry] @@ -320,7 +321,7 @@ class RunModuleTestCase(unittest.TestCase, CodeExecutionMixin): self._fix_ns_for_legacy_pyc(expected_ns, alter_sys) self.check_code_execution(create_ns, expected_ns) finally: - self._del_pkg(pkg_dir, depth, mod_name) + self._del_pkg(pkg_dir) if verbose > 1: print("Module executed successfully") def _check_package(self, depth, alter_sys=False, @@ -361,7 +362,7 @@ class RunModuleTestCase(unittest.TestCase, CodeExecutionMixin): self._fix_ns_for_legacy_pyc(expected_ns, alter_sys) self.check_code_execution(create_ns, expected_ns) finally: - self._del_pkg(pkg_dir, depth, pkg_name) + self._del_pkg(pkg_dir) if verbose > 1: print("Package executed successfully") def _add_relative_modules(self, base_dir, source, depth): @@ -424,7 +425,7 @@ from ..uncle.cousin import nephew self.assertIn("nephew", d2) del d2 # Ensure __loader__ entry doesn't keep file open finally: - self._del_pkg(pkg_dir, depth, mod_name) + self._del_pkg(pkg_dir) if verbose > 1: print("Module executed successfully") def test_run_module(self): @@ -447,7 +448,7 @@ from ..uncle.cousin import nephew result = self._make_pkg("", 1, "__main__") pkg_dir, _, mod_name, _ = result mod_name = mod_name.replace(".__main__", "") - self.addCleanup(self._del_pkg, pkg_dir, 1, mod_name) + self.addCleanup(self._del_pkg, pkg_dir) init = os.path.join(pkg_dir, "__runpy_pkg__", "__init__.py") exceptions = (ImportError, AttributeError, TypeError, ValueError) @@ -470,6 +471,31 @@ from ..uncle.cousin import nephew else: self.fail("Nothing raised; expected {}".format(name)) + def test_submodule_imported_warning(self): + pkg_dir, _, mod_name, _ = self._make_pkg("", 1) + try: + __import__(mod_name) + with self.assertWarnsRegex(RuntimeWarning, + r"found in sys\.modules"): + run_module(mod_name) + finally: + self._del_pkg(pkg_dir) + + def test_package_imported_no_warning(self): + pkg_dir, _, mod_name, _ = self._make_pkg("", 1, "__main__") + self.addCleanup(self._del_pkg, pkg_dir) + package = mod_name.replace(".__main__", "") + # No warning should occur if we only imported the parent package + __import__(package) + self.assertIn(package, sys.modules) + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + run_module(package) + # But the warning should occur if we imported the __main__ submodule + __import__(mod_name) + with self.assertWarnsRegex(RuntimeWarning, r"found in sys\.modules"): + run_module(package) + def test_run_package_in_namespace_package(self): for depth in range(1, 4): if verbose > 1: print("Testing package depth:", depth) @@ -524,7 +550,7 @@ from ..uncle.cousin import nephew try: self.check_code_execution(create_ns, expected_ns) finally: - self._del_pkg(pkg_dir, depth, mod_name) + self._del_pkg(pkg_dir) def test_pkgutil_walk_packages(self): # This is a dodgy hack to use the test_runpy infrastructure to test @@ -548,7 +574,7 @@ from ..uncle.cousin import nephew expected_modules.add(pkg_name + ".runpy_test") pkg_dir, mod_fname, mod_name, mod_spec = ( self._make_pkg("", max_depth)) - self.addCleanup(self._del_pkg, pkg_dir, max_depth, mod_name) + self.addCleanup(self._del_pkg, pkg_dir) for depth in range(2, max_depth+1): self._add_relative_modules(pkg_dir, "", depth) for finder, mod_name, ispkg in pkgutil.walk_packages([pkg_dir]): diff --git a/Misc/NEWS b/Misc/NEWS index 3d2cc19..1ecb6f1 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,10 @@ Release date: TBA Core and Builtins ----------------- +- Issue #27487: Warn if a submodule argument to "python -m" or + runpy.run_module() is found in sys.modules after parent packages are + imported, but before the submodule is executed. + - Issue #27558: Fix a SystemError in the implementation of "raise" statement. In a brand new thread, raise a RuntimeError since there is no active exception to reraise. Patch written by Xiang Zhang. -- cgit v0.12