Patchwork [1,of,3] py3: conditionalize httplib import

login
register
mail settings
Submitter Pulkit Goyal
Date June 30, 2016, 9:47 a.m.
Message ID <752f11e6e60f6214335d.1467280038@pulkit-goyal>
Download mbox | patch
Permalink /patch/15692/
State Accepted
Headers show

Comments

Pulkit Goyal - June 30, 2016, 9:47 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1467109913 -19800
#      Tue Jun 28 16:01:53 2016 +0530
# Node ID 752f11e6e60f6214335dd761e6bb045d63747bbb
# Parent  f251c4c4f77bd2be42ff69b71c74c2778febac90
py3: conditionalize httplib import

The httplib library is renamed to http.client in python 3. So the
import is conditionalized and a test is added in check-code to warn
to use util.httplib
Martijn Pieters - July 1, 2016, 1:48 p.m.
On 30 June 2016 at 10:47, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1467109913 -19800
> #      Tue Jun 28 16:01:53 2016 +0530
> # Node ID 752f11e6e60f6214335dd761e6bb045d63747bbb
> # Parent  f251c4c4f77bd2be42ff69b71c74c2778febac90
> py3: conditionalize httplib import
>
> The httplib library is renamed to http.client in python 3. So the
> import is conditionalized and a test is added in check-code to warn
> to use util.httplib
>

This one looks a-okay to me.


