Patchwork [1,of,1] demandimport: allow extensions to import own modules by absolute name

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Aug. 26, 2013, 8:23 a.m.
Message ID <8901b0024b1713147552.1377505438@feefifofum>
Download mbox | patch
Permalink /patch/2262/
State Superseded
Commit 621a26eb3a99a8a31f71ccafba7ef587af88fec8
Headers show

Comments

Katsunori FUJIWARA - Aug. 26, 2013, 8:23 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1377503444 -32400
#      Mon Aug 26 16:50:44 2013 +0900
# Node ID 8901b0024b1713147552e1c7e23113cb901dd705
# Parent  d4a0055af149cdea20b3136b66cae8a24b2e2a98
demandimport: allow extensions to import own modules by absolute name

Before this patch, python modules of each extensions can't import
another one in own extension by absolute name, because root modules of
each extensions are loaded with "hgext_" prefix.

For example, "import extroot.bar" in "extroot/foo.py" of "extroot"
extension fails, even though "import bar" in it succeeds.

Installing extensions into site-packages of python library path can
avoid this problem, but this solution is not reasonable in some cases:
using binary package of Mercurial on Windows, for example.

This patch retries to import with "hgext_" prefix after ImportError,
if the module in the extension may try to import another one in own
extension.

This patch doesn't change some "_import()"/"_origimport()" invocations
below, because ordinary extensions shouldn't cause such invocations.

    - invocation of "_import()" when root module imports sub-module by
      absolute path without "fromlist"

      for example, "import a.b" in "a.__init__.py".

      extensions can't import in this way without explicit "hgext_"
      prefix, because they are loaded with such prefix.

    - invocation of "_origimport()" when "level != -1" with "fromlist"

      for example, importing after "from __future__ import
      absolute_import" (level == 0), or "from . import b" or "from .a
      import b" (0 < level),

      for portability between python versions and environments,
      extensions shouldn't cause "level != -1".
Augie Fackler - Aug. 26, 2013, 1:53 p.m.
On Mon, Aug 26, 2013 at 05:23:58PM +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1377503444 -32400
> #      Mon Aug 26 16:50:44 2013 +0900
> # Node ID 8901b0024b1713147552e1c7e23113cb901dd705
> # Parent  d4a0055af149cdea20b3136b66cae8a24b2e2a98
> demandimport: allow extensions to import own modules by absolute name

Sounds pretty useful. Can you rework the test so the module isn't in
$PWD when hg runs and it tries to import itself? Right now the test
doesn't actually prove this works, as far as I recall.

