Patchwork [1,of,2] demandimport: import sub-module relatively as expected (issue5208)

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Aug. 6, 2016, 1:28 p.m.
Message ID <1881f4a509a31d88a5db.1470490094@juju>
Download mbox | patch
Permalink /patch/16153/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Aug. 6, 2016, 1:28 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1470489873 -32400
#      Sat Aug 06 22:24:33 2016 +0900
# Node ID 1881f4a509a31d88a5db583c6db8d186040eba94
# Parent  2dd8c225e94c19d03713458bbeacc71c30347c51
demandimport: import sub-module relatively as expected (issue5208)

Before this patch, importing sub-module might (1) fail or (2) success
but import incorrect module, because demandimport tries to import
sub-module with level=-1 (on Python 2.x) or level=0 (on Python 3.x),
which is default value of "level" argument at construction of
"_demandmod" proxy object.

  (1) on Python 3.x, importing sub-module always fails to import
      existing sub-module

  (2) both on Python 2.x and 3.x, importing sub-module might import
      same name module on root level unintentionally

      On Python 2.x, existing sub-module is prior to this unexpected
      module. Therefore, this problem hasn't appeared.

To import sub-module relatively as expected, this patch specifies "1"
as import level explicitly at construction of "_demandmod" proxy
object for sub-module.
timeless - Aug. 10, 2016, 6:02 a.m.
FUJIWARA Katsunori wrote:
> Before this patch, importing sub-module might (1) fail or (2) success

success => succeed

and thanks for fixing this.

Patch

diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -117,7 +117,8 @@  class _demandmod(object):
                 if '.' in p:
                     h, t = p.split('.', 1)
                 if getattr(mod, h, nothing) is nothing:
-                    setattr(mod, h, _demandmod(p, mod.__dict__, mod.__dict__))
+                    setattr(mod, h,
+                            _demandmod(p, mod.__dict__, mod.__dict__, 1))
                 elif t:
                     subload(getattr(mod, h), t)
 
@@ -211,7 +212,7 @@  def _demandimport(name, globals=None, lo
             for comp in modname.split('.')[1:]:
                 if getattr(mod, comp, nothing) is nothing:
                     setattr(mod, comp,
-                            _demandmod(comp, mod.__dict__, mod.__dict__))
+                            _demandmod(comp, mod.__dict__, mod.__dict__, 1))
                 mod = getattr(mod, comp)
             return mod
 
diff --git a/tests/test-extension.t b/tests/test-extension.t
--- a/tests/test-extension.t
+++ b/tests/test-extension.t
@@ -432,6 +432,36 @@  Examine module importing.
   REL: this is absextroot.xsub1.xsub2.called.func()
   REL: this relimporter imports 'this is absextroot.relimportee'
 
+Examine whether sub-module is imported relatively as expected.
+
+See also issue5208 for detail about example case on Python 3.x.
+
+  $ f -q $TESTTMP/extlibroot/lsub1/lsub2/notexist.py
+  $TESTTMP/extlibroot/lsub1/lsub2/notexist.py: file not found
+
+  $ cat > $TESTTMP/notexist.py <<EOF
+  > text = 'notexist.py at root is loaded unintentionally\n'
+  > EOF
+
+  $ cat > $TESTTMP/checkrelativity.py <<EOF
+  > from mercurial import cmdutil
+  > cmdtable = {}
+  > command = cmdutil.command(cmdtable)
+  > 
+  > # demand import avoids failure of importing notexist here
+  > import extlibroot.lsub1.lsub2.notexist
+  > 
+  > @command('checkrelativity', [], norepo=True)
+  > def checkrelativity(ui, *args, **opts):
+  >     try:
+  >         ui.write(extlibroot.lsub1.lsub2.notexist.text)
+  >         return 1 # unintentional success
+  >     except ImportError:
+  >         pass # intentional failure
+  > EOF
+
+  $ (PYTHONPATH=${PYTHONPATH}${PATHSEP}${TESTTMP}; hg --config extensions.checkrelativity=$TESTTMP/checkrelativity.py checkrelativity)
+
 #endif
 
   $ cd ..