[Python-checkins] cpython: Issue #14605: Make explicit the entries on sys.path_hooks that used to

brett.cannon python-checkins at python.org
Thu Apr 26 02:55:22 CEST 2012


http://hg.python.org/cpython/rev/57d558f1904d
changeset:   76566:57d558f1904d
user:        Brett Cannon <brett at python.org>
date:        Wed Apr 25 20:54:04 2012 -0400
summary:
  Issue #14605: Make explicit the entries on sys.path_hooks that used to
be implicit.

Added a warning for when sys.path_hooks is found to be empty. Also
changed the meaning of None in sys.path_importer_cache to represent
trying sys.path_hooks again (an interpretation of previous semantics).
Also added a warning for when None was found.

The long-term goal is for None in sys.path_importer_cache to represent
the same as imp.NullImporter: no finder found for that sys.path entry.

files:
  Lib/importlib/_bootstrap.py                 |   64 ++-----
  Lib/importlib/test/import_/test_fromlist.py |    4 +-
  Lib/importlib/test/import_/test_path.py     |   82 ++-------
  Lib/pkgutil.py                              |   13 +-
  Lib/runpy.py                                |   12 +-
  Lib/test/test_cmd_line_script.py            |   15 +-
  Misc/NEWS                                   |    5 +
  Python/importlib.h                          |  Bin 
  Python/pythonrun.c                          |    2 +-
  9 files changed, 71 insertions(+), 126 deletions(-)


diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -752,15 +752,15 @@
     """Meta path finder for sys.(path|path_hooks|path_importer_cache)."""
 
     @classmethod
-    def _path_hooks(cls, path, hooks=None):
+    def _path_hooks(cls, path):
         """Search sequence of hooks for a finder for 'path'.
 
         If 'hooks' is false then use sys.path_hooks.
 
         """
-        if hooks is None:
-            hooks = sys.path_hooks
-        for hook in hooks:
+        if not sys.path_hooks:
+            _warnings.warn('sys.path_hooks is empty', ImportWarning)
+        for hook in sys.path_hooks:
             try:
                 return hook(path)
             except ImportError:
@@ -770,14 +770,11 @@
                               path=path)
 
     @classmethod
-    def _path_importer_cache(cls, path, default=None):
+    def _path_importer_cache(cls, path):
         """Get the finder for the path from sys.path_importer_cache.
 
         If the path is not in the cache, find the appropriate finder and cache
-        it. If None is cached, get the default finder and cache that
-        (if applicable).
-
-        Because of NullImporter, some finder should be returned. The only
+        it. Because of NullImporter, some finder should be returned. The only
         explicit fail case is if None is cached but the path cannot be used for
         the default hook, for which ImportError is raised.
 
@@ -790,9 +787,13 @@
             finder = cls._path_hooks(path)
             sys.path_importer_cache[path] = finder
         else:
-            if finder is None and default:
-                # Raises ImportError on failure.
-                finder = default(path)
+            if finder is None:
+                msg = ("'None' in sys.path_importer_cache[{!r}], so retrying "
+                       "finder search; in future versions of Python 'None' "
+                       "will represent no finder".format(path))
+                _warnings.warn(msg, ImportWarning)
+                del sys.path_importer_cache[path]
+                finder = cls._path_hooks(path)
                 sys.path_importer_cache[path] = finder
         return finder
 
@@ -931,29 +932,6 @@
 
 # Import itself ###############################################################
 
-_DEFAULT_PATH_HOOK = None  # Set in _setup()
-
-class _DefaultPathFinder(PathFinder):
-
-    """Subclass of PathFinder that implements implicit semantics for
-    __import__."""
-
-    @classmethod
-    def _path_hooks(cls, path):
-        """Search sys.path_hooks as well as implicit path hooks."""
-        try:
-            return super()._path_hooks(path)
-        except ImportError:
-            implicit_hooks = [_DEFAULT_PATH_HOOK, _imp.NullImporter]
-            return super()._path_hooks(path, implicit_hooks)
-
-    @classmethod
-    def _path_importer_cache(cls, path):
-        """Use the default path hook when None is stored in
-        sys.path_importer_cache."""
-        return super()._path_importer_cache(path, _DEFAULT_PATH_HOOK)
-
-
 class _ImportLockContext:
 
     """Context manager for the import lock."""
@@ -1008,7 +986,7 @@
         raise ValueError("Empty module name")
 
 
-_IMPLICIT_META_PATH = [BuiltinImporter, FrozenImporter, _DefaultPathFinder]
+_IMPLICIT_META_PATH = [BuiltinImporter, FrozenImporter, PathFinder]
 
 _ERR_MSG = 'No module named {!r}'
 
@@ -1203,12 +1181,6 @@
     if builtin_os == 'nt':
         SOURCE_SUFFIXES.append('.pyw')
 
-    supported_loaders = [(ExtensionFileLoader, _suffix_list(3), False),
-                         (SourceFileLoader, _suffix_list(1), True),
-                         (SourcelessFileLoader, _suffix_list(2), True)]
-    setattr(self_module, '_DEFAULT_PATH_HOOK',
-            FileFinder.path_hook(*supported_loaders))
-
 
 def _install(sys_module, _imp_module):
     """Install importlib as the implementation of import.