>
> Before this patch, python modules of each extensions can't import
> another one in own extension by absolute name, because root modules of
> each extensions are loaded with "hgext_" prefix.
>
> For example, "import extroot.bar" in "extroot/foo.py" of "extroot"
> extension fails, even though "import bar" in it succeeds.
>
> Installing extensions into site-packages of python library path can
> avoid this problem, but this solution is not reasonable in some cases:
> using binary package of Mercurial on Windows, for example.
>
> This patch retries to import with "hgext_" prefix after ImportError,
> if the module in the extension may try to import another one in own
> extension.
>
> This patch doesn't change some "_import()"/"_origimport()" invocations
> below, because ordinary extensions shouldn't cause such invocations.
>
>     - invocation of "_import()" when root module imports sub-module by
>       absolute path without "fromlist"
>
>       for example, "import a.b" in "a.__init__.py".
>
>       extensions can't import in this way without explicit "hgext_"
>       prefix, because they are loaded with such prefix.
>
>     - invocation of "_origimport()" when "level != -1" with "fromlist"
>
>       for example, importing after "from __future__ import
>       absolute_import" (level == 0), or "from . import b" or "from .a
>       import b" (0 < level),
>
>       for portability between python versions and environments,
>       extensions shouldn't cause "level != -1".
>
> diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
> --- a/mercurial/demandimport.py
> +++ b/mercurial/demandimport.py
> @@ -38,6 +38,21 @@
>  else:
>      _import = _origimport
>
> +def _hgextimport(importfunc, name, globals, *args):
> +    try:
> +        return importfunc(name, globals, *args)
> +    except ImportError:
> +        if not globals:
> +            raise
> +        # extensions are loaded with "hgext_" prefix
> +        hgextname = 'hgext_%s' % name
> +        nameroot = hgextname.split('.', 1)[0]
> +        contextroot = globals.get('__name__', '').split('.', 1)[0]
> +        if nameroot != contextroot:
> +            raise
> +        # retry to import with "hgext_" prefix
> +        return importfunc(hgextname, globals, *args)
> +
>  class _demandmod(object):
>      """module demand-loader and proxy"""
>      def __init__(self, name, globals, locals):
> @@ -55,7 +70,7 @@
>      def _load(self):
>          if not self._module:
>              head, globals, locals, after = self._data
> -            mod = _origimport(head, globals, locals)
> +            mod = _hgextimport(_origimport, head, globals, locals)
>              # load submodules
>              def subload(mod, p):
>                  h, t = p, None
> @@ -92,7 +107,7 @@
>  def _demandimport(name, globals=None, locals=None, fromlist=None, level=-1):
>      if not locals or name in ignore or fromlist == ('*',):
>          # these cases we can't really delay
> -        return _import(name, globals, locals, fromlist, level)
> +        return _hgextimport(_import, name, globals, locals, fromlist, level)
>      elif not fromlist:
>          # import a [as b]
>          if '.' in name: # a.b
> @@ -111,7 +126,7 @@
>              # from . import b,c,d or from .a import b,c,d
>              return _origimport(name, globals, locals, fromlist, level)
>          # from a import b,c,d
> -        mod = _origimport(name, globals, locals)
> +        mod = _hgextimport(_origimport, name, globals, locals)
>          # recurse down the module chain
>          for comp in name.split('.')[1:]:
>              if getattr(mod, comp, nothing) is nothing:
> diff --git a/tests/test-extension.t b/tests/test-extension.t
> --- a/tests/test-extension.t
> +++ b/tests/test-extension.t
> @@ -129,6 +129,73 @@
>    $ echo 'foo = !' >> $HGRCPATH
>    $ echo 'bar = !' >> $HGRCPATH
>
> +Check absolute/relative import of extension specific modules
> +
> +  $ mkdir extroot
> +  $ cat > extroot/__init__.py <<EOF
> +  > extrootstr = 'this is extroot.extrootstr'
> +  > import foo
> +  > def extsetup(ui):
> +  >     ui.write('(extroot) ', foo.func(), '\n')
> +  > EOF
> +  $ cat > extroot/foo.py <<EOF
> +  > buf = []
> +  > def func():
> +  >     # "not locals" case
> +  >     import bar         # relative
> +  >     buf.append('import bar in func(): %s' % bar.barstr)
> +  >     import extroot.bar # absolute
> +  >     buf.append('import extroot.bar in func(): %s' % extroot.bar.barstr)
> +  >
> +  >     return '\n(extroot) '.join(buf)
> +  >
> +  > # "fromlist == ('*',)" case
> +  > from bar import *         # relative
> +  > buf.append('from bar import *: %s' % barstr)
> +  > from extroot.bar import * # absolute
> +  > buf.append('from extroot.bar import *: %s' % barstr)
> +  >
> +  > # "not fromlist" and "if '.' in name" case
> +  > import sub1.baz         # relative
> +  > buf.append('import sub1.baz: %s' % sub1.baz.bazstr)
> +  > import extroot.sub1.baz # absolute
> +  > buf.append('import extroot.sub1.baz: %s' % extroot.sub1.baz.bazstr)
> +  >
> +  > # "not fromlist" and NOT "if '.' in name" case
> +  > import sub1    # relative
> +  > buf.append('import sub1: %s' % sub1.sub1str)
> +  > import extroot # absolute
> +  > buf.append('import extroot: %s' % extroot.extrootstr)
> +  >
> +  > # NOT "not fromlist" and NOT "level != -1" case
> +  > from bar import barstr         # relative
> +  > buf.append('from bar import barstr: %s' % barstr)
> +  > from extroot.bar import barstr # absolute
> +  > buf.append('from extroot.bar import barstr: %s' % barstr)
> +  > EOF
> +  $ cat > extroot/bar.py <<EOF
> +  > barstr = 'this is extroot.bar.barstr'
> +  > EOF
> +  $ mkdir extroot/sub1
> +  $ cat > extroot/sub1/__init__.py <<EOF
> +  > sub1str = 'this is extroot.sub1.sub1str'
> +  > EOF
> +  $ cat > extroot/sub1/baz.py <<EOF
> +  > bazstr = 'this is extroot.sub1.baz.bazstr'
> +  > EOF
> +  $ hg --config extensions.extroot=extroot tip --template '{rev}\n'
> +  (extroot) from bar import *: this is extroot.bar.barstr
> +  (extroot) from extroot.bar import *: this is extroot.bar.barstr
> +  (extroot) import sub1.baz: this is extroot.sub1.baz.bazstr
> +  (extroot) import extroot.sub1.baz: this is extroot.sub1.baz.bazstr
> +  (extroot) import sub1: this is extroot.sub1.sub1str
> +  (extroot) import extroot: this is extroot.extrootstr
> +  (extroot) from bar import barstr: this is extroot.bar.barstr
> +  (extroot) from extroot.bar import barstr: this is extroot.bar.barstr
> +  (extroot) import bar in func(): this is extroot.bar.barstr
> +  (extroot) import extroot.bar in func(): this is extroot.bar.barstr
> +  0
> +
>    $ cd ..
>
>  hide outer repo
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Katsunori FUJIWARA - Aug. 27, 2013, 7:26 a.m.
At Mon, 26 Aug 2013 09:53:23 -0400,
Augie Fackler wrote:
> 
> On Mon, Aug 26, 2013 at 05:23:58PM +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1377503444 -32400
> > #      Mon Aug 26 16:50:44 2013 +0900
> > # Node ID 8901b0024b1713147552e1c7e23113cb901dd705
> > # Parent  d4a0055af149cdea20b3136b66cae8a24b2e2a98
> > demandimport: allow extensions to import own modules by absolute name
> 
> Sounds pretty useful. Can you rework the test so the module isn't in
> $PWD when hg runs and it tries to import itself? Right now the test
> doesn't actually prove this works, as far as I recall.

