On 12/19/2016 04:18 PM, Yuya Nishihara wrote:
# HG changeset patch
# User Yuya Nishihara <y...@tcha.org>
# Date 1482155160 -32400
#      Mon Dec 19 22:46:00 2016 +0900
# Branch stable
# Node ID 58ebfc1af9f290638bed3cc9faf9d6f2deb0fc20
# 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

O.o …


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.

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(__import__('contextlib'), '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(__import__('contextlib'), 'unknownattr') = False

That message is a bit misleading as we are not testing against
"__import__('contextlib')", but
"__import__('contextlib',…,['unknownattr'])"

So the test code is correct but the test output is a bit confusing. One way to clear this would be to drop the '__import__()' call around contextlib, but I'm open to other idea to make this clearer (including insisting the current form is clear).

It also looks like this while file could be transalted to unittest but this might be another adventure.

 node = <module 'mercurial.node' from '?'>

--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to