Patchwork [1,of,4,RFC] pycompat: delay loading modules registered to stub

login
register
mail settings
Submitter Yuya Nishihara
Date Aug. 14, 2016, 9:33 a.m.
Message ID <131cc484d7fe95d07f67.1471167225@mimosa>
Download mbox | patch
Permalink /patch/16276/
State Changes Requested
Headers show

Comments

Yuya Nishihara - Aug. 14, 2016, 9:33 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1471153584 -32400
#      Sun Aug 14 14:46:24 2016 +0900
# Node ID 131cc484d7fe95d07f677c16dbb0f506847e1d38
# Parent  0cb2d4db308b97e8fe7faa8d45a47b228037f230
pycompat: delay loading modules registered to stub

New _pycompatstub is compatible with our demandimporter. try-except is
replaced by version comparison because ImportError will no longer be raised
immediately.
Gregory Szorc - Aug. 14, 2016, 4:21 p.m.
On Sun, Aug 14, 2016 at 2:33 AM, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1471153584 -32400
> #      Sun Aug 14 14:46:24 2016 +0900
> # Node ID 131cc484d7fe95d07f677c16dbb0f506847e1d38
> # Parent  0cb2d4db308b97e8fe7faa8d45a47b228037f230
> pycompat: delay loading modules registered to stub
>
> New _pycompatstub is compatible with our demandimporter. try-except is
> replaced by version comparison because ImportError will no longer be raised
> immediately.
>

Nit: I think you mean "incompatible." This patch looks good otherwise.