OK, I'll resend rewritten one, soon.

> >
> > Before this patch, python modules of each extensions can't import
> > another one in own extension by absolute name, because root modules of
> > each extensions are loaded with "hgext_" prefix.
> >
> > For example, "import extroot.bar" in "extroot/foo.py" of "extroot"
> > extension fails, even though "import bar" in it succeeds.
> >
> > Installing extensions into site-packages of python library path can
> > avoid this problem, but this solution is not reasonable in some cases:
> > using binary package of Mercurial on Windows, for example.
> >
> > This patch retries to import with "hgext_" prefix after ImportError,
> > if the module in the extension may try to import another one in own
> > extension.
> >
> > This patch doesn't change some "_import()"/"_origimport()" invocations
> > below, because ordinary extensions shouldn't cause such invocations.
> >
> >     - invocation of "_import()" when root module imports sub-module by
> >       absolute path without "fromlist"
> >
> >       for example, "import a.b" in "a.__init__.py".
> >
> >       extensions can't import in this way without explicit "hgext_"
> >       prefix, because they are loaded with such prefix.
> >
> >     - invocation of "_origimport()" when "level != -1" with "fromlist"
> >
> >       for example, importing after "from __future__ import
> >       absolute_import" (level == 0), or "from . import b" or "from .a
> >       import b" (0 < level),
> >
> >       for portability between python versions and environments,
> >       extensions shouldn't cause "level != -1".
> >
> > diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
> > --- a/mercurial/demandimport.py
> > +++ b/mercurial/demandimport.py
> > @@ -38,6 +38,21 @@
> >  else:
> >      _import = _origimport
> >
> > +def _hgextimport(importfunc, name, globals, *args):
> > +    try:
> > +        return importfunc(name, globals, *args)
> > +    except ImportError:
> > +        if not globals:
> > +            raise
> > +        # extensions are loaded with "hgext_" prefix
> > +        hgextname = 'hgext_%s' % name
> > +        nameroot = hgextname.split('.', 1)[0]
> > +        contextroot = globals.get('__name__', '').split('.', 1)[0]
> > +        if nameroot != contextroot:
> > +            raise
> > +        # retry to import with "hgext_" prefix
> > +        return importfunc(hgextname, globals, *args)
> > +
> >  class _demandmod(object):
> >      """module demand-loader and proxy"""
> >      def __init__(self, name, globals, locals):
> > @@ -55,7 +70,7 @@
> >      def _load(self):
> >          if not self._module:
> >              head, globals, locals, after = self._data
> > -            mod = _origimport(head, globals, locals)
> > +            mod = _hgextimport(_origimport, head, globals, locals)
> >              # load submodules
> >              def subload(mod, p):
> >                  h, t = p, None
> > @@ -92,7 +107,7 @@
> >  def _demandimport(name, globals=None, locals=None, fromlist=None, level=-1):
> >      if not locals or name in ignore or fromlist == ('*',):
> >          # these cases we can't really delay
> > -        return _import(name, globals, locals, fromlist, level)
> > +        return _hgextimport(_import, name, globals, locals, fromlist, level)
> >      elif not fromlist:
> >          # import a [as b]
> >          if '.' in name: # a.b
> > @@ -111,7 +126,7 @@
> >              # from . import b,c,d or from .a import b,c,d
> >              return _origimport(name, globals, locals, fromlist, level)
> >          # from a import b,c,d
> > -        mod = _origimport(name, globals, locals)
> > +        mod = _hgextimport(_origimport, name, globals, locals)
> >          # recurse down the module chain
> >          for comp in name.split('.')[1:]:
> >              if getattr(mod, comp, nothing) is nothing:
> > diff --git a/tests/test-extension.t b/tests/test-extension.t
> > --- a/tests/test-extension.t
> > +++ b/tests/test-extension.t
> > @@ -129,6 +129,73 @@
> >    $ echo 'foo = !' >> $HGRCPATH
> >    $ echo 'bar = !' >> $HGRCPATH
> >
> > +Check absolute/relative import of extension specific modules
> > +
> > +  $ mkdir extroot
> > +  $ cat > extroot/__init__.py <<EOF
> > +  > extrootstr = 'this is extroot.extrootstr'
> > +  > import foo
> > +  > def extsetup(ui):
> > +  >     ui.write('(extroot) ', foo.func(), '\n')
> > +  > EOF
> > +  $ cat > extroot/foo.py <<EOF
> > +  > buf = []
> > +  > def func():
> > +  >     # "not locals" case
> > +  >     import bar         # relative
> > +  >     buf.append('import bar in func(): %s' % bar.barstr)
> > +  >     import extroot.bar # absolute
> > +  >     buf.append('import extroot.bar in func(): %s' % extroot.bar.barstr)
> > +  >
> > +  >     return '\n(extroot) '.join(buf)
> > +  >
> > +  > # "fromlist == ('*',)" case
> > +  > from bar import *         # relative
> > +  > buf.append('from bar import *: %s' % barstr)
> > +  > from extroot.bar import * # absolute
> > +  > buf.append('from extroot.bar import *: %s' % barstr)
> > +  >
> > +  > # "not fromlist" and "if '.' in name" case
> > +  > import sub1.baz         # relative
> > +  > buf.append('import sub1.baz: %s' % sub1.baz.bazstr)
> > +  > import extroot.sub1.baz # absolute
> > +  > buf.append('import extroot.sub1.baz: %s' % extroot.sub1.baz.bazstr)
> > +  >
> > +  > # "not fromlist" and NOT "if '.' in name" case
> > +  > import sub1    # relative
> > +  > buf.append('import sub1: %s' % sub1.sub1str)
> > +  > import extroot # absolute
> > +  > buf.append('import extroot: %s' % extroot.extrootstr)
> > +  >
> > +  > # NOT "not fromlist" and NOT "level != -1" case
> > +  > from bar import barstr         # relative
> > +  > buf.append('from bar import barstr: %s' % barstr)
> > +  > from extroot.bar import barstr # absolute
> > +  > buf.append('from extroot.bar import barstr: %s' % barstr)
> > +  > EOF
> > +  $ cat > extroot/bar.py <<EOF
> > +  > barstr = 'this is extroot.bar.barstr'
> > +  > EOF
> > +  $ mkdir extroot/sub1
> > +  $ cat > extroot/sub1/__init__.py <<EOF
> > +  > sub1str = 'this is extroot.sub1.sub1str'
> > +  > EOF
> > +  $ cat > extroot/sub1/baz.py <<EOF
> > +  > bazstr = 'this is extroot.sub1.baz.bazstr'
> > +  > EOF
> > +  $ hg --config extensions.extroot=extroot tip --template '{rev}\n'
> > +  (extroot) from bar import *: this is extroot.bar.barstr
> > +  (extroot) from extroot.bar import *: this is extroot.bar.barstr
> > +  (extroot) import sub1.baz: this is extroot.sub1.baz.bazstr
> > +  (extroot) import extroot.sub1.baz: this is extroot.sub1.baz.bazstr
> > +  (extroot) import sub1: this is extroot.sub1.sub1str
> > +  (extroot) import extroot: this is extroot.extrootstr
> > +  (extroot) from bar import barstr: this is extroot.bar.barstr
> > +  (extroot) from extroot.bar import barstr: this is extroot.bar.barstr
> > +  (extroot) import bar in func(): this is extroot.bar.barstr
> > +  (extroot) import extroot.bar in func(): this is extroot.bar.barstr
> > +  0
> > +
> >    $ cd ..
> >
> >  hide outer repo
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -38,6 +38,21 @@ 
 else:
     _import = _origimport
 