@@ -1218,6 +1190,8 @@
 
     """
     _setup(sys_module, _imp_module)
-    orig_import = builtins.__import__
-    builtins.__import__ = __import__
-    builtins.__original_import__ = orig_import
+    supported_loaders = [(ExtensionFileLoader, _suffix_list(3), False),
+                         (SourceFileLoader, _suffix_list(1), True),
+                         (SourcelessFileLoader, _suffix_list(2), True)]
+    sys.path_hooks.extend([FileFinder.path_hook(*supported_loaders),
+                           _imp.NullImporter])
diff --git a/Lib/importlib/test/import_/test_fromlist.py b/Lib/importlib/test/import_/test_fromlist.py
--- a/Lib/importlib/test/import_/test_fromlist.py
+++ b/Lib/importlib/test/import_/test_fromlist.py
@@ -1,6 +1,7 @@
 """Test that the semantics relating to the 'fromlist' argument are correct."""
 from .. import util
 from . import util as import_util
+import imp
 import unittest
 
 class ReturnValue(unittest.TestCase):
@@ -73,7 +74,8 @@
     def test_no_module_from_package(self):
         # [no module]
         with util.mock_modules('pkg.__init__') as importer:
-            with util.import_state(meta_path=[importer]):
+            with util.import_state(meta_path=[importer],
+                                   path_hooks=[imp.NullImporter]):
                 module = import_util.import_('pkg', fromlist='non_existent')
                 self.assertEqual(module.__name__, 'pkg')
                 self.assertTrue(not hasattr(module, 'non_existent'))
diff --git a/Lib/importlib/test/import_/test_path.py b/Lib/importlib/test/import_/test_path.py
--- a/Lib/importlib/test/import_/test_path.py
+++ b/Lib/importlib/test/import_/test_path.py
@@ -9,6 +9,7 @@
 from test import support
 from types import MethodType
 import unittest
+import warnings
 
 
 class FinderTests(unittest.TestCase):
@@ -64,12 +65,18 @@
             self.assertTrue(path in sys.path_importer_cache)
             self.assertTrue(sys.path_importer_cache[path] is importer)
 
-    def test_path_importer_cache_has_None(self):
-        # Test that if sys.path_importer_cache has None that None is returned.
-        clear_cache = {path: None for path in sys.path}
-        with util.import_state(path_importer_cache=clear_cache):
-            for name in ('asynchat', 'sys', '<test module>'):
-                self.assertTrue(machinery.PathFinder.find_module(name) is None)
+    def test_empty_path_hooks(self):
+        # Test that if sys.path_hooks is empty a warning is raised and
+        # PathFinder returns None.
+        # tried again (with a warning).
+        with util.import_state(path_importer_cache={}, path_hooks=[],
+                               path=['bogus_path']):
+            with warnings.catch_warnings(record=True) as w:
+                warnings.simplefilter('always')
+                self.assertIsNone(machinery.PathFinder.find_module('os'))
+                self.assertNotIn('os', sys.path_importer_cache)
+                self.assertEqual(len(w), 1)
+                self.assertTrue(issubclass(w[-1].category, ImportWarning))
 
     def test_path_importer_cache_has_None_continues(self):
         # Test that having None in sys.path_importer_cache causes the search to
@@ -78,9 +85,16 @@
         module = '<test module>'
         importer = util.mock_modules(module)
         with util.import_state(path=['1', '2'],
-                            path_importer_cache={'1': None, '2': importer}):
-            loader = machinery.PathFinder.find_module(module)
-            self.assertTrue(loader is importer)
+                            path_importer_cache={'1': None, '2': importer},
+                            path_hooks=[imp.NullImporter]):
+            with warnings.catch_warnings(record=True) as w:
+                warnings.simplefilter('always')
+                loader = machinery.PathFinder.find_module(module)
+                self.assertTrue(loader is importer)
+                self.assertEqual(len(w), 1)
+                warned = w[0]
+                self.assertTrue(issubclass(warned.category, ImportWarning))
+                self.assertIn(repr(None), str(warned.message))
 
     def test_path_importer_cache_empty_string(self):
         # The empty string should create a finder using the cwd.
@@ -94,57 +108,9 @@
             self.assertIn(os.curdir, sys.path_importer_cache)
 
 
-class DefaultPathFinderTests(unittest.TestCase):
-
-    """Test _bootstrap._DefaultPathFinder."""
-
-    def test_implicit_hooks(self):
-        # Test that the implicit path hooks are used.
-        bad_path = '<path>'
-        module = '<module>'
-        assert not os.path.exists(bad_path)
-        existing_path = tempfile.mkdtemp()
-        try:
-            with util.import_state():
-                nothing = _bootstrap._DefaultPathFinder.find_module(module,
-                                                        path=[existing_path])
-                self.assertTrue(nothing is None)
-                self.assertTrue(existing_path in sys.path_importer_cache)
-                result = isinstance(sys.path_importer_cache[existing_path],
-                                    imp.NullImporter)
-                self.assertFalse(result)
-                nothing = _bootstrap._DefaultPathFinder.find_module(module,
-                                                            path=[bad_path])
-                self.assertTrue(nothing is None)
-                self.assertTrue(bad_path in sys.path_importer_cache)
-                self.assertTrue(isinstance(sys.path_importer_cache[bad_path],
-                                           imp.NullImporter))
-        finally:
-            os.rmdir(existing_path)
-
-
-    def test_path_importer_cache_has_None(self):
-        # Test that the default hook is used when sys.path_importer_cache
-        # contains None for a path.
-        module = '<test module>'
-        importer = util.mock_modules(module)
-        path = '<test path>'
-        # XXX Not blackbox.
-        original_hook = _bootstrap._DEFAULT_PATH_HOOK
-        mock_hook = import_util.mock_path_hook(path, importer=importer)
-        _bootstrap._DEFAULT_PATH_HOOK = mock_hook
-        try:
-            with util.import_state(path_importer_cache={path: None}):
-                loader = _bootstrap._DefaultPathFinder.find_module(module,
-                                                                    path=[path])
-                self.assertTrue(loader is importer)
-        finally:
-            _bootstrap._DEFAULT_PATH_HOOK = original_hook
-
-
 def test_main():
     from test.support import run_unittest
-    run_unittest(FinderTests, DefaultPathFinderTests)
+    run_unittest(FinderTests)
 
 if __name__ == '__main__':
     test_main()
diff --git a/Lib/pkgutil.py b/Lib/pkgutil.py
--- a/Lib/pkgutil.py
+++ b/Lib/pkgutil.py
@@ -379,18 +379,15 @@
         for path_hook in sys.path_hooks:
             try:
                 importer = path_hook(path_item)
+                sys.path_importer_cache.setdefault(path_item, importer)
                 break
             except ImportError:
                 pass
         else:
-            importer = None
-        sys.path_importer_cache.setdefault(path_item, importer)
-
-    if importer is None:
-        try:
-            importer = ImpImporter(path_item)
-        except ImportError:
-            importer = None
+            try:
+                importer = ImpImporter(path_item)
+            except ImportError:
+                importer = None
     return importer
 
 
diff --git a/Lib/runpy.py b/Lib/runpy.py
--- a/Lib/runpy.py
+++ b/Lib/runpy.py
@@ -9,6 +9,7 @@
 # Written by Nick Coghlan <ncoghlan at gmail.com>
 #    to implement PEP 338 (Executing Modules as Scripts)
 
+import os
 import sys
 import imp
 from pkgutil import read_code
@@ -94,7 +95,7 @@
     for attr in ("get_filename", "_get_filename"):
         meth = getattr(loader, attr, None)
         if meth is not None:
-            return meth(mod_name)
+            return os.path.abspath(meth(mod_name))
     return None
 
 # Helper to get the loader, code and filename for a module
@@ -198,10 +199,6 @@
     try:
         importer = cache[path_name]
     except KeyError:
-        # Not yet cached. Flag as using the
-        # standard machinery until we finish
-        # checking the hooks
-        cache[path_name] = None
         for hook in sys.path_hooks:
             try:
                 importer = hook(path_name)
@@ -213,10 +210,7 @@
             # NullImporter throws ImportError if the supplied path is a
             # *valid* directory entry (and hence able to be handled
             # by the standard import machinery)
-            try:
-                importer = imp.NullImporter(path_name)
-            except ImportError:
-                return None
+            importer = imp.NullImporter(path_name)
         cache[path_name] = importer
     return importer
 
diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py
--- a/Lib/test/test_cmd_line_script.py
+++ b/Lib/test/test_cmd_line_script.py
@@ -1,5 +1,6 @@
 # tests command line execution of scripts
 
+import importlib
 import unittest
 import sys
 import os
@@ -49,12 +50,16 @@
 """
 
 def _make_test_script(script_dir, script_basename, source=test_source):