>
> diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
> --- a/mercurial/pycompat.py
> +++ b/mercurial/pycompat.py
> @@ -34,30 +34,32 @@ empty = _queue.Empty
>  queue = _queue.Queue
>
>  class _pycompatstub(object):
> -    pass
> -
> -def _alias(alias, origin, items):
> -    """ populate a _pycompatstub
> +    def __init__(self):
> +        self._aliases = {}
>
> -    copies items from origin to alias
> -    """
> -    for item in items:
> +    def _registeraliases(self, origin, items):
> +        """Add items that will be populated at the first access"""
> +        self._aliases.update((item.replace('_', '').lower(), (origin,
> item))
> +                             for item in items)
> +
> +    def __getattr__(self, name):
>          try:
> -            lcase = item.replace('_', '').lower()
> -            setattr(alias, lcase, getattr(origin, item))
> -        except AttributeError:
> -            pass
> +            origin, item = self._aliases[name]
> +        except KeyError:
> +            raise AttributeError(name)
> +        self.__dict__[name] = obj = getattr(origin, item)
> +        return obj
>
>  httpserver = _pycompatstub()
>  urlreq = _pycompatstub()
>  urlerr = _pycompatstub()
> -try:
> +if sys.version_info[0] < 3:
>      import BaseHTTPServer
>      import CGIHTTPServer
>      import SimpleHTTPServer
>      import urllib2
>      import urllib
> -    _alias(urlreq, urllib, (
> +    urlreq._registeraliases(urllib, (
>          "addclosehook",
>          "addinfourl",
>          "ftpwrapper",
> @@ -71,7 +73,7 @@ try:
>          "url2pathname",
>          "urlencode",
>      ))
> -    _alias(urlreq, urllib2, (
> +    urlreq._registeraliases(urllib2, (
>          "AbstractHTTPHandler",
>          "BaseHandler",
>          "build_opener",
> @@ -87,24 +89,24 @@ try:
>          "Request",
>          "urlopen",
>      ))
> -    _alias(urlerr, urllib2, (
> +    urlerr._registeraliases(urllib2, (
>          "HTTPError",
>          "URLError",
>      ))
> -    _alias(httpserver, BaseHTTPServer, (
> +    httpserver._registeraliases(BaseHTTPServer, (
>          "HTTPServer",
>          "BaseHTTPRequestHandler",
>      ))
> -    _alias(httpserver, SimpleHTTPServer, (
> +    httpserver._registeraliases(SimpleHTTPServer, (
>          "SimpleHTTPRequestHandler",
>      ))
> -    _alias(httpserver, CGIHTTPServer, (
> +    httpserver._registeraliases(CGIHTTPServer, (
>          "CGIHTTPRequestHandler",
>      ))
>
> -except ImportError:
> +else:
>      import urllib.request
> -    _alias(urlreq, urllib.request, (
> +    urlreq._registeraliases(urllib.request, (
>          "AbstractHTTPHandler",
>          "addclosehook",
>          "addinfourl",
> @@ -132,12 +134,12 @@ except ImportError:
>          "urlopen",
>      ))
>      import urllib.error
> -    _alias(urlerr, urllib.error, (
> +    urlerr._registeraliases(urllib.error, (
>          "HTTPError",
>          "URLError",
>      ))
>      import http.server
> -    _alias(httpserver, http.server, (
> +    httpserver._registeraliases(http.server, (
>          "HTTPServer",
>          "BaseHTTPRequestHandler",
>          "SimpleHTTPRequestHandler",
> diff --git a/tests/test-check-py3-compat.t b/tests/test-check-py3-compat.t
> --- a/tests/test-check-py3-compat.t
> +++ b/tests/test-check-py3-compat.t
> @@ -122,24 +122,22 @@
>    mercurial/hook.py: error importing: <TypeError> str expected, not bytes
> (error at i18n.py:*) (glob)
>    mercurial/httpconnection.py: error importing: <TypeError> str expected,
> not bytes (error at i18n.py:*) (glob)
>    mercurial/httppeer.py: error importing: <TypeError> str expected, not
> bytes (error at i18n.py:*) (glob)
> -  mercurial/keepalive.py: error importing: <TypeError> getattr():
> attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/localrepo.py: error importing: <TypeError> getattr():
> attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/lock.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/mail.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/manifest.py: error importing: <TypeError> getattr():
> attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/match.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/mdiff.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/merge.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/minirst.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/namespaces.py: error importing: <TypeError> getattr():
> attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/obsolete.py: error importing: <TypeError> getattr():
> attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/patch.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/pathutil.py: error importing: <TypeError> getattr():
> attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/peer.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/pure/mpatch.py: error importing: <TypeError> getattr():
> attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/pure/parsers.py: error importing: <TypeError> getattr():
> attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/pushkey.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> -  mercurial/pvec.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at pycompat.py:*) (glob)
> +  mercurial/keepalive.py: error importing: <TypeError> getattr():
> attribute name must be string (error at util.py:*) (glob)
> +  mercurial/localrepo.py: error importing: <TypeError> getattr():
> attribute name must be string (error at util.py:*) (glob)
> +  mercurial/lock.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/mail.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/manifest.py: error importing: <TypeError> getattr():
> attribute name must be string (error at util.py:*) (glob)
> +  mercurial/match.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/mdiff.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/merge.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/minirst.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/namespaces.py: error importing: <TypeError> getattr():
> attribute name must be string (error at util.py:*) (glob)
> +  mercurial/obsolete.py: error importing: <TypeError> getattr():
> attribute name must be string (error at util.py:*) (glob)
> +  mercurial/patch.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/pathutil.py: error importing: <TypeError> getattr():
> attribute name must be string (error at util.py:*) (glob)
> +  mercurial/peer.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/pushkey.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
> +  mercurial/pvec.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
>    mercurial/registrar.py: error importing: <TypeError> getattr():
> attribute name must be string (error at util.py:*) (glob)
>    mercurial/repair.py: error importing: <TypeError> getattr(): attribute
> name must be string (error at util.py:*) (glob)
>    mercurial/repoview.py: error importing: <TypeError> getattr():
> attribute name must be string (error at util.py:*) (glob)
> @@ -168,6 +166,6 @@
>    mercurial/verify.py: error importing: <TypeError> attribute name must
> be string, not 'bytes' (error at mdiff.py:*) (glob)
>    mercurial/win32.py: error importing module: <ImportError> No module
> named 'msvcrt' (line *) (glob)
>    mercurial/windows.py: error importing module: <ImportError> No module
> named 'msvcrt' (line *) (glob)
> -  mercurial/wireproto.py: error importing module: <TypeError> a
> bytes-like object is required, not 'str' (line *) (glob)
> +  mercurial/wireproto.py: error importing module: <TypeError> unorderable
> types: str() >= tuple() (line *) (glob)
>
>  #endif
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - Aug. 14, 2016, 10:02 p.m.
On Sun, 14 Aug 2016 09:21:07 -0700, Gregory Szorc wrote:
> On Sun, Aug 14, 2016 at 2:33 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1471153584 -32400
> > #      Sun Aug 14 14:46:24 2016 +0900
> > # Node ID 131cc484d7fe95d07f677c16dbb0f506847e1d38
> > # Parent  0cb2d4db308b97e8fe7faa8d45a47b228037f230
> > pycompat: delay loading modules registered to stub
> >
> > New _pycompatstub is compatible with our demandimporter. try-except is
> > replaced by version comparison because ImportError will no longer be raised
> > immediately.
> 
> Nit: I think you mean "incompatible." This patch looks good otherwise.

I wanted to say something like "new _pycompatstub works well with our
demandimporter." Can you update the commit message in flight?
timeless - Aug. 15, 2016, 4:48 a.m.
"Replacement _pycompatstub designed to be compatible with our demandimporter"

On Sun, Aug 14, 2016 at 6:02 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Sun, 14 Aug 2016 09:21:07 -0700, Gregory Szorc wrote:
>> On Sun, Aug 14, 2016 at 2:33 AM, Yuya Nishihara <yuya@tcha.org> wrote:
>> > # HG changeset patch
>> > # User Yuya Nishihara <yuya@tcha.org>
>> > # Date 1471153584 -32400
>> > #      Sun Aug 14 14:46:24 2016 +0900
>> > # Node ID 131cc484d7fe95d07f677c16dbb0f506847e1d38
>> > # Parent  0cb2d4db308b97e8fe7faa8d45a47b228037f230
>> > pycompat: delay loading modules registered to stub
>> >
>> > New _pycompatstub is compatible with our demandimporter. try-except is
>> > replaced by version comparison because ImportError will no longer be raised
>> > immediately.
>>
>> Nit: I think you mean "incompatible." This patch looks good otherwise.
>
> I wanted to say something like "new _pycompatstub works well with our
> demandimporter." Can you update the commit message in flight?
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - Aug. 15, 2016, 9:53 a.m.
On Mon, 15 Aug 2016 00:48:32 -0400, timeless wrote:
> "Replacement _pycompatstub designed to be compatible with our demandimporter"

Thanks. I'll resend the first patch.

Patch

diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -34,30 +34,32 @@  empty = _queue.Empty
 queue = _queue.Queue
 
 class _pycompatstub(object):
-    pass
-
-def _alias(alias, origin, items):
-    """ populate a _pycompatstub
+    def __init__(self):
+        self._aliases = {}
 
-    copies items from origin to alias
-    """
-    for item in items:
+    def _registeraliases(self, origin, items):
+        """Add items that will be populated at the first access"""
+        self._aliases.update((item.replace('_', '').lower(), (origin, item))
+                             for item in items)
+
+    def __getattr__(self, name):
         try:
-            lcase = item.replace('_', '').lower()
-            setattr(alias, lcase, getattr(origin, item))
-        except AttributeError:
-            pass
+            origin, item = self._aliases[name]
+        except KeyError:
+            raise AttributeError(name)
+        self.__dict__[name] = obj = getattr(origin, item)
+        return obj
 
 httpserver = _pycompatstub()
 urlreq = _pycompatstub()
 urlerr = _pycompatstub()
-try:
+if sys.version_info[0] < 3:
     import BaseHTTPServer
     import CGIHTTPServer
     import SimpleHTTPServer
     import urllib2
     import urllib
-    _alias(urlreq, urllib, (
+    urlreq._registeraliases(urllib, (
         "addclosehook",
         "addinfourl",
         "ftpwrapper",
@@ -71,7 +73,7 @@  try:
         "url2pathname",
         "urlencode",
     ))
-    _alias(urlreq, urllib2, (
+    urlreq._registeraliases(urllib2, (
         "AbstractHTTPHandler",
         "BaseHandler",
         "build_opener",
@@ -87,24 +89,24 @@  try:
         "Request",
         "urlopen",
     ))
-    _alias(urlerr, urllib2, (
+    urlerr._registeraliases(urllib2, (
         "HTTPError",
         "URLError",
     ))
-    _alias(httpserver, BaseHTTPServer, (
+    httpserver._registeraliases(BaseHTTPServer, (
         "HTTPServer",
         "BaseHTTPRequestHandler",
     ))
-    _alias(httpserver, SimpleHTTPServer, (
+    httpserver._registeraliases(SimpleHTTPServer, (
         "SimpleHTTPRequestHandler",
     ))
-    _alias(httpserver, CGIHTTPServer, (
+    httpserver._registeraliases(CGIHTTPServer, (
         "CGIHTTPRequestHandler",
     ))
 
-except ImportError:
+else:
     import urllib.request
-    _alias(urlreq, urllib.request, (
+    urlreq._registeraliases(urllib.request, (
         "AbstractHTTPHandler",
         "addclosehook",
         "addinfourl",
@@ -132,12 +134,12 @@  except ImportError:
         "urlopen",
     ))
     import urllib.error
-    _alias(urlerr, urllib.error, (
+    urlerr._registeraliases(urllib.error, (
         "HTTPError",
         "URLError",
     ))
     import http.server
-    _alias(httpserver, http.server, (
+    httpserver._registeraliases(http.server, (
         "HTTPServer",
         "BaseHTTPRequestHandler",
         "SimpleHTTPRequestHandler",
diff --git a/tests/test-check-py3-compat.t b/tests/test-check-py3-compat.t
--- a/tests/test-check-py3-compat.t
+++ b/tests/test-check-py3-compat.t
@@ -122,24 +122,22 @@ 
   mercurial/hook.py: error importing: <TypeError> str expected, not bytes (error at i18n.py:*) (glob)
   mercurial/httpconnection.py: error importing: <TypeError> str expected, not bytes (error at i18n.py:*) (glob)
   mercurial/httppeer.py: error importing: <TypeError> str expected, not bytes (error at i18n.py:*) (glob)
-  mercurial/keepalive.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/localrepo.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/lock.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/mail.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/manifest.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/match.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/mdiff.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/merge.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/minirst.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/namespaces.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/obsolete.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/patch.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/pathutil.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/peer.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/pure/mpatch.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/pure/parsers.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/pushkey.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/pvec.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
+  mercurial/keepalive.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/localrepo.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/lock.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/mail.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/manifest.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/match.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/mdiff.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/merge.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/minirst.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/namespaces.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/obsolete.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/patch.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/pathutil.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/peer.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/pushkey.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
+  mercurial/pvec.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/registrar.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/repair.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/repoview.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
@@ -168,6 +166,6 @@ 
   mercurial/verify.py: error importing: <TypeError> attribute name must be string, not 'bytes' (error at mdiff.py:*) (glob)
   mercurial/win32.py: error importing module: <ImportError> No module named 'msvcrt' (line *) (glob)
   mercurial/windows.py: error importing module: <ImportError> No module named 'msvcrt' (line *) (glob)
-  mercurial/wireproto.py: error importing module: <TypeError> a bytes-like object is required, not 'str' (line *) (glob)
+  mercurial/wireproto.py: error importing module: <TypeError> unorderable types: str() >= tuple() (line *) (glob)
 
 #endif