Patchwork [3,of,3] demandimport: error out early on missing attribute of non package (issue5373)

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 27, 2016, 2:12 p.m.
Message ID <24aee852593b87f35330.1474985536@mimosa>
Download mbox | patch
Permalink /patch/16793/
State Accepted
Headers show

Comments

Yuya Nishihara - Sept. 27, 2016, 2:12 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1474900137 -32400
#      Mon Sep 26 23:28:57 2016 +0900
# Node ID 24aee852593b87f35330b9c482c4f2a4a2595e1d
# Parent  4a8f4b4339238e71ef27a737bb9af2d8b4adb665
demandimport: error out early on missing attribute of non package (issue5373)

If the parent module isn't a package, all valid attributes must be obtained
from it. We can raise ImportError early if any attributes not found.
Pierre-Yves David - Sept. 27, 2016, 3:09 p.m.
On 09/27/2016 04:12 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1474900137 -32400
> #      Mon Sep 26 23:28:57 2016 +0900
> # Node ID 24aee852593b87f35330b9c482c4f2a4a2595e1d
> # Parent  4a8f4b4339238e71ef27a737bb9af2d8b4adb665
> demandimport: error out early on missing attribute of non package (issue5373)
>
> If the parent module isn't a package, all valid attributes must be obtained
> from it. We can raise ImportError early if any attributes not found.

This seems fine. I've pushed it.

I assume this superseed timeless series about reject?
timeless - Sept. 27, 2016, 8:38 p.m.
It works. So, yes.

On Tue, Sep 27, 2016 at 11:09 AM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 09/27/2016 04:12 PM, Yuya Nishihara wrote:
>>
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1474900137 -32400
>> #      Mon Sep 26 23:28:57 2016 +0900
>> # Node ID 24aee852593b87f35330b9c482c4f2a4a2595e1d
>> # Parent  4a8f4b4339238e71ef27a737bb9af2d8b4adb665
>> demandimport: error out early on missing attribute of non package
>> (issue5373)
>>
>> If the parent module isn't a package, all valid attributes must be
>> obtained
>> from it. We can raise ImportError early if any attributes not found.
>
>
> This seems fine. I've pushed it.
>
> I assume this superseed timeless series about reject?
>
> --
> Pierre-Yves David
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -191,11 +191,16 @@  def _demandimport(name, globals=None, lo
         def processfromitem(mod, attr):
             """Process an imported symbol in the import statement.
 
-            If the symbol doesn't exist in the parent module, it must be a
-            module. We set missing modules up as _demandmod instances.
+            If the symbol doesn't exist in the parent module, and if the
+            parent module is a package, it must be a module. We set missing
+            modules up as _demandmod instances.
             """
             symbol = getattr(mod, attr, nothing)
+            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)
                 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
@@ -63,6 +63,15 @@  print("re =", f(re))
 print("re.stderr =", f(re.stderr))
 print("re =", f(re))
 
+import contextlib
+print("contextlib =", f(contextlib))
+try:
+    from contextlib import unknownattr
+    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)
+
 demandimport.disable()
 os.environ['HGDEMANDIMPORT'] = 'disable'
 # this enable call should not actually enable demandimport!
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
@@ -16,4 +16,6 @@  fred = <proxied module 're'>
 re = <unloaded module 'sys'>
 re.stderr = <open file '<whatever>', mode 'w' at 0x?>
 re = <proxied module 'sys'>
+contextlib = <unloaded module 'contextlib'>
+contextlib.unknownattr = ImportError: cannot import name unknownattr
 node = <module 'mercurial.node' from '?'>