> diff -r f251c4c4f77b -r 752f11e6e60f contrib/check-code.py
> --- a/contrib/check-code.py     Mon Jun 27 19:10:30 2016 +0530
> +++ b/contrib/check-code.py     Tue Jun 28 16:01:53 2016 +0530
> @@ -330,6 +330,7 @@
>      (r'^import xmlrpclib', "don't use xmlrpclib, use util.xmlrpclib"),
>      (r'^import cPickle', "don't use cPickle, use util.pickle"),
>      (r'^import pickle', "don't use pickle, use util.pickle"),
> +    (r'^import httplib', "don't use httplib, use util.httplib"),
>      (r'\.next\(\)', "don't use .next(), use next(...)"),
>
>      # rules depending on implementation of repquote()
> diff -r f251c4c4f77b -r 752f11e6e60f mercurial/httppeer.py
> --- a/mercurial/httppeer.py     Mon Jun 27 19:10:30 2016 +0530
> +++ b/mercurial/httppeer.py     Tue Jun 28 16:01:53 2016 +0530
> @@ -9,7 +9,6 @@
>  from __future__ import absolute_import
>
>  import errno
> -import httplib
>  import os
>  import socket
>  import tempfile
> @@ -27,6 +26,7 @@
>      wireproto,
>  )
>
> +httplib = util.httplib
>  urlerr = util.urlerr
>  urlreq = util.urlreq
>
> diff -r f251c4c4f77b -r 752f11e6e60f mercurial/keepalive.py
> --- a/mercurial/keepalive.py    Mon Jun 27 19:10:30 2016 +0530
> +++ b/mercurial/keepalive.py    Tue Jun 28 16:01:53 2016 +0530
> @@ -111,7 +111,6 @@
>
>  import errno
>  import hashlib
> -import httplib
>  import socket
>  import sys
>  import thread
> @@ -120,6 +119,7 @@
>      util,
>  )
>
> +httplib = util.httplib
>  urlerr = util.urlerr
>  urlreq = util.urlreq
>
> diff -r f251c4c4f77b -r 752f11e6e60f mercurial/pycompat.py
> --- a/mercurial/pycompat.py     Mon Jun 27 19:10:30 2016 +0530
> +++ b/mercurial/pycompat.py     Tue Jun 28 16:01:53 2016 +0530
> @@ -18,6 +18,13 @@
>      pickle.dumps # silence pyflakes
>
>  try:
> +    import httplib
> +    httplib.HTTPException
> +except ImportError:
> +    import http.client as httplib
> +    httplib.HTTPException
> +
> +try:
>      import SocketServer as socketserver
>      socketserver.ThreadingMixIn
>  except ImportError:
> diff -r f251c4c4f77b -r 752f11e6e60f mercurial/url.py
> --- a/mercurial/url.py  Mon Jun 27 19:10:30 2016 +0530
> +++ b/mercurial/url.py  Tue Jun 28 16:01:53 2016 +0530
> @@ -10,7 +10,6 @@
>  from __future__ import absolute_import
>
>  import base64
> -import httplib
>  import os
>  import socket
>
> @@ -22,8 +21,9 @@
>      sslutil,
>      util,
>  )
> +
> +httplib = util.httplib
>  stringio = util.stringio
> -
>  urlerr = util.urlerr
>  urlreq = util.urlreq
>
> diff -r f251c4c4f77b -r 752f11e6e60f mercurial/util.py
> --- a/mercurial/util.py Mon Jun 27 19:10:30 2016 +0530
> +++ b/mercurial/util.py Tue Jun 28 16:01:53 2016 +0530
> @@ -47,6 +47,7 @@
>
>  for attr in (
>      'empty',
> +    'httplib',
>      'pickle',
>      'queue',
>      'urlerr',
> diff -r f251c4c4f77b -r 752f11e6e60f tests/get-with-headers.py
> --- a/tests/get-with-headers.py Mon Jun 27 19:10:30 2016 +0530
> +++ b/tests/get-with-headers.py Tue Jun 28 16:01:53 2016 +0530
> @@ -5,11 +5,16 @@
>
>  from __future__ import absolute_import, print_function
>
> -import httplib
>  import json
>  import os
>  import sys
>
> +from mercurial import (
> +    util,
> +)
> +
> +httplib = util.httplib
> +
>  try:
>      import msvcrt
>      msvcrt.setmode(sys.stdout.fileno(), os.O_BINARY)
> diff -r f251c4c4f77b -r 752f11e6e60f tests/test-check-py3-compat.t
> --- a/tests/test-check-py3-compat.t     Mon Jun 27 19:10:30 2016 +0530
> +++ b/tests/test-check-py3-compat.t     Tue Jun 28 16:01:53 2016 +0530
> @@ -61,7 +61,7 @@
>    hgext/largefiles/lfutil.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>    hgext/largefiles/localstore.py: error importing module: <SystemError> Parent module 'hgext.largefiles' not loaded, cannot perform relative import (line *) (glob)
>    hgext/largefiles/overrides.py: error importing module: <SyntaxError> invalid syntax (archival.py, line *) (line *) (glob)
> -  hgext/largefiles/proto.py: error importing: <ImportError> No module named 'httplib' (error at httppeer.py:*) (glob)
> +  hgext/largefiles/proto.py: error importing: <SyntaxError> invalid syntax (bundle2.py, line *) (error at httppeer.py:*) (glob)
>    hgext/largefiles/remotestore.py: error importing: <SyntaxError> invalid syntax (bundle*.py, line *) (error at wireproto.py:*) (glob)
>    hgext/largefiles/reposetup.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>    hgext/largefiles/storefactory.py: error importing: <SyntaxError> invalid syntax (bundle2.py, line *) (error at bundlerepo.py:*) (glob)
> @@ -114,8 +114,8 @@
>    mercurial/hgweb/wsgicgi.py: error importing module: <SystemError> Parent module 'mercurial.hgweb' not loaded, cannot perform relative import (line *) (glob)
>    mercurial/hook.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>    mercurial/httpconnection.py: error importing: <ImportError> No module named 'rfc822' (error at __init__.py:*) (glob)
> -  mercurial/httppeer.py: error importing module: <ImportError> No module named 'httplib' (line *) (glob)
> -  mercurial/keepalive.py: error importing module: <ImportError> No module named 'httplib' (line *) (glob)
> +  mercurial/httppeer.py: error importing module: <SyntaxError> invalid syntax (bundle2.py, line *) (line *) (glob)
> +  mercurial/keepalive.py: error importing module: <ImportError> No module named 'thread' (line *) (glob)
>    mercurial/localrepo.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>    mercurial/mail.py: error importing module: <AttributeError> module 'email' has no attribute 'Header' (line *) (glob)
>    mercurial/manifest.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
> @@ -141,7 +141,7 @@
>    mercurial/templater.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>    mercurial/ui.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>    mercurial/unionrepo.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
> -  mercurial/url.py: error importing module: <ImportError> No module named 'httplib' (line *) (glob)
> +  mercurial/url.py: error importing: <ImportError> No module named 'rfc822' (error at __init__.py:*) (glob)
>    mercurial/verify.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>    mercurial/win*.py: error importing module: <ImportError> No module named 'msvcrt' (line *) (glob)
>    mercurial/windows.py: error importing module: <ImportError> No module named '_winreg' (line *) (glob)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - July 1, 2016, 2:37 p.m.
On Thu, 30 Jun 2016 15:17:18 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1467109913 -19800
> #      Tue Jun 28 16:01:53 2016 +0530
> # Node ID 752f11e6e60f6214335dd761e6bb045d63747bbb
> # Parent  f251c4c4f77bd2be42ff69b71c74c2778febac90
> py3: conditionalize httplib import

Queued the first patch per Martijn's review, thanks.

> --- a/mercurial/pycompat.py	Mon Jun 27 19:10:30 2016 +0530
> +++ b/mercurial/pycompat.py	Tue Jun 28 16:01:53 2016 +0530
> @@ -18,6 +18,13 @@
>      pickle.dumps # silence pyflakes
>  
>  try:
> +    import httplib
> +    httplib.HTTPException
> +except ImportError:
> +    import http.client as httplib
> +    httplib.HTTPException

The util module is getting more and more demandimport unfriendly. I'd rather
make them test the Python version than try importing. Any idea?

if sys.version_info[0] < 3:
    import httplib
else:
    import http.client as httplib
Pulkit Goyal - July 1, 2016, 2:43 p.m.
> if sys.version_info[0] < 3:
>     import httplib
> else:
>     import http.client as httplib

Yeah we can have just one if else and fit all of them there. I will
send a patch related to this, if its okay.
Yuya Nishihara - July 3, 2016, 10:54 a.m.
On Fri, 1 Jul 2016 20:13:11 +0530, Pulkit Goyal wrote:
> > if sys.version_info[0] < 3:
> >     import httplib
> > else:
> >     import http.client as httplib  
> 
> Yeah we can have just one if else and fit all of them there. I will
> send a patch related to this, if its okay.

There seems no -1, let's do that. You might have to work around
test-check-pyflakes.t, which would report unused imports.
Martijn Pieters - July 4, 2016, 11:44 a.m.
On 3 July 2016 at 11:54, Yuya Nishihara <yuya@tcha.org> wrote:
> On Fri, 1 Jul 2016 20:13:11 +0530, Pulkit Goyal wrote:
>> > if sys.version_info[0] < 3:
>> >     import httplib
>> > else:
>> >     import http.client as httplib
>>
>> Yeah we can have just one if else and fit all of them there. I will
>> send a patch related to this, if its okay.
>
> There seems no -1, let's do that. You might have to work around
> test-check-pyflakes.t, which would report unused imports.

Is there any chance we could switch to using flake8 here instead?
flake8 *uses* pyflakes but adds `# noqa: code, code` support to ignore
specific linting errors on specific lines. See
http://flake8.pycqa.org/en/latest/user/ignoring-errors.html.

Flake8 is also vastly more flexible and our check-code.py could
possibly be converted into a plugin for flake8.
Yuya Nishihara - July 4, 2016, 1:14 p.m.
On Mon, 4 Jul 2016 12:44:41 +0100, Martijn Pieters wrote:
> On 3 July 2016 at 11:54, Yuya Nishihara <yuya@tcha.org> wrote:
> > On Fri, 1 Jul 2016 20:13:11 +0530, Pulkit Goyal wrote:  
> >> > if sys.version_info[0] < 3:
> >> >     import httplib
> >> > else:
> >> >     import http.client as httplib  
> >>
> >> Yeah we can have just one if else and fit all of them there. I will
> >> send a patch related to this, if its okay.  
> >
> > There seems no -1, let's do that. You might have to work around
> > test-check-pyflakes.t, which would report unused imports.  
> 
> Is there any chance we could switch to using flake8 here instead?
> flake8 *uses* pyflakes but adds `# noqa: code, code` support to ignore
> specific linting errors on specific lines. See
> http://flake8.pycqa.org/en/latest/user/ignoring-errors.html.

Sounds nice. We can get rid of the custom filterpyflakes.py script.

> Flake8 is also vastly more flexible and our check-code.py could
> possibly be converted into a plugin for flake8.
Gregory Szorc - July 4, 2016, 5:14 p.m.
On Mon, Jul 4, 2016 at 4:44 AM, Martijn Pieters <mj@zopatista.com> wrote:

> On 3 July 2016 at 11:54, Yuya Nishihara <yuya@tcha.org> wrote:
> > On Fri, 1 Jul 2016 20:13:11 +0530, Pulkit Goyal wrote:
> >> > if sys.version_info[0] < 3:
> >> >     import httplib
> >> > else:
> >> >     import http.client as httplib
> >>
> >> Yeah we can have just one if else and fit all of them there. I will
> >> send a patch related to this, if its okay.
> >
> > There seems no -1, let's do that. You might have to work around
> > test-check-pyflakes.t, which would report unused imports.
>
> Is there any chance we could switch to using flake8 here instead?
> flake8 *uses* pyflakes but adds `# noqa: code, code` support to ignore
> specific linting errors on specific lines. See
> http://flake8.pycqa.org/en/latest/user/ignoring-errors.html.
>
> Flake8 is also vastly more flexible and our check-code.py could
> possibly be converted into a plugin for flake8.


Keep in mind that flake8 wraps multiple linting packages. If we go with
flake8, we may need to customize it to only run the linting packages we
care about (possibly just pyflakes).
Martijn Pieters - July 4, 2016, 7:13 p.m.
On 4 July 2016 at 18:14, Gregory Szorc <gregory.szorc@gmail.com> wrote:
>> Is there any chance we could switch to using flake8 here instead?
>> flake8 *uses* pyflakes but adds `# noqa: code, code` support to ignore
>> specific linting errors on specific lines. See
>> http://flake8.pycqa.org/en/latest/user/ignoring-errors.html.
>>
>> Flake8 is also vastly more flexible and our check-code.py could
>> possibly be converted into a plugin for flake8.
>
>
> Keep in mind that flake8 wraps multiple linting packages. If we go with
> flake8, we may need to customize it to only run the linting packages we care
> about (possibly just pyflakes).

Absolutely. I was thinking about using flake8 with only the same
checks as are done now, but with the added flexibility of flake8's
configurability.

I've filed a feature request for flake8 to add even more flexibility:
switching off specific tests per file rather than per line or per
project. See https://gitlab.com/pycqa/flake8/issues/156

Patch

diff -r f251c4c4f77b -r 752f11e6e60f contrib/check-code.py
--- a/contrib/check-code.py	Mon Jun 27 19:10:30 2016 +0530
+++ b/contrib/check-code.py	Tue Jun 28 16:01:53 2016 +0530
@@ -330,6 +330,7 @@ 
     (r'^import xmlrpclib', "don't use xmlrpclib, use util.xmlrpclib"),
     (r'^import cPickle', "don't use cPickle, use util.pickle"),
     (r'^import pickle', "don't use pickle, use util.pickle"),
+    (r'^import httplib', "don't use httplib, use util.httplib"),
     (r'\.next\(\)', "don't use .next(), use next(...)"),
 
     # rules depending on implementation of repquote()
diff -r f251c4c4f77b -r 752f11e6e60f mercurial/httppeer.py
--- a/mercurial/httppeer.py	Mon Jun 27 19:10:30 2016 +0530
+++ b/mercurial/httppeer.py	Tue Jun 28 16:01:53 2016 +0530
@@ -9,7 +9,6 @@ 
 from __future__ import absolute_import
 
 import errno
-import httplib
 import os
 import socket
 import tempfile
@@ -27,6 +26,7 @@ 
     wireproto,
 )
 
