[Python-checkins] cpython: rollback 005fd1fe31ab (see #14609 and #14582)

[benjamin.peterson]

http://hg.python.org/cpython/rev/db5e3431ee4c
changeset:   76390:db5e3431ee4c
user:        Benjamin Peterson <benjamin at python.org>
date:        Wed Apr 18 10:55:43 2012 -0400
summary:
  rollback 005fd1fe31ab (see #14609 and #14582)

Being able to overload a sys.module entry during import of a module was broken
by this changeset.

files:
  Lib/importlib/_bootstrap.py                |   16 +---
  Lib/importlib/test/import_/test_caching.py |   32 ++++++++-
  Misc/NEWS                                  |    3 -
  Python/import.c                            |   19 +----
  Python/importlib.h                         |  Bin 
  5 files changed, 40 insertions(+), 30 deletions(-)


diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -984,12 +984,12 @@
     loader = _find_module(name, path)
     if loader is None:
         raise ImportError(_ERR_MSG.format(name), name=name)
-    elif name in sys.modules:
-        # The parent module already imported this module.
-        module = sys.modules[name]
-    else:
-        module = loader.load_module(name)
+    elif name not in sys.modules:
+        # The parent import may have already imported this module.
+        loader.load_module(name)
         verbose_message('import {!r} # {!r}', name, loader)
+    # Backwards-compatibility; be nicer to skip the dict lookup.
+    module = sys.modules[name]
     if parent:
         # Set the module as an attribute on its parent.
         parent_module = sys.modules[parent]
@@ -1088,11 +1088,7 @@
         # Return up to the first dot in 'name'. This is complicated by the fact
         # that 'name' may be relative.
         if level == 0:
-            index = name.find('.')
-            if index == -1:
-                return module
-            else:
-                return sys.modules[name[:index]]
+            return sys.modules[name.partition('.')[0]]
         elif not name:
             return module
         else:
diff --git a/Lib/importlib/test/import_/test_caching.py b/Lib/importlib/test/import_/test_caching.py
--- a/Lib/importlib/test/import_/test_caching.py
+++ b/Lib/importlib/test/import_/test_caching.py
@@ -47,12 +47,36 @@
         mock.load_module = MethodType(load_module, mock)
         return mock
 
-    def test_using_loader_return(self):
-        loader_return = 'hi there!'
-        with self.create_mock('module', return_=loader_return) as mock:
+    # __import__ inconsistent between loaders and built-in import when it comes
+    #   to when to use the module in sys.modules and when not to.
+    @import_util.importlib_only
+    def test_using_cache_after_loader(self):
+        # [from cache on return]
+        with self.create_mock('module') as mock:
             with util.import_state(meta_path=[mock]):
                 module = import_util.import_('module')
-                self.assertEqual(module, loader_return)
+                self.assertEqual(id(module), id(sys.modules['module']))
+
+    # See test_using_cache_after_loader() for reasoning.
+    @import_util.importlib_only
+    def test_using_cache_for_assigning_to_attribute(self):
+        # [from cache to attribute]
+        with self.create_mock('pkg.__init__', 'pkg.module') as importer:
+            with util.import_state(meta_path=[importer]):
+                module = import_util.import_('pkg.module')
+                self.assertTrue(hasattr(module, 'module'))
+                self.assertTrue(id(module.module), id(sys.modules['pkg.module']))
+
+    # See test_using_cache_after_loader() for reasoning.
+    @import_util.importlib_only
+    def test_using_cache_for_fromlist(self):
+        # [from cache for fromlist]
+        with self.create_mock('pkg.__init__', 'pkg.module') as importer:
+            with util.import_state(meta_path=[importer]):
+                module = import_util.import_('pkg', fromlist=['module'])
+                self.assertTrue(hasattr(module, 'module'))
+                self.assertEqual(id(module.module),
+                                 id(sys.modules['pkg.module']))
 
 
 def test_main():
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -23,9 +23,6 @@
   fails to import now uses the new path and name attributes from
   Issue #1559549.
 
-- Issue #14582: Import directly returns the module as returned by a loader when
-  possible instead of fetching it from sys.modules.
-
 - Issue #13889: Check and (if necessary) set FPU control word before calling
   any of the dtoa.c string <-> float conversion functions, on MSVC builds of
   Python.  This fixes issues when embedding Python in a Delphi app.
diff --git a/Python/import.c b/Python/import.c
--- a/Python/import.c
+++ b/Python/import.c
@@ -2447,22 +2447,15 @@
             Py_DECREF(partition);
 
             if (level == 0) {
-                if (PyUnicode_GET_LENGTH(name) ==
-                        PyUnicode_GET_LENGTH(front)) {
-                    final_mod = mod;
+                final_mod = PyDict_GetItem(interp->modules, front);
+                Py_DECREF(front);
+                if (final_mod == NULL) {
+                    PyErr_Format(PyExc_KeyError,
+                                 "%R not in sys.modules as expected", front);
                 }
                 else {
-                    final_mod = PyDict_GetItem(interp->modules, front);
-                    if (final_mod == NULL) {
-                        PyErr_Format(PyExc_KeyError,
-                                     "%R not in sys.modules as expected", front);
-                    }
+                    Py_INCREF(final_mod);
                 }
-                Py_DECREF(front);
-                if (final_mod == NULL) {
-                    goto error_with_unlock;
-                }
-                Py_INCREF(final_mod);
             }
             else {
                 Py_ssize_t cut_off = PyUnicode_GET_LENGTH(name) -
diff --git a/Python/importlib.h b/Python/importlib.h
index 392b941b6deaf2be794da994178e0af499f1ba81..6279e00d5b1b2f405e6ff99b2234bf05321b10d7
GIT binary patch
[stripped]

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