From 7c7dd45554ef8f2a2cb4a140d02343e01d1c2223 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 6 Feb 2019 20:46:16 -0800 Subject: [PATCH 01/43] Properly handle SyntaxErrors in Python source files. SyntaxErrors in the target module will rise normally, while SyntaxErrors in dependencies will be added to badmodules. This includes a new regression test. --- Lib/modulefinder.py | 3 +++ Lib/test/test_modulefinder.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 10320a74d94249..ab8d2aa28849b3 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -323,6 +323,9 @@ def _safe_import_hook(self, name, caller, fromlist, level=-1): except ImportError as msg: self.msg(2, "ImportError:", str(msg)) self._add_badmodule(name, caller) + except SyntaxError as msg: + self.msg(2, "SyntaxError:", str(msg)) + self._add_badmodule(name, caller) else: if fromlist: for sub in fromlist: diff --git a/Lib/test/test_modulefinder.py b/Lib/test/test_modulefinder.py index e4df2a90d4a4d0..73156bde21d14b 100644 --- a/Lib/test/test_modulefinder.py +++ b/Lib/test/test_modulefinder.py @@ -218,6 +218,19 @@ def foo(): pass "" ] +syntax_error_test = [ + "a.module", + ["a", "a.module", "b"], + ["b.module"], [], + """\ +a/__init__.py +a/module.py + import b.module +b/__init__.py +b/module.py + ? # SyntaxError: invalid syntax +"""] + def open_file(path): dirname = os.path.dirname(path) @@ -299,6 +312,9 @@ def test_relative_imports_3(self): def test_relative_imports_4(self): self._do_test(relative_import_test_4) + def test_syntax_error(self): + self._do_test(syntax_error_test) + def test_bytecode(self): base_path = os.path.join(TEST_DIR, 'a') source_path = base_path + importlib.machinery.SOURCE_SUFFIXES[0] From 8f98ca6df2987ff7a14611479917c486acf3279c Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 6 Feb 2019 21:13:04 -0800 Subject: [PATCH 02/43] Fix name collision bug. This fixes an issue where a "fromlist" import with the same name as a previously failed import would be incorrectly added to badmodules. This includes a new regression test. --- Lib/modulefinder.py | 6 +++--- Lib/test/test_modulefinder.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index ab8d2aa28849b3..39fc9f4a2f471f 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -329,14 +329,14 @@ def _safe_import_hook(self, name, caller, fromlist, level=-1): else: if fromlist: for sub in fromlist: - if sub in self.badmodules: - self._add_badmodule(sub, caller) + fullname = name + "." + sub + if fullname in self.badmodules: + self._add_badmodule(fullname, caller) continue try: self.import_hook(name, caller, [sub], level=level) except ImportError as msg: self.msg(2, "ImportError:", str(msg)) - fullname = name + "." + sub self._add_badmodule(fullname, caller) def scan_opcodes(self, co): diff --git a/Lib/test/test_modulefinder.py b/Lib/test/test_modulefinder.py index 73156bde21d14b..ebd96e1c8a2dd0 100644 --- a/Lib/test/test_modulefinder.py +++ b/Lib/test/test_modulefinder.py @@ -232,6 +232,20 @@ def foo(): pass """] +same_name_as_bad_test = [ + "a.module", + ["a", "a.module", "b", "b.c"], + ["c"], [], + """\ +a/__init__.py +a/module.py + import c + from b import c +b/__init__.py +b/c.py +"""] + + def open_file(path): dirname = os.path.dirname(path) try: @@ -315,6 +329,9 @@ def test_relative_imports_4(self): def test_syntax_error(self): self._do_test(syntax_error_test) + def test_same_name_as_bad(self): + self._do_test(same_name_as_bad_test) + def test_bytecode(self): base_path = os.path.join(TEST_DIR, 'a') source_path = base_path + importlib.machinery.SOURCE_SUFFIXES[0] From c00c4b6cd72c062300df9f2a4583b44b76399676 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 6 Feb 2019 20:49:47 -0800 Subject: [PATCH 03/43] Replace mutable default values. Bound empty lists have been replaced with the "if param is None" idiom. --- Lib/modulefinder.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 39fc9f4a2f471f..3d0ec442a5aa20 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -69,7 +69,7 @@ def __repr__(self): class ModuleFinder: - def __init__(self, path=None, debug=0, excludes=[], replace_paths=[]): + def __init__(self, path=None, debug=0, excludes=None, replace_paths=None): if path is None: path = sys.path self.path = path @@ -77,8 +77,8 @@ def __init__(self, path=None, debug=0, excludes=[], replace_paths=[]): self.badmodules = {} self.debug = debug self.indent = 0 - self.excludes = excludes - self.replace_paths = replace_paths + self.excludes = excludes if excludes is not None else [] + self.replace_paths = replace_paths if replace_paths is not None else [] self.processed_paths = [] # Used in debugging only def msg(self, level, str, *args): From 94e38d42cd391d731c9c6b8bc96b7f2f01c2ef1d Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 6 Feb 2019 20:43:12 -0800 Subject: [PATCH 04/43] Replace deprecated imp usage. Constants imported from imp have been moved to private module-level constants, and ModuleFinder.find_module has been refactored to use importlib. Other than an improvement on how frozen builtin imports are reported (as the frozen imports they are, rather than the stdlib modules they *may* have originated from), these changes maintain complete compatibility with past versions... including odd behavior for returning relative (below current directory, but not a C extension) vs. absolute (above current directory, or a C extension) paths. --- Lib/modulefinder.py | 85 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 75 insertions(+), 10 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 3d0ec442a5aa20..a46a07a567caf1 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -2,15 +2,14 @@ import dis import importlib._bootstrap_external +import importlib.util import importlib.machinery import marshal import os import sys import types import warnings -with warnings.catch_warnings(): - warnings.simplefilter('ignore', DeprecationWarning) - import imp + LOAD_CONST = dis.opmap['LOAD_CONST'] IMPORT_NAME = dis.opmap['IMPORT_NAME'] @@ -19,6 +18,16 @@ STORE_OPS = STORE_NAME, STORE_GLOBAL EXTENDED_ARG = dis.EXTENDED_ARG +# Old imp constants: + +_SEARCH_ERROR = 0 +_PY_SOURCE = 1 +_PY_COMPILED = 2 +_C_EXTENSION = 3 +_PKG_DIRECTORY = 5 +_C_BUILTIN = 6 +_PY_FROZEN = 7 + # Modulefinder does a good job at simulating Python's, but it can not # handle __path__ modifications packages make at runtime. Therefore there # is a mechanism whereby you can register extra paths in this map for a @@ -105,14 +114,14 @@ def msgout(self, *args): def run_script(self, pathname): self.msg(2, "run_script", pathname) with open(pathname) as fp: - stuff = ("", "r", imp.PY_SOURCE) + stuff = ("", "r", _PY_SOURCE) self.load_module('__main__', fp, pathname, stuff) def load_file(self, pathname): dir, name = os.path.split(pathname) name, ext = os.path.splitext(name) with open(pathname) as fp: - stuff = (ext, "r", imp.PY_SOURCE) + stuff = (ext, "r", _PY_SOURCE) self.load_module(name, fp, pathname, stuff) def import_hook(self, name, caller=None, fromlist=None, level=-1): @@ -279,13 +288,13 @@ def import_module(self, partname, fqname, parent): def load_module(self, fqname, fp, pathname, file_info): suffix, mode, type = file_info self.msgin(2, "load_module", fqname, fp and "fp", pathname) - if type == imp.PKG_DIRECTORY: + if type == _PKG_DIRECTORY: m = self.load_package(fqname, pathname) self.msgout(2, "load_module ->", m) return m - if type == imp.PY_SOURCE: + if type == _PY_SOURCE: co = compile(fp.read()+'\n', pathname, 'exec') - elif type == imp.PY_COMPILED: + elif type == _PY_COMPILED: try: data = fp.read() importlib._bootstrap_external._classify_pyc(data, fqname, {}) @@ -448,10 +457,66 @@ def find_module(self, name, path, parent=None): if path is None: if name in sys.builtin_module_names: - return (None, None, ("", "", imp.C_BUILTIN)) + return (None, None, ("", "", _C_BUILTIN)) path = self.path - return imp.find_module(name, path) + + old_path = sys.path + + try: + + sys.path = path + old_path + + try: + spec = importlib.util.find_spec(fullname) + except ImportError: + spec = importlib.util.find_spec(name) + + finally: + + sys.path = old_path + + if spec is None: + raise ImportError("No module named {name!r}".format(name=name), name=name) + + if isinstance(spec.loader, type): + + if issubclass(spec.loader, importlib.machinery.BuiltinImporter): + return None, None, ("", "", _C_BUILTIN) + + if issubclass(spec.loader, importlib.machinery.FrozenImporter): + return None, None, ("", "", _PY_FROZEN) + + file_path = spec.origin + + if os.path.commonpath(map(os.path.abspath, (file_path, os.getcwd()))) == os.getcwd(): + file_path = os.path.relpath(file_path) + + base, suffix = os.path.splitext(file_path) + + if suffix in importlib.machinery.EXTENSION_SUFFIXES: + file_path = os.path.abspath(file_path) + kind = _C_EXTENSION + mode = "rb" + + elif suffix in importlib.machinery.SOURCE_SUFFIXES: + + if name != "__init__" and os.path.basename(base) == "__init__": + return (None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY)) + + kind = _PY_SOURCE + mode = "r" + + elif suffix in importlib.machinery.BYTECODE_SUFFIXES: + kind = _PY_COMPILED + mode = "rb" + + else: + return None, None, ("", "", _SEARCH_ERROR) # Should never happen. + + file = open(file_path, mode) + + return file, file_path, (suffix, mode, kind) def report(self): """Print a report to stdout, listing the found modules with their From 9bda94b717767ec00b084b579a6b5d3fb3750e18 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 7 Feb 2019 19:34:14 -0800 Subject: [PATCH 05/43] Fixed whitespace. Whoops. --- Lib/modulefinder.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index a46a07a567caf1..dec03e7ee13737 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -483,7 +483,7 @@ def find_module(self, name, path, parent=None): if issubclass(spec.loader, importlib.machinery.BuiltinImporter): return None, None, ("", "", _C_BUILTIN) - + if issubclass(spec.loader, importlib.machinery.FrozenImporter): return None, None, ("", "", _PY_FROZEN) @@ -493,14 +493,14 @@ def find_module(self, name, path, parent=None): file_path = os.path.relpath(file_path) base, suffix = os.path.splitext(file_path) - + if suffix in importlib.machinery.EXTENSION_SUFFIXES: file_path = os.path.abspath(file_path) kind = _C_EXTENSION mode = "rb" elif suffix in importlib.machinery.SOURCE_SUFFIXES: - + if name != "__init__" and os.path.basename(base) == "__init__": return (None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY)) From c366485fc348671e7577553ca3ec2b68c559be8a Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 7 Feb 2019 20:13:38 -0800 Subject: [PATCH 06/43] Handle differing drives on Windows. Specifically, when checking whether the returned path should be relative or absolute. --- Lib/modulefinder.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index dec03e7ee13737..87ae230f6e24ca 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -489,8 +489,13 @@ def find_module(self, name, path, parent=None): file_path = spec.origin - if os.path.commonpath(map(os.path.abspath, (file_path, os.getcwd()))) == os.getcwd(): - file_path = os.path.relpath(file_path) + try: + + if os.path.commonpath(map(os.path.abspath, (file_path, os.getcwd()))) == os.getcwd(): + file_path = os.path.relpath(file_path) + + except ValueError: + pass base, suffix = os.path.splitext(file_path) From dc9180d35491b986252534ee91ae35e117463488 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 7 Feb 2019 20:14:23 -0800 Subject: [PATCH 07/43] Again, fixed whitespace. Whoops. --- Lib/modulefinder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 87ae230f6e24ca..82b2620be3507a 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -490,7 +490,7 @@ def find_module(self, name, path, parent=None): file_path = spec.origin try: - + if os.path.commonpath(map(os.path.abspath, (file_path, os.getcwd()))) == os.getcwd(): file_path = os.path.relpath(file_path) From faf2902d056058a214ef5e0c01892856970d2519 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Fri, 8 Feb 2019 04:46:59 +0000 Subject: [PATCH 08/43] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst b/Misc/NEWS.d/next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst new file mode 100644 index 00000000000000..c9df83b480c0ca --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst @@ -0,0 +1,2 @@ +:mod:`modulefinder` has been refactored to use the non-deprecated :mod:`importlib`. Additionally, several long-standing bugs have been fixed. +Patch by Brandt Bucher. \ No newline at end of file From a1db2ecc3c515a2a2e8fcaa85c58c823790eb1e8 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 7 Feb 2019 21:44:11 -0800 Subject: [PATCH 09/43] Fix relative imports. Hopefully, this will help the failing tests on Windows. --- Lib/modulefinder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 82b2620be3507a..93d8ed1840f128 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -468,7 +468,7 @@ def find_module(self, name, path, parent=None): sys.path = path + old_path try: - spec = importlib.util.find_spec(fullname) + spec = importlib.util.find_spec(fullname, parent) except ImportError: spec = importlib.util.find_spec(name) From bd73fa5f44c8745378dff1b2c31af36bd95e8fbe Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Feb 2019 20:15:20 -0800 Subject: [PATCH 10/43] Fix import side-effects. importlib.util.find_spec(fullname, parent), where fullname contains a dot, results in the parent module being imported. This fix ensures that children are correctly located while also avoiding import side-effects by temporarily clearing sys.modules and modifying sys.path. Now, neither a dotted name nor parent are required. --- Lib/modulefinder.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 93d8ed1840f128..8a87cc7db95c93 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -462,19 +462,19 @@ def find_module(self, name, path, parent=None): path = self.path old_path = sys.path + old_modules = sys.modules try: sys.path = path + old_path + sys.modules = {} - try: - spec = importlib.util.find_spec(fullname, parent) - except ImportError: - spec = importlib.util.find_spec(name) + spec = importlib.util.find_spec(name) finally: sys.path = old_path + sys.modules = old_modules if spec is None: raise ImportError("No module named {name!r}".format(name=name), name=name) From 210edadff9bbeb0c54e747f2ec15d940381b4e24 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Feb 2019 21:12:32 -0800 Subject: [PATCH 11/43] Wait for sys.path to update. I have a hunch the failing tests are due to a race condition in how sys.path updates. Sleep a bit here to see. --- Lib/modulefinder.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 8a87cc7db95c93..43d1bd812d8d78 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -7,6 +7,7 @@ import marshal import os import sys +import time import types import warnings @@ -469,6 +470,7 @@ def find_module(self, name, path, parent=None): sys.path = path + old_path sys.modules = {} + time.sleep(0.000001) spec = importlib.util.find_spec(name) finally: From 3a9dd35e48241e6b98502f7c8589cbdfd01d5a52 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Feb 2019 21:32:09 -0800 Subject: [PATCH 12/43] Bump sleep time. More test are passing, which supports my theory. --- Lib/modulefinder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 43d1bd812d8d78..21a63aeeed480c 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -470,7 +470,7 @@ def find_module(self, name, path, parent=None): sys.path = path + old_path sys.modules = {} - time.sleep(0.000001) + time.sleep(0.00001) spec = importlib.util.find_spec(name) finally: From 2763440a3e8a829ead794e2ac82654bdb269d1e6 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Feb 2019 22:16:18 -0800 Subject: [PATCH 13/43] WIP: Replace sleep with busy wait. *crosses fingers* --- Lib/modulefinder.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 21a63aeeed480c..29688c7b741cd4 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -7,7 +7,6 @@ import marshal import os import sys -import time import types import warnings @@ -470,7 +469,7 @@ def find_module(self, name, path, parent=None): sys.path = path + old_path sys.modules = {} - time.sleep(0.00001) + [i for i in range(10000)] # XXX: Busy wait. spec = importlib.util.find_spec(name) finally: From f9939bf4168a32f234981efb90371bfcbebb7ff3 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Feb 2019 22:39:24 -0800 Subject: [PATCH 14/43] Remove busy wait. I guess that wasn't the issue... --- Lib/modulefinder.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 29688c7b741cd4..8a87cc7db95c93 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -469,7 +469,6 @@ def find_module(self, name, path, parent=None): sys.path = path + old_path sys.modules = {} - [i for i in range(10000)] # XXX: Busy wait. spec = importlib.util.find_spec(name) finally: From ddb8752ce66d733b56e8aa09128fc7781a5159ff Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 9 Feb 2019 08:24:23 -0800 Subject: [PATCH 15/43] Remove NEWS entry. This will be replaced with several more specific entries detailing the various bug fixes. --- .../next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst b/Misc/NEWS.d/next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst deleted file mode 100644 index c9df83b480c0ca..00000000000000 --- a/Misc/NEWS.d/next/Library/2019-02-08-04-46-58.bpo-35936.q2ArjU.rst +++ /dev/null @@ -1,2 +0,0 @@ -:mod:`modulefinder` has been refactored to use the non-deprecated :mod:`importlib`. Additionally, several long-standing bugs have been fixed. -Patch by Brandt Bucher. \ No newline at end of file From 8d7f272c9e412354487089e360eda4bdb20730ac Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 9 Feb 2019 10:55:22 -0800 Subject: [PATCH 16/43] Mutate sys.path in-place. Perhaps there is some underlying reference issue with straight-up sys.path replacement. Once again, fingers crossed! --- Lib/modulefinder.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 8a87cc7db95c93..39ebe96b309552 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -461,19 +461,19 @@ def find_module(self, name, path, parent=None): path = self.path - old_path = sys.path + old_path = sys.path[:] old_modules = sys.modules try: - sys.path = path + old_path + sys.path[:0] = path sys.modules = {} spec = importlib.util.find_spec(name) finally: - sys.path = old_path + sys.path[:] = old_path sys.modules = old_modules if spec is None: From 1a594f0ceba5175c8fb3df1c428b3a883691edbe Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 9 Feb 2019 11:23:00 -0800 Subject: [PATCH 17/43] Enter RLock during import search. Perhaps test threads are stomping on each others' sys.path? --- Lib/modulefinder.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 39ebe96b309552..8c68bf32ce70d0 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -7,6 +7,7 @@ import marshal import os import sys +import threading import types import warnings @@ -464,17 +465,19 @@ def find_module(self, name, path, parent=None): old_path = sys.path[:] old_modules = sys.modules - try: + with threading.RLock(): + + try: - sys.path[:0] = path - sys.modules = {} + sys.path[:0] = path + sys.modules = {} - spec = importlib.util.find_spec(name) + spec = importlib.util.find_spec(name) - finally: + finally: - sys.path[:] = old_path - sys.modules = old_modules + sys.path[:] = old_path + sys.modules = old_modules if spec is None: raise ImportError("No module named {name!r}".format(name=name), name=name) From 6dabded23ab481ec2786530ecb6dc6e693dc6450 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 9 Feb 2019 14:41:46 -0800 Subject: [PATCH 18/43] Try multiprocessing.RLock instead of threading. These are processes, after all... --- Lib/modulefinder.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 8c68bf32ce70d0..ab69fc13ea6d37 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -5,9 +5,9 @@ import importlib.util import importlib.machinery import marshal +import multiprocessing import os import sys -import threading import types import warnings @@ -462,14 +462,14 @@ def find_module(self, name, path, parent=None): path = self.path - old_path = sys.path[:] - old_modules = sys.modules + with multiprocessing.RLock(): - with threading.RLock(): + old_path = sys.path[:] + old_modules = sys.modules try: - sys.path[:0] = path + sys.path[1:1] = path sys.modules = {} spec = importlib.util.find_spec(name) From 7095d14cc86bdd3ad13601baecd8240af8bb50a6 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 9 Feb 2019 15:29:21 -0800 Subject: [PATCH 19/43] Try a couple of possible fixes for failing lookups. This is a longshot, but it's pretty much all I have left in terms of ideas. --- Lib/modulefinder.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index ab69fc13ea6d37..5f43be0612449a 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -462,22 +462,27 @@ def find_module(self, name, path, parent=None): path = self.path - with multiprocessing.RLock(): + old_path = sys.path[:] + old_modules = sys.modules.copy() + old_path_importer_cache = sys.path_importer_cache.copy() - old_path = sys.path[:] - old_modules = sys.modules + try: - try: + sys._clear_type_cache() - sys.path[1:1] = path - sys.modules = {} + sys.path[:0] = path + sys.modules.clear() + sys.path_importer_cache.clear() - spec = importlib.util.find_spec(name) + sys._clear_type_cache() - finally: + spec = importlib.util.find_spec(name) + + finally: - sys.path[:] = old_path - sys.modules = old_modules + sys.path[:] = old_path + sys.modules.update(old_modules) + sys.path_importer_cache.update(old_path_importer_cache) if spec is None: raise ImportError("No module named {name!r}".format(name=name), name=name) From 1aea4af4622371ff24f7b6f9903768517aa9b815 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 9 Feb 2019 15:56:25 -0800 Subject: [PATCH 20/43] Remove sys._clear_type_cache calls. Is it a refleak issue? --- Lib/modulefinder.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 5f43be0612449a..776ab10b200877 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -468,14 +468,10 @@ def find_module(self, name, path, parent=None): try: - sys._clear_type_cache() - sys.path[:0] = path sys.modules.clear() sys.path_importer_cache.clear() - sys._clear_type_cache() - spec = importlib.util.find_spec(name) finally: From 603481a8a067bab5303afb7ef4861d7adbc9e3d6 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Sun, 10 Feb 2019 00:25:26 +0000 Subject: [PATCH 21/43] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst b/Misc/NEWS.d/next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst new file mode 100644 index 00000000000000..50596cf9e43f09 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst @@ -0,0 +1,2 @@ +:mod:`modulefinder` no longer crashes when encountering syntax errors in followed imports. +Patch by Brandt Bucher. \ No newline at end of file From f022f83fc5d042fa55eeb00bc341f05cb68510ff Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Sun, 10 Feb 2019 00:47:14 +0000 Subject: [PATCH 22/43] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst b/Misc/NEWS.d/next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst new file mode 100644 index 00000000000000..a9bf8c9a636ccb --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst @@ -0,0 +1,2 @@ +:mod:`modulefinder` correctly handles modules that have the same name as a bad package. +Patch by Brandt Bucher. \ No newline at end of file From 2568d5acb7a6de6d09569634f8b060f8c48ae9d3 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Sun, 10 Feb 2019 17:31:02 +0000 Subject: [PATCH 23/43] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst b/Misc/NEWS.d/next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst new file mode 100644 index 00000000000000..d96a18ab6e1bc2 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst @@ -0,0 +1,2 @@ +:mod:`modulefinder` no longer uses the deprecated :mod:`imp` module. +Patch by Brandt Bucher. \ No newline at end of file From bbae72e76ca723320363f517d24a0c56e81ad8cd Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 10 Feb 2019 09:53:59 -0800 Subject: [PATCH 24/43] Break out new code into standalone function. I've also added helpful documentation outlining the reasoning behind various design decisions. --- Lib/modulefinder.py | 147 ++++++++++++++++++++++++-------------------- 1 file changed, 82 insertions(+), 65 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 776ab10b200877..9b789a77d760dc 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -5,7 +5,6 @@ import importlib.util import importlib.machinery import marshal -import multiprocessing import os import sys import types @@ -53,6 +52,87 @@ def ReplacePackage(oldname, newname): replacePackageMap[oldname] = newname +def _find_module(name, path=None): + """`importlib`-only replacement for `imp.find_module`.""" + + # To correctly find the module on the given path, we need to + # temporarily clear the import state of the program and modify sys.path. + + sys_path = sys.path[:] + sys_modules = sys.modules.copy() + sys_path_importer_cache = sys.path_importer_cache.copy() + + try: + + sys.path[:0] = path + sys.modules.clear() + sys.path_importer_cache.clear() + + # It is important that `name` not include any dots. + # Otherwise, the parent package could be actually imported. + + spec = importlib.util.find_spec(name) + + finally: + + sys.path[:] = sys_path + sys.modules.update(sys_modules) + sys.path_importer_cache.update(sys_path_importer_cache) + + if spec is None: + raise ImportError("No module named {name!r}".format(name=name), name=name) + + if isinstance(spec.loader, type): # Some special cases: + + if issubclass(spec.loader, importlib.machinery.BuiltinImporter): + return None, None, ("", "", _C_BUILTIN) + + if issubclass(spec.loader, importlib.machinery.FrozenImporter): + return None, None, ("", "", _PY_FROZEN) + + file_path = spec.origin + + try: + + # `imp` behavior is s bit funny here: + + # If the path is a descendant of the CWD, it is reported as a relative filepath + # ...unless it is an extension, in which case it will be absolute (see below) + # ...otherwise, it is fully resolved. + + if os.path.commonpath(map(os.path.abspath, (file_path, os.getcwd()))) == os.getcwd(): + file_path = os.path.relpath(file_path) # imp has weird rules + + except ValueError: + pass + + base, suffix = os.path.splitext(file_path) + + if suffix in importlib.machinery.EXTENSION_SUFFIXES: + file_path = os.path.abspath(file_path) + kind = _C_EXTENSION + mode = "rb" + + elif suffix in importlib.machinery.SOURCE_SUFFIXES: + + if name != "__init__" and os.path.basename(base) == "__init__": + return (None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY)) + + kind = _PY_SOURCE + mode = "r" + + elif suffix in importlib.machinery.BYTECODE_SUFFIXES: + kind = _PY_COMPILED + mode = "rb" + + else: # Should never happen. + return None, None, ("", "", _SEARCH_ERROR) + + file = open(file_path, mode) + + return file, file_path, (suffix, mode, kind) + + class Module: def __init__(self, name, file=None, path=None): @@ -462,70 +542,7 @@ def find_module(self, name, path, parent=None): path = self.path - old_path = sys.path[:] - old_modules = sys.modules.copy() - old_path_importer_cache = sys.path_importer_cache.copy() - - try: - - sys.path[:0] = path - sys.modules.clear() - sys.path_importer_cache.clear() - - spec = importlib.util.find_spec(name) - - finally: - - sys.path[:] = old_path - sys.modules.update(old_modules) - sys.path_importer_cache.update(old_path_importer_cache) - - if spec is None: - raise ImportError("No module named {name!r}".format(name=name), name=name) - - if isinstance(spec.loader, type): - - if issubclass(spec.loader, importlib.machinery.BuiltinImporter): - return None, None, ("", "", _C_BUILTIN) - - if issubclass(spec.loader, importlib.machinery.FrozenImporter): - return None, None, ("", "", _PY_FROZEN) - - file_path = spec.origin - - try: - - if os.path.commonpath(map(os.path.abspath, (file_path, os.getcwd()))) == os.getcwd(): - file_path = os.path.relpath(file_path) - - except ValueError: - pass - - base, suffix = os.path.splitext(file_path) - - if suffix in importlib.machinery.EXTENSION_SUFFIXES: - file_path = os.path.abspath(file_path) - kind = _C_EXTENSION - mode = "rb" - - elif suffix in importlib.machinery.SOURCE_SUFFIXES: - - if name != "__init__" and os.path.basename(base) == "__init__": - return (None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY)) - - kind = _PY_SOURCE - mode = "r" - - elif suffix in importlib.machinery.BYTECODE_SUFFIXES: - kind = _PY_COMPILED - mode = "rb" - - else: - return None, None, ("", "", _SEARCH_ERROR) # Should never happen. - - file = open(file_path, mode) - - return file, file_path, (suffix, mode, kind) + return _find_module(name, path) def report(self): """Print a report to stdout, listing the found modules with their From 68337928bcf288dfe6289b672d44ae75459a247e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 10 Feb 2019 10:26:20 -0800 Subject: [PATCH 25/43] Don't modify import state in-place. Changing references seems like a safer option and doesn't require extra copy operations. --- Lib/modulefinder.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 9b789a77d760dc..a2f916c2f0c2ba 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -58,15 +58,15 @@ def _find_module(name, path=None): # To correctly find the module on the given path, we need to # temporarily clear the import state of the program and modify sys.path. - sys_path = sys.path[:] - sys_modules = sys.modules.copy() - sys_path_importer_cache = sys.path_importer_cache.copy() + sys_path = sys.path + sys_modules = sys.modules + sys_path_importer_cache = sys.path_importer_cache try: - sys.path[:0] = path - sys.modules.clear() - sys.path_importer_cache.clear() + sys.path = path + sys.path + sys.modules = {} + sys.path_importer_cache = {} # It is important that `name` not include any dots. # Otherwise, the parent package could be actually imported. @@ -75,9 +75,9 @@ def _find_module(name, path=None): finally: - sys.path[:] = sys_path - sys.modules.update(sys_modules) - sys.path_importer_cache.update(sys_path_importer_cache) + sys.path = sys_path + sys.modules = sys_modules + sys.path_importer_cache = sys_path_importer_cache if spec is None: raise ImportError("No module named {name!r}".format(name=name), name=name) From 0af9f08ad3c4e01aea073b675223d266f5da973f Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 10 Feb 2019 10:31:35 -0800 Subject: [PATCH 26/43] Reorder new import. Minor. Keep our imports in ASCIIbetical order. --- Lib/modulefinder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index a2f916c2f0c2ba..ce317776afe45a 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -2,8 +2,8 @@ import dis import importlib._bootstrap_external -import importlib.util import importlib.machinery +import importlib.util import marshal import os import sys From 4bb452e7e2d3a09bb39b039fee708bcc7ac2746f Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 11 Feb 2019 07:34:00 -0800 Subject: [PATCH 27/43] Minor style stuff. Remove a comment, change a docstring, and add an ACK. --- Lib/modulefinder.py | 4 ++-- Misc/ACKS | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index ce317776afe45a..cdcd7eea3f8a86 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -53,7 +53,7 @@ def ReplacePackage(oldname, newname): def _find_module(name, path=None): - """`importlib`-only replacement for `imp.find_module`.""" + """An importlib reimplementation of imp.find_module (for our purposes).""" # To correctly find the module on the given path, we need to # temporarily clear the import state of the program and modify sys.path. @@ -101,7 +101,7 @@ def _find_module(name, path=None): # ...otherwise, it is fully resolved. if os.path.commonpath(map(os.path.abspath, (file_path, os.getcwd()))) == os.getcwd(): - file_path = os.path.relpath(file_path) # imp has weird rules + file_path = os.path.relpath(file_path) except ValueError: pass diff --git a/Misc/ACKS b/Misc/ACKS index 81b51f75199159..6aaab0fbdad1c9 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -222,6 +222,7 @@ Ian Bruntlett Floris Bruynooghe Matt Bryant Stan Bubrouski +Brandt Bucher Colm Buckley Erik de Bueger Jan-Hein Bührman From a9fd3c654f1f0097a8681d0bfb477e644d1420a9 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 13 Feb 2019 08:42:31 -0800 Subject: [PATCH 28/43] Replace importlib.util.find_spec with importlib.machinery.PathFinder.find_spec. This swap avoids behavior changes and concurrency issues. The reverted changes will be proposed and reviewed separately. Thanks @ncoghlan! --- Lib/modulefinder.py | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index cdcd7eea3f8a86..470285ac6e1e58 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -3,7 +3,6 @@ import dis import importlib._bootstrap_external import importlib.machinery -import importlib.util import marshal import os import sys @@ -55,29 +54,7 @@ def ReplacePackage(oldname, newname): def _find_module(name, path=None): """An importlib reimplementation of imp.find_module (for our purposes).""" - # To correctly find the module on the given path, we need to - # temporarily clear the import state of the program and modify sys.path. - - sys_path = sys.path - sys_modules = sys.modules - sys_path_importer_cache = sys.path_importer_cache - - try: - - sys.path = path + sys.path - sys.modules = {} - sys.path_importer_cache = {} - - # It is important that `name` not include any dots. - # Otherwise, the parent package could be actually imported. - - spec = importlib.util.find_spec(name) - - finally: - - sys.path = sys_path - sys.modules = sys_modules - sys.path_importer_cache = sys_path_importer_cache + spec = importlib.machinery.PathFinder.find_spec(name, path+sys.path) if spec is None: raise ImportError("No module named {name!r}".format(name=name), name=name) From 826eef5440b93fd563351c30ca2aa50b705a09d5 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 13 Feb 2019 09:20:15 -0800 Subject: [PATCH 29/43] Invalidate caches before find_spec call. It appears that we're still hitting some caches. This *should* help... --- Lib/modulefinder.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 470285ac6e1e58..fc4df2e4a0925f 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -54,6 +54,8 @@ def ReplacePackage(oldname, newname): def _find_module(name, path=None): """An importlib reimplementation of imp.find_module (for our purposes).""" + importlib.machinery.PathFinder.invalidate_caches() + spec = importlib.machinery.PathFinder.find_spec(name, path+sys.path) if spec is None: From de22d673a71c1b48444b311c95862cd0d2159b49 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 13 Feb 2019 09:21:00 -0800 Subject: [PATCH 30/43] Fix whitespace. Whoops. --- Lib/modulefinder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index fc4df2e4a0925f..a9cce548e48036 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -55,7 +55,7 @@ def _find_module(name, path=None): """An importlib reimplementation of imp.find_module (for our purposes).""" importlib.machinery.PathFinder.invalidate_caches() - + spec = importlib.machinery.PathFinder.find_spec(name, path+sys.path) if spec is None: From 4de4b3dc1c5fed4aeae253f05a7a1b8d594a5fe8 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 13 Feb 2019 10:55:45 -0800 Subject: [PATCH 31/43] Remove duplicate-bpo NEWS entries. These will be replaced with new ones properly referencing their respective bpo issue numbers. --- .../next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst | 2 -- .../next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst | 2 -- 2 files changed, 4 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst delete mode 100644 Misc/NEWS.d/next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst b/Misc/NEWS.d/next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst deleted file mode 100644 index 50596cf9e43f09..00000000000000 --- a/Misc/NEWS.d/next/Library/2019-02-10-00-25-25.bpo-35936.oKRkrD.rst +++ /dev/null @@ -1,2 +0,0 @@ -:mod:`modulefinder` no longer crashes when encountering syntax errors in followed imports. -Patch by Brandt Bucher. \ No newline at end of file diff --git a/Misc/NEWS.d/next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst b/Misc/NEWS.d/next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst deleted file mode 100644 index a9bf8c9a636ccb..00000000000000 --- a/Misc/NEWS.d/next/Library/2019-02-10-00-47-13.bpo-35936.UFhYLj.rst +++ /dev/null @@ -1,2 +0,0 @@ -:mod:`modulefinder` correctly handles modules that have the same name as a bad package. -Patch by Brandt Bucher. \ No newline at end of file From 3085a62105684ecb94e337801612017a2d8e70bc Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Wed, 13 Feb 2019 18:56:24 +0000 Subject: [PATCH 32/43] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2019-02-13-18-56-22.bpo-17396.oKRkrD.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-02-13-18-56-22.bpo-17396.oKRkrD.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-13-18-56-22.bpo-17396.oKRkrD.rst b/Misc/NEWS.d/next/Library/2019-02-13-18-56-22.bpo-17396.oKRkrD.rst new file mode 100644 index 00000000000000..50596cf9e43f09 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-13-18-56-22.bpo-17396.oKRkrD.rst @@ -0,0 +1,2 @@ +:mod:`modulefinder` no longer crashes when encountering syntax errors in followed imports. +Patch by Brandt Bucher. \ No newline at end of file From 7b76e58faab6aba59abef84170d35dc3fed86214 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Wed, 13 Feb 2019 18:56:28 +0000 Subject: [PATCH 33/43] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2019-02-13-18-56-27.bpo-35376.UFhYLj.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-02-13-18-56-27.bpo-35376.UFhYLj.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-13-18-56-27.bpo-35376.UFhYLj.rst b/Misc/NEWS.d/next/Library/2019-02-13-18-56-27.bpo-35376.UFhYLj.rst new file mode 100644 index 00000000000000..a9bf8c9a636ccb --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-13-18-56-27.bpo-35376.UFhYLj.rst @@ -0,0 +1,2 @@ +:mod:`modulefinder` correctly handles modules that have the same name as a bad package. +Patch by Brandt Bucher. \ No newline at end of file From 744de0c7d451ac4880b612fad2445a4a1226d44b Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 13 Feb 2019 11:34:17 -0800 Subject: [PATCH 34/43] Empty commit. This is to force a rebuild on the PR. From 7371742d8661c6fc8dd2275d3e268fef3ed40d74 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 16 Feb 2019 14:02:34 -0800 Subject: [PATCH 35/43] Remove NEWS entry. This is going to have info about the mutable defaults added. --- .../next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst b/Misc/NEWS.d/next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst deleted file mode 100644 index d96a18ab6e1bc2..00000000000000 --- a/Misc/NEWS.d/next/Library/2019-02-10-17-31-00.bpo-35936.1af0OQ.rst +++ /dev/null @@ -1,2 +0,0 @@ -:mod:`modulefinder` no longer uses the deprecated :mod:`imp` module. -Patch by Brandt Bucher. \ No newline at end of file From 540e819ffeb926ea7d69f8ae847b900a1f244032 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Sat, 16 Feb 2019 22:19:33 +0000 Subject: [PATCH 36/43] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2019-02-16-22-19-32.bpo-35936.Ay5WtD.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-02-16-22-19-32.bpo-35936.Ay5WtD.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-16-22-19-32.bpo-35936.Ay5WtD.rst b/Misc/NEWS.d/next/Library/2019-02-16-22-19-32.bpo-35936.Ay5WtD.rst new file mode 100644 index 00000000000000..55a028ec8349cd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-16-22-19-32.bpo-35936.Ay5WtD.rst @@ -0,0 +1,2 @@ +:mod:`modulefinder` no longer depends on the deprecated :mod:`imp` module, and the initializer for :class:`modulefinder.ModuleFinder` now has immutable default arguments. +Patch by Brandt Bucher. \ No newline at end of file From d6f19a6777e46f50e115f31b3c5bfc7ce82dbb69 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 17 Feb 2019 07:36:00 -0800 Subject: [PATCH 37/43] Remove loader subclass checks. Identity checking is sufficient here. --- Lib/modulefinder.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index a9cce548e48036..22502a14140ff7 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -61,13 +61,13 @@ def _find_module(name, path=None): if spec is None: raise ImportError("No module named {name!r}".format(name=name), name=name) - if isinstance(spec.loader, type): # Some special cases: + # Some special cases: - if issubclass(spec.loader, importlib.machinery.BuiltinImporter): - return None, None, ("", "", _C_BUILTIN) + if spec.loader is importlib.machinery.BuiltinImporter: + return None, None, ("", "", _C_BUILTIN) - if issubclass(spec.loader, importlib.machinery.FrozenImporter): - return None, None, ("", "", _PY_FROZEN) + if spec.loader is importlib.machinery.FrozenImporter: + return None, None, ("", "", _PY_FROZEN) file_path = spec.origin From 04fc93a2f744defcf5083025be75b8cc987be3b8 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 17 Feb 2019 07:42:06 -0800 Subject: [PATCH 38/43] Use spec.loader for package checking. This is more robust than using a string comparison with "__init__". --- Lib/modulefinder.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 22502a14140ff7..d49cfbcae84b70 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -85,7 +85,7 @@ def _find_module(name, path=None): except ValueError: pass - base, suffix = os.path.splitext(file_path) + _, suffix = os.path.splitext(file_path) if suffix in importlib.machinery.EXTENSION_SUFFIXES: file_path = os.path.abspath(file_path) @@ -94,8 +94,8 @@ def _find_module(name, path=None): elif suffix in importlib.machinery.SOURCE_SUFFIXES: - if name != "__init__" and os.path.basename(base) == "__init__": - return (None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY)) + if spec.loader.is_package(name): + return None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY) kind = _PY_SOURCE mode = "r" From a506dc4e9b6a2b8792c741b31ce57563587b58c0 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 17 Feb 2019 08:08:56 -0800 Subject: [PATCH 39/43] Remove sys.path from path. This isn't actually needed (or wanted) here. --- Lib/modulefinder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index d49cfbcae84b70..7511a237359f66 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -56,7 +56,7 @@ def _find_module(name, path=None): importlib.machinery.PathFinder.invalidate_caches() - spec = importlib.machinery.PathFinder.find_spec(name, path+sys.path) + spec = importlib.machinery.PathFinder.find_spec(name, path) if spec is None: raise ImportError("No module named {name!r}".format(name=name), name=name) From d1fa6afb67aa7275e3ab43069d4e2db8befe7e4c Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 17 Feb 2019 08:31:35 -0800 Subject: [PATCH 40/43] Use loader type to classify imports. This is probably a better idea than using the extension alone. --- Lib/modulefinder.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 7511a237359f66..8d4a1e9a1c9ef4 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -87,12 +87,12 @@ def _find_module(name, path=None): _, suffix = os.path.splitext(file_path) - if suffix in importlib.machinery.EXTENSION_SUFFIXES: + if isinstance(spec.loader, importlib.machinery.ExtensionFileLoader): file_path = os.path.abspath(file_path) kind = _C_EXTENSION mode = "rb" - elif suffix in importlib.machinery.SOURCE_SUFFIXES: + elif isinstance(spec.loader, importlib.machinery.SourceFileLoader): if spec.loader.is_package(name): return None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY) @@ -100,7 +100,7 @@ def _find_module(name, path=None): kind = _PY_SOURCE mode = "r" - elif suffix in importlib.machinery.BYTECODE_SUFFIXES: + elif isinstance(spec.loader, importlib.machinery.SourcelessFileLoader): kind = _PY_COMPILED mode = "rb" From 8840bc0d0522d652a9fc60109f188c9a57fa4f54 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 17 Feb 2019 08:33:30 -0800 Subject: [PATCH 41/43] Remove absolute/relative path formatting. Interestingly, it looks like PathFinder does this for us! That's great, because it really cleans up this section a lot. --- Lib/modulefinder.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 8d4a1e9a1c9ef4..c3c872d5d94978 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -71,24 +71,9 @@ def _find_module(name, path=None): file_path = spec.origin - try: - - # `imp` behavior is s bit funny here: - - # If the path is a descendant of the CWD, it is reported as a relative filepath - # ...unless it is an extension, in which case it will be absolute (see below) - # ...otherwise, it is fully resolved. - - if os.path.commonpath(map(os.path.abspath, (file_path, os.getcwd()))) == os.getcwd(): - file_path = os.path.relpath(file_path) - - except ValueError: - pass - _, suffix = os.path.splitext(file_path) if isinstance(spec.loader, importlib.machinery.ExtensionFileLoader): - file_path = os.path.abspath(file_path) kind = _C_EXTENSION mode = "rb" From 654461b3d45d735280ef57cdccb928c4236b50c5 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 17 Feb 2019 08:37:33 -0800 Subject: [PATCH 42/43] Reorder branching. Hopefully we can get a tiny boost here by checking likely outcomes first. --- Lib/modulefinder.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index c3c872d5d94978..7b7435e41887ea 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -71,20 +71,17 @@ def _find_module(name, path=None): file_path = spec.origin - _, suffix = os.path.splitext(file_path) - - if isinstance(spec.loader, importlib.machinery.ExtensionFileLoader): - kind = _C_EXTENSION - mode = "rb" - - elif isinstance(spec.loader, importlib.machinery.SourceFileLoader): - - if spec.loader.is_package(name): - return None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY) + if spec.loader.is_package(name): + return None, os.path.dirname(file_path), ("", "", _PKG_DIRECTORY) + if isinstance(spec.loader, importlib.machinery.SourceFileLoader): kind = _PY_SOURCE mode = "r" + elif isinstance(spec.loader, importlib.machinery.ExtensionFileLoader): + kind = _C_EXTENSION + mode = "rb" + elif isinstance(spec.loader, importlib.machinery.SourcelessFileLoader): kind = _PY_COMPILED mode = "rb" @@ -93,6 +90,7 @@ def _find_module(name, path=None): return None, None, ("", "", _SEARCH_ERROR) file = open(file_path, mode) + suffix = os.path.splitext(file_path)[-1] return file, file_path, (suffix, mode, kind) From 9a732c13e07903e31f1008f6faa39232c4db5d6e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 17 Feb 2019 09:27:39 -0800 Subject: [PATCH 43/43] Add comments on cache invalidation. This makes the unintuitive reasoning behind this call explicit. --- Lib/modulefinder.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py index 7b7435e41887ea..0061ef415ce322 100644 --- a/Lib/modulefinder.py +++ b/Lib/modulefinder.py @@ -54,6 +54,10 @@ def ReplacePackage(oldname, newname): def _find_module(name, path=None): """An importlib reimplementation of imp.find_module (for our purposes).""" + # It's necessary to clear the caches for our Finder first, in case any + # modules are being added/deleted/modified at runtime. In particular, + # test_modulefinder.py changes file tree contents in a cache-breaking way: + importlib.machinery.PathFinder.invalidate_caches() spec = importlib.machinery.PathFinder.find_spec(name, path)