Experiments with making the stdlib hooks more robust

edschofield · edschofield · commit 50ccc8097d30 · 2013-12-08T17:10:15.000+11:00
diff --git a/docs/changelog.rst b/docs/changelog.rst
@@ -2,6 +2,41 @@ What's new
 **********
 
 
+.. whats-new-0.11:
+
+What's new in version 0.11
+==========================
+
+More robust implementation of standard_library hooks
+----------------------------------------------------
+
+``future.standard_library`` now no longer installs import hooks by default.
+These were bleeding into surrounding code, causing incompatibilities with
+modules like ``requests`` (issue #19). 
+
+Now ``future.standard_library`` provides the context manager
+``enable_hooks()``. Use it as follows::
+
+    >>> from future import standard_library
+    >>> with standard_library.enable_hooks():
+    ...     import queue
+    ...     import socketserver
+    ...     from http.client import HTTPConnection
+    >>> import requests
+    >>> # etc.
+
+If you prefer, the following imports are also available directly::
+
+    >>> from future.standard_library import queue
+    >>> from future.standard_library import socketserver
+    >>> from future.standard_library.http.client import HTTPConnection
+
+
+As usual, this has no effect on Python 3.
+
+*Note*: this is a backward-incompatible change.
+
+
 .. whats-new-0.10:
 
 What's new in version 0.10.x
diff --git a/docs/roadmap.rst b/docs/roadmap.rst
@@ -22,8 +22,8 @@ futurize script
 future package
 --------------
 
-- Add more tests for bytes ... preferably all from test_bytes.py in Py3.3.
-- Add disable_hooks() and enable_hooks() as functions in the
+- [Done] Add more tests for bytes ... preferably all from test_bytes.py in Py3.3.
+- [Done] Add remove_hooks() and install_hooks() as functions in the
   :mod:`future.standard_library` module. (See the uprefix module for how
   to do this.)
 
diff --git a/future/standard_library/__init__.py b/future/standard_library/__init__.py
@@ -31,11 +31,11 @@
 
 To turn off the import hooks, use::
 
-    standard_library.disable_hooks()
+    standard_library.remove_hooks()
 
 and to turn it on again, use::
 
-    standard_library.enable_hooks()
+    standard_library.install_hooks()
 
 This is a cleaner alternative to this idiom (see
 http://docs.pythonsprints.com/python3_porting/py-porting.html)::
@@ -102,6 +102,7 @@
 #   pickle (fast one)
 #   dbm
 #   urllib
+#   test
 
 # These ones are new (i.e. no problem)
 #   http
@@ -208,7 +209,7 @@ class RenameImport(object):
     # Different RenameImport classes are created when importing this module from
     # different source files. This causes isinstance(hook, RenameImport) checks
     # to produce inconsistent results. We add this RENAMER attribute here so
-    # disable_hooks() and enable_hooks() can find instances of these classes
+    # remove_hooks() and install_hooks() can find instances of these classes
     # easily:
     RENAMER = True
 
@@ -325,7 +326,53 @@ def _find_and_load_module(self, name, path=None):
         ]
 
 
-def enable_hooks():
+class enable_hooks(object):
+    """
+    Acts as a context manager. Use like this:
+    
+    >>> from future import standard_library
+    >>> with standard_library.enable_hooks():
+    ...     import http.client
+    >>> import requests     # incompatible with ``future``'s standard library hooks
+    """
+    def __enter__(self):
+        print('Entering CM')
+        self.hooks_were_installed = detect_hooks()
+        install_hooks()
+        return self
+
+    def __exit__(self, *args):
+        print('Exiting CM')
+        if not self.hooks_were_installed:
+            remove_hooks()
+
+
+class suspend_hooks(object):
+    """
+    Acts as a context manager. Use like this:
+    
+    >>> from future import standard_library
+    >>> standard_library.install_hooks()
+    >>> import http.client
+    >>> # ...
+    >>> with standard_library.suspend_hooks():
+    >>>     import requests     # incompatible with ``future``'s standard library hooks
+
+    If the hooks were disabled before the context, they are not installed when
+    the context is left.
+    """
+    def __enter__(self):
+        self.hooks_were_installed = detect_hooks()
+        remove_hooks()
+        return self
+    def __exit__(self, *args):
+        if not self.hooks_were_installed:
+            install_hooks()
+
+
+def install_hooks():
+    print('sys.meta_path was: {}'.format(sys.meta_path))
+    print('Installing hooks ...')
     if utils.PY3:
         return
     for (newmodname, newobjname, oldmodname, oldobjname) in MOVES:
@@ -336,28 +383,58 @@ def enable_hooks():
 
     # Add it unless it's there already
     newhook = RenameImport(RENAMES)
-    if not any([hasattr(hook, 'RENAMER') for hook in sys.meta_path]):
+    if not detect_hooks():
         sys.meta_path.append(newhook)
+    print('sys.meta_path is now: {}'.format(sys.meta_path))
 
 
-def disable_hooks():
+def remove_hooks():
+    """
+    Use to remove the ``future.standard_library`` import hooks.
+    """
+    print('sys.meta_path was: {}'.format(sys.meta_path))
+    print('Uninstalling hooks ...')
     if not utils.PY3:
         # Loop backwards, so deleting items keeps the ordering:
         for i, hook in list(enumerate(sys.meta_path))[::-1]:
             if hasattr(hook, 'RENAMER'):
                 del sys.meta_path[i]
+    print('sys.meta_path is now: {}'.format(sys.meta_path))
 
 
-@contextlib.contextmanager
-def suspend_hooks():
-    disable_hooks()
-    try:
-        yield
-    except Exception as e:
-        raise e
-    finally:
-        enable_hooks()
+def disable_hooks():
+    """
+    Deprecated. Use remove_hooks() instead. This will be removed by
+    ``future`` v1.0.
+    """
+    remove_hooks()
 
 
-if not utils.PY3:
-    enable_hooks()
+def detect_hooks():
+    """
+    Returns True if the import hooks are installed, False if not.
+    """
+    print('Detecting hooks ...')
+    present = any([hasattr(hook, 'RENAMER') for hook in sys.meta_path])
+    if present:
+        print('Detected.')
+    else:
+        print('Not detected.')
+    return present
+
+
+# Now import the modules:
+with enable_hooks():
+    for (oldname, newname) in RENAMES.items():
+        if newname == 'winreg' and sys.platform not in ['win32', 'win64']:
+            continue
+        if newname in REPLACED_MODULES:
+            # Skip this check for e.g. the stdlib's ``test`` module,
+            # which we have replaced completely.
+            continue
+        newmod = __import__(newname)
+        globals()[newname] = newmod
+
+
+# if not utils.PY3:
+#     install_hooks()
diff --git a/future/tests/test_standard_library.py b/future/tests/test_standard_library.py
@@ -47,9 +47,9 @@ def test_suspend_hooks(self):
         """
         example_PY2_check = False
         with standard_library.suspend_hooks():
-            # An example of fragile import code that we don't want to break:
+            # An example of code that we don't want to break:
             try:
-                import builtins
+                import builtins     # fragile check for Python 3.x
             except ImportError:
                 example_PY2_check = True
         if utils.PY2:
@@ -59,13 +59,14 @@ def test_suspend_hooks(self):
         # The import should succeed again now:
         import builtins
 
-    def test_disable_hooks(self):
+    def test_remove_hooks(self):
         example_PY2_check = False
 
-        standard_library.enable_hooks()
+        standard_library.install_hooks()
         old_meta_path = copy.copy(sys.meta_path)
+        import builtins
 
-        standard_library.disable_hooks()
+        standard_library.remove_hooks()
         self.assertTrue(len(old_meta_path) == len(sys.meta_path) + 1)
 
         # An example of fragile import code that we don't want to break:
@@ -77,11 +78,33 @@ def test_disable_hooks(self):
             self.assertTrue(example_PY2_check)
         else:
             self.assertFalse(example_PY2_check)
-        standard_library.enable_hooks()
+        standard_library.install_hooks()
         # The import should succeed again now:
         import builtins
         self.assertTrue(len(old_meta_path) == len(sys.meta_path))
 
+    def test_remove_hooks2(self):
+        """
+        This verifies that modules like http.client are no longer accessible after
+        disabling import hooks, even if they have been previously imported.
+        
+        The reason for this test is that Python caches imported modules in sys.modules.
+        """
+        standard_library.remove_hooks()
+        try:
+            from . import verify_remove_hooks_affects_imported_modules
+        except RuntimeError as e:
+            self.fail(e.message)
+        finally:
+            standard_library.install_hooks()
+
+    def test_requests(self):
+        """
+        GitHub issue #19: conflict with ``requests``
+        """
+        # The below should succeed while ``requests`` is installed:
+        from . import verify_requests_is_not_broken
+
     @unittest.skipIf(utils.PY3, 'not testing for old urllib on Py3')
     def test_old_urllib_import(self):
         """
@@ -143,23 +166,23 @@ def test_itertools_zip_longest(self):
         self.assertEqual(list(zip_longest(a, b)),
                          [(1, 2), (2, 4), (None, 6)])
 
-    def test_import_from_module(self):
-        """
-        Tests whether e.g. "import socketserver" succeeds in a module
-        imported by another module.
-        """
-        code1 = '''
-                from future import standard_library
-                import importme2
-                '''
-        code2 = '''
-                import socketserver
-                print('Import succeeded!')
-                '''
-        self._write_test_script(code1, 'importme1.py')
-        self._write_test_script(code2, 'importme2.py')
-        output = self._run_test_script('importme1.py')
-        print(output)
+    # def test_import_from_module(self):
+    #     """
+    #     Tests whether e.g. "import socketserver" succeeds in a module
+    #     imported by another module. We do not want it to!
+    #     """
+    #     code1 = '''
+    #             from future import standard_library
+    #             import importme2
+    #             '''
+    #     code2 = '''
+    #             import socketserver
+    #             print('Import succeeded!')
+    #             '''
+    #     self._write_test_script(code1, 'importme1.py')
+    #     self._write_test_script(code2, 'importme2.py')
+    #     output = self._run_test_script('importme1.py')
+    #     print(output)
 
     def test_configparser(self):
         import configparser
