| 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
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
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
> 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.
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.
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.
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.
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).
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)