+def _hgextimport(importfunc, name, globals, *args):
+    try:
+        return importfunc(name, globals, *args)
+    except ImportError:
+        if not globals:
+            raise
+        # extensions are loaded with "hgext_" prefix
+        hgextname = 'hgext_%s' % name
+        nameroot = hgextname.split('.', 1)[0]
+        contextroot = globals.get('__name__', '').split('.', 1)[0]
+        if nameroot != contextroot:
+            raise
+        # retry to import with "hgext_" prefix
+        return importfunc(hgextname, globals, *args)
+
 class _demandmod(object):
     """module demand-loader and proxy"""
     def __init__(self, name, globals, locals):
@@ -55,7 +70,7 @@ 
     def _load(self):
         if not self._module:
             head, globals, locals, after = self._data
-            mod = _origimport(head, globals, locals)
+            mod = _hgextimport(_origimport, head, globals, locals)
             # load submodules
             def subload(mod, p):
                 h, t = p, None
@@ -92,7 +107,7 @@ 
 def _demandimport(name, globals=None, locals=None, fromlist=None, level=-1):
     if not locals or name in ignore or fromlist == ('*',):
         # these cases we can't really delay
-        return _import(name, globals, locals, fromlist, level)
+        return _hgextimport(_import, name, globals, locals, fromlist, level)
     elif not fromlist:
         # import a [as b]
         if '.' in name: # a.b