-    return make_script(script_dir, script_basename, source)
+    to_return = make_script(script_dir, script_basename, source)
+    importlib.invalidate_caches()
+    return to_return
 
 def _make_test_zip_pkg(zip_dir, zip_basename, pkg_name, script_basename,
                        source=test_source, depth=1):
-    return make_zip_pkg(zip_dir, zip_basename, pkg_name, script_basename,
-                        source, depth)
+    to_return = make_zip_pkg(zip_dir, zip_basename, pkg_name, script_basename,
+                             source, depth)
+    importlib.invalidate_caches()
+    return to_return
 
 # There's no easy way to pass the script directory in to get
 # -m to work (avoiding that is the whole point of making
@@ -72,7 +77,9 @@
     else:
         path = repr(path)
     source = launch_source % (path, module_name)
-    return make_script(script_dir, script_basename, source)
+    to_return = make_script(script_dir, script_basename, source)
+    importlib.invalidate_caches()
+    return to_return
 
 class CmdLineTest(unittest.TestCase):
     def _check_output(self, script_name, exit_code, data,
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,11 @@
 Core and Builtins
 -----------------
 
+- Issue #14605: No longer have implicit entries in sys.path_hooks. If
+  sys.path_hooks is found to be empty, a warning will be raised. If None is
+  found in sys.path_importer_cache, a warning is raised and a search on
+  sys.path_hooks is attempted.
+
 - Issue #13903: Implement PEP 412. Individual dictionary instances can now share
   their keys with other dictionaries. Classes take advantage of this to share
   their instance dictionary keys for improved memory and performance.
diff --git a/Python/importlib.h b/Python/importlib.h
index 991334e6316f63a1c5e7bd01307b045cffd9798e..7a83c0239cb1e9ca030daf74a03bfe3c6859b220
GIT binary patch
[stripped]
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -229,7 +229,7 @@
         Py_FatalError("Py_Initialize: can't save _imp to sys.modules");
     }
 
-    value = PyObject_CallMethod(importlib, "_setup", "OO", sysmod, impmod);
+    value = PyObject_CallMethod(importlib, "_install", "OO", sysmod, impmod);
     if (value == NULL) {
         PyErr_Print();
         Py_FatalError("Py_Initialize: importlib install failed");

-- 
Repository URL: http://hg.python.org/cpython


More information about the Python-checkins mailing list