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.

6 files changed, 65 insertions, 125 deletions

diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index 47ccd41..2f42906 100644
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -752,15 +752,15 @@ class PathFinder:
     """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 @@ class PathFinder:
                               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 @@ class PathFinder:
             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 @@ class FileFinder:
 
 # 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 @@ def _sanity_check(name, package, level):
         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 @@ def _setup(sys_module, _imp_module):
     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 @@ def _install(sys_module, _imp_module):
 
     """
     _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
index b903e8e..4ff5f5e 100644
--- 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 @@ class HandlingFromlist(unittest.TestCase):
     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
index fe47717..a3a4515 100644
--- a/Lib/importlib/test/import_/test_path.py
+++ b/Lib/importlib/test/import_/test_path.py
@@ -9,6 +9,7 @@ import tempfile
 from test import support
 from types import MethodType
 import unittest
+import warnings
 
 
 class FinderTests(unittest.TestCase):
@@ -64,12 +65,18 @@ class FinderTests(unittest.TestCase):
             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 @@ class FinderTests(unittest.TestCase):
         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 @@ class FinderTests(unittest.TestCase):
             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
index ef027be..932abbc 100644
--- a/Lib/pkgutil.py
+++ b/Lib/pkgutil.py
@@ -379,18 +379,15 @@ def get_importer(path_item):
         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
index 31e5e55..f826e04 100644
--- a/Lib/runpy.py
+++ b/Lib/runpy.py
@@ -9,6 +9,7 @@ importers when locating support scripts as well as when importing modules.
 # 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 @@ def _get_filename(loader, mod_name):
     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 @@ def _get_importer(path_name):
     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 @@ def _get_importer(path_name):
             # 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
index 77af347..155aafc 100644
--- 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 @@ print('cwd==%a' % os.getcwd())
 """
 
 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 @@ def _make_launch_script(script_dir, script_basename, module_name, path=None):
     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,