@@ -111,7 +126,7 @@ 
             # from . import b,c,d or from .a import b,c,d
             return _origimport(name, globals, locals, fromlist, level)
         # from a import b,c,d
-        mod = _origimport(name, globals, locals)
+        mod = _hgextimport(_origimport, name, globals, locals)
         # recurse down the module chain
         for comp in name.split('.')[1:]:
             if getattr(mod, comp, nothing) is nothing:
diff --git a/tests/test-extension.t b/tests/test-extension.t
--- a/tests/test-extension.t
+++ b/tests/test-extension.t
@@ -129,6 +129,73 @@ 
   $ echo 'foo = !' >> $HGRCPATH
   $ echo 'bar = !' >> $HGRCPATH
 
+Check absolute/relative import of extension specific modules
+
+  $ mkdir extroot
+  $ cat > extroot/__init__.py <<EOF
+  > extrootstr = 'this is extroot.extrootstr'
+  > import foo
+  > def extsetup(ui):
+  >     ui.write('(extroot) ', foo.func(), '\n')
+  > EOF
+  $ cat > extroot/foo.py <<EOF
+  > buf = []
+  > def func():
+  >     # "not locals" case
+  >     import bar         # relative
+  >     buf.append('import bar in func(): %s' % bar.barstr)
+  >     import extroot.bar # absolute
+  >     buf.append('import extroot.bar in func(): %s' % extroot.bar.barstr)
+  > 
+  >     return '\n(extroot) '.join(buf)
+  > 
+  > # "fromlist == ('*',)" case
+  > from bar import *         # relative
+  > buf.append('from bar import *: %s' % barstr)
+  > from extroot.bar import * # absolute
+  > buf.append('from extroot.bar import *: %s' % barstr)
+  > 
+  > # "not fromlist" and "if '.' in name" case
+  > import sub1.baz         # relative
+  > buf.append('import sub1.baz: %s' % sub1.baz.bazstr)
+  > import extroot.sub1.baz # absolute
+  > buf.append('import extroot.sub1.baz: %s' % extroot.sub1.baz.bazstr)
+  > 
+  > # "not fromlist" and NOT "if '.' in name" case
+  > import sub1    # relative
+  > buf.append('import sub1: %s' % sub1.sub1str)
+  > import extroot # absolute
+  > buf.append('import extroot: %s' % extroot.extrootstr)
+  > 
+  > # NOT "not fromlist" and NOT "level != -1" case
+  > from bar import barstr         # relative
+  > buf.append('from bar import barstr: %s' % barstr)
+  > from extroot.bar import barstr # absolute
+  > buf.append('from extroot.bar import barstr: %s' % barstr)
+  > EOF
+  $ cat > extroot/bar.py <<EOF
+  > barstr = 'this is extroot.bar.barstr'
+  > EOF
+  $ mkdir extroot/sub1
+  $ cat > extroot/sub1/__init__.py <<EOF
+  > sub1str = 'this is extroot.sub1.sub1str'
+  > EOF
+  $ cat > extroot/sub1/baz.py <<EOF
+  > bazstr = 'this is extroot.sub1.baz.bazstr'
+  > EOF
+  $ hg --config extensions.extroot=extroot tip --template '{rev}\n'
+  (extroot) from bar import *: this is extroot.bar.barstr
+  (extroot) from extroot.bar import *: this is extroot.bar.barstr
+  (extroot) import sub1.baz: this is extroot.sub1.baz.bazstr
+  (extroot) import extroot.sub1.baz: this is extroot.sub1.baz.bazstr
+  (extroot) import sub1: this is extroot.sub1.sub1str
+  (extroot) import extroot: this is extroot.extrootstr
+  (extroot) from bar import barstr: this is extroot.bar.barstr
+  (extroot) from extroot.bar import barstr: this is extroot.bar.barstr
+  (extroot) import bar in func(): this is extroot.bar.barstr
+  (extroot) import extroot.bar in func(): this is extroot.bar.barstr
+  0
+
   $ cd ..
 
 hide outer repo