Patchwork [STABLE,V2] demandimport: do not raise ImportError for unknown item in fromlist

login
register
mail settings
Submitter Yuya Nishihara
Date Dec. 20, 2016, 12:48 p.m.
Message ID <09d785da911964c32827.1482238126@mimosa>
Download mbox | patch
Permalink /patch/17972/
State Accepted
Headers show

Comments

Yuya Nishihara - Dec. 20, 2016, 12:48 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1482155160 -32400
#      Mon Dec 19 22:46:00 2016 +0900
# Branch stable
# Node ID 09d785da911964c3282768438024a3b8e63dbd53
# Parent  7817df5585db1d87d3f6c7f496085c86d87e2e9a
demandimport: do not raise ImportError for unknown item in fromlist

This is the behavior of the default __import__() function, which doesn't
validate the existence of the fromlist items. Later on, the missing attribute
is detected while processing the import statement.

https://hg.python.org/cpython/file/v2.7.13/Python/import.c#l2575

The comtypes library relies on this (maybe) undocumented behavior, and we
got a bug report to TortoiseHg, sigh.

https://bitbucket.org/tortoisehg/thg/issues/4647/

The test added at 26a4e46af2bc verifies the behavior of the import statement,
so this patch only adds the test of __import__() function and works around
CPython/PyPy difference.
Pierre-Yves David - Dec. 21, 2016, 12:08 p.m.
On 12/20/2016 01:48 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1482155160 -32400
> #      Mon Dec 19 22:46:00 2016 +0900
> # Branch stable
> # Node ID 09d785da911964c3282768438024a3b8e63dbd53
> # Parent  7817df5585db1d87d3f6c7f496085c86d87e2e9a
> demandimport: do not raise ImportError for unknown item in fromlist

That one is pushed. Thanks for the test clarification.

Cheers,

Patch

diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -199,8 +199,11 @@  def _demandimport(name, globals=None, lo
             nonpkg = getattr(mod, '__path__', nothing) is nothing
             if symbol is nothing:
                 if nonpkg:
-                    # do not try relative import, which would raise ValueError
-                    raise ImportError('cannot import name %s' % attr)
+                    # do not try relative import, which would raise ValueError,
+                    # and leave unknown attribute as the default __import__()
+                    # would do. the missing attribute will be detected later
+                    # while processing the import statement.
+                    return
                 mn = '%s.%s' % (mod.__name__, attr)
                 if mn in ignore:
                     importfunc = _origimport
diff --git a/tests/test-demandimport.py b/tests/test-demandimport.py
--- a/tests/test-demandimport.py
+++ b/tests/test-demandimport.py
@@ -70,7 +70,16 @@  try:
     print('no demandmod should be created for attribute of non-package '
           'module:\ncontextlib.unknownattr =', f(unknownattr))
 except ImportError as inst:
-    print('contextlib.unknownattr = ImportError: %s' % inst)
+    print('contextlib.unknownattr = ImportError: %s'
+          % rsub(r"'", '', str(inst)))
+
+# Unlike the import statement, __import__() function should not raise
+# ImportError even if fromlist has an unknown item
+# (see Python/import.c:import_module_level() and ensure_fromlist())
+contextlibimp = __import__('contextlib', globals(), locals(), ['unknownattr'])
+print("__import__('contextlib', ..., ['unknownattr']) =", f(contextlibimp))
+print("hasattr(contextlibimp, 'unknownattr') =",
+      util.safehasattr(contextlibimp, 'unknownattr'))
 
 demandimport.disable()
 os.environ['HGDEMANDIMPORT'] = 'disable'
diff --git a/tests/test-demandimport.py.out b/tests/test-demandimport.py.out
--- a/tests/test-demandimport.py.out
+++ b/tests/test-demandimport.py.out
@@ -18,4 +18,6 @@  re.stderr = <open file '<whatever>', mod
 re = <proxied module 'sys'>
 contextlib = <unloaded module 'contextlib'>
 contextlib.unknownattr = ImportError: cannot import name unknownattr
+__import__('contextlib', ..., ['unknownattr']) = <module 'contextlib' from '?'>
+hasattr(contextlibimp, 'unknownattr') = False
 node = <module 'mercurial.node' from '?'>