+httplib = util.httplib
 urlerr = util.urlerr
 urlreq = util.urlreq
 
diff -r f251c4c4f77b -r 752f11e6e60f mercurial/keepalive.py
--- a/mercurial/keepalive.py	Mon Jun 27 19:10:30 2016 +0530
+++ b/mercurial/keepalive.py	Tue Jun 28 16:01:53 2016 +0530
@@ -111,7 +111,6 @@ 
 
 import errno
 import hashlib
-import httplib
 import socket
 import sys
 import thread
@@ -120,6 +119,7 @@ 
     util,
 )
 
+httplib = util.httplib
 urlerr = util.urlerr
 urlreq = util.urlreq
 
diff -r f251c4c4f77b -r 752f11e6e60f mercurial/pycompat.py
--- a/mercurial/pycompat.py	Mon Jun 27 19:10:30 2016 +0530
+++ b/mercurial/pycompat.py	Tue Jun 28 16:01:53 2016 +0530
@@ -18,6 +18,13 @@ 
     pickle.dumps # silence pyflakes
 
 try:
+    import httplib
+    httplib.HTTPException
+except ImportError:
+    import http.client as httplib
+    httplib.HTTPException
+
+try:
     import SocketServer as socketserver
     socketserver.ThreadingMixIn
 except ImportError:
diff -r f251c4c4f77b -r 752f11e6e60f mercurial/url.py
--- a/mercurial/url.py	Mon Jun 27 19:10:30 2016 +0530
+++ b/mercurial/url.py	Tue Jun 28 16:01:53 2016 +0530
@@ -10,7 +10,6 @@ 
 from __future__ import absolute_import
 
 import base64
-import httplib
 import os
 import socket
 
@@ -22,8 +21,9 @@ 
     sslutil,
     util,
 )
+
+httplib = util.httplib
 stringio = util.stringio
-
 urlerr = util.urlerr
 urlreq = util.urlreq
 
diff -r f251c4c4f77b -r 752f11e6e60f mercurial/util.py
--- a/mercurial/util.py	Mon Jun 27 19:10:30 2016 +0530
+++ b/mercurial/util.py	Tue Jun 28 16:01:53 2016 +0530
@@ -47,6 +47,7 @@ 
 
 for attr in (
     'empty',
+    'httplib',
     'pickle',
     'queue',
     'urlerr',
diff -r f251c4c4f77b -r 752f11e6e60f tests/get-with-headers.py
--- a/tests/get-with-headers.py	Mon Jun 27 19:10:30 2016 +0530
+++ b/tests/get-with-headers.py	Tue Jun 28 16:01:53 2016 +0530
@@ -5,11 +5,16 @@ 
 
 from __future__ import absolute_import, print_function
 
-import httplib
 import json
 import os
 import sys
 
+from mercurial import (
+    util,
+)
+
+httplib = util.httplib
+
 try:
     import msvcrt
     msvcrt.setmode(sys.stdout.fileno(), os.O_BINARY)
diff -r f251c4c4f77b -r 752f11e6e60f tests/test-check-py3-compat.t
--- a/tests/test-check-py3-compat.t	Mon Jun 27 19:10:30 2016 +0530
+++ b/tests/test-check-py3-compat.t	Tue Jun 28 16:01:53 2016 +0530
@@ -61,7 +61,7 @@ 
   hgext/largefiles/lfutil.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
   hgext/largefiles/localstore.py: error importing module: <SystemError> Parent module 'hgext.largefiles' not loaded, cannot perform relative import (line *) (glob)
   hgext/largefiles/overrides.py: error importing module: <SyntaxError> invalid syntax (archival.py, line *) (line *) (glob)
-  hgext/largefiles/proto.py: error importing: <ImportError> No module named 'httplib' (error at httppeer.py:*) (glob)
+  hgext/largefiles/proto.py: error importing: <SyntaxError> invalid syntax (bundle2.py, line *) (error at httppeer.py:*) (glob)
   hgext/largefiles/remotestore.py: error importing: <SyntaxError> invalid syntax (bundle*.py, line *) (error at wireproto.py:*) (glob)
   hgext/largefiles/reposetup.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
   hgext/largefiles/storefactory.py: error importing: <SyntaxError> invalid syntax (bundle2.py, line *) (error at bundlerepo.py:*) (glob)
@@ -114,8 +114,8 @@ 
   mercurial/hgweb/wsgicgi.py: error importing module: <SystemError> Parent module 'mercurial.hgweb' not loaded, cannot perform relative import (line *) (glob)
   mercurial/hook.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
   mercurial/httpconnection.py: error importing: <ImportError> No module named 'rfc822' (error at __init__.py:*) (glob)
-  mercurial/httppeer.py: error importing module: <ImportError> No module named 'httplib' (line *) (glob)
-  mercurial/keepalive.py: error importing module: <ImportError> No module named 'httplib' (line *) (glob)
+  mercurial/httppeer.py: error importing module: <SyntaxError> invalid syntax (bundle2.py, line *) (line *) (glob)
+  mercurial/keepalive.py: error importing module: <ImportError> No module named 'thread' (line *) (glob)
   mercurial/localrepo.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
   mercurial/mail.py: error importing module: <AttributeError> module 'email' has no attribute 'Header' (line *) (glob)
   mercurial/manifest.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
@@ -141,7 +141,7 @@ 
   mercurial/templater.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
   mercurial/ui.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
   mercurial/unionrepo.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
-  mercurial/url.py: error importing module: <ImportError> No module named 'httplib' (line *) (glob)
+  mercurial/url.py: error importing: <ImportError> No module named 'rfc822' (error at __init__.py:*) (glob)
   mercurial/verify.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
   mercurial/win*.py: error importing module: <ImportError> No module named 'msvcrt' (line *) (glob)
   mercurial/windows.py: error importing module: <ImportError> No module named '_winreg' (line *) (glob)