Patchwork util: use string.hexdigits instead of defining it ourselves

login
register
mail settings
Submitter Augie Fackler
Date Oct. 7, 2016, 12:58 p.m.
Message ID <2ef5a0c25a084f8793e0.1475845112@augie-macbookair2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/16890/
State Accepted
Headers show

Comments

Augie Fackler - Oct. 7, 2016, 12:58 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1475845103 14400
#      Fri Oct 07 08:58:23 2016 -0400
# Node ID 2ef5a0c25a084f8793e0dfe656ae6401fd2c1bd2
# Parent  f3a2125968377fb1d4b9ea3f4917260d5aca3536
util: use string.hexdigits instead of defining it ourselves

This resolves some Python 3 weirdness.
Pulkit Goyal - Oct. 7, 2016, 1:13 p.m.
On Oct 7, 2016 2:58 PM, "Augie Fackler" <raf@durin42.com> wrote:
>
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1475845103 14400
> #      Fri Oct 07 08:58:23 2016 -0400
> # Node ID 2ef5a0c25a084f8793e0dfe656ae6401fd2c1bd2
> # Parent  f3a2125968377fb1d4b9ea3f4917260d5aca3536
> util: use string.hexdigits instead of defining it ourselves
>
> This resolves some Python 3 weirdness.
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -28,6 +28,7 @@ import re as remod
>  import shutil
>  import signal
>  import socket
> +import string
>  import subprocess
>  import sys
>  import tempfile
> @@ -2297,9 +2298,8 @@ def parsebool(s):
>      """
>      return _booleans.get(s.lower(), None)
> -_hexdig = '0123456789ABCDEFabcdef'
>  _hextochr = dict((a + b, chr(int(a + b, 16)))
> -                 for a in _hexdig for b in _hexdig)
> +                 for a in string.hexdigits for b in string.hexdigits
Adding a u in front of '0123....' will work. This one also looks good.
>
>  def _urlunquote(s):
>      """Decode HTTP/HTML % encoding.
> 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
> @@ -120,48 +120,35 @@
>    mercurial/httpconnection.py: error importing: <TypeError> Can't mix
strings and bytes in path components (error at i18n.py:*)
>    mercurial/httppeer.py: error importing: <TypeError> Can't mix strings
and bytes in path components (error at i18n.py:*)
>    mercurial/i18n.py: error importing module: <TypeError> bytes expected,
not str (line *)
> -  mercurial/keepalive.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/localrepo.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/lock.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/mail.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/manifest.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/match.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/mdiff.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/merge.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/minirst.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/namespaces.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/obsolete.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/patch.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/pathutil.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/peer.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/profiling.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/pushkey.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/pvec.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/registrar.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/repair.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/repoview.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/revlog.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/revset.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/scmutil.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/scmwindows.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/similar.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/simplemerge.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/sshpeer.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/sshserver.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/sslutil.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/statichttprepo.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/store.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/streamclone.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/subrepo.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/tagmerge.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/tags.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/templatefilters.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/templatekw.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/templater.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/transaction.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/ui.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> -  mercurial/unionrepo.py: error importing: <TypeError> int() can't
convert non-string with explicit base (error at util.py:*)
> -  mercurial/url.py: error importing: <TypeError> int() can't convert
non-string with explicit base (error at util.py:*)
> +  mercurial/keepalive.py: error importing module: <AttributeError>
module 'mercurial.util' has no attribute 'httplib' (line *)
> +  mercurial/localrepo.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/mail.py: error importing module: <AttributeError> module
'email' has no attribute 'Header' (line *)
> +  mercurial/manifest.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/merge.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/namespaces.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/obsolete.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/patch.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/pushkey.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/pvec.py: error importing module: <NameError> name 'xrange'
is not defined (line *)
> +  mercurial/repair.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/repoview.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/revlog.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/revset.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/scmutil.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/scmwindows.py: error importing module: <ImportError> No
module named 'winreg' (line *)
> +  mercurial/simplemerge.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/sshpeer.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/sshserver.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/statichttprepo.py: error importing: <AttributeError> module
'mercurial.util' has no attribute 'urlerr' (error at byterange.py:*)
> +  mercurial/store.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/streamclone.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/subrepo.py: error importing: <TypeError> unorderable types:
str() >= tuple() (error at util.py:*)
> +  mercurial/templatefilters.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/templatekw.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/templater.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/ui.py: error importing: <TypeError> unorderable types: str()
>= tuple() (error at util.py:*)
> +  mercurial/unionrepo.py: error importing: <TypeError> unorderable
types: str() >= tuple() (error at util.py:*)
> +  mercurial/url.py: error importing: <AttributeError> module
'mercurial.util' has no attribute 'urlerr' (error at httpconnection.py:*)
>    mercurial/verify.py: error importing module: <TypeError> unorderable
types: str() >= tuple() (line *)
>    mercurial/win32.py: error importing module: <ImportError> No module
named 'msvcrt' (line *)
>    mercurial/windows.py: error importing module: <ImportError> No module
named 'msvcrt' (line *)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - Oct. 7, 2016, 1:14 p.m.
> On Oct 7, 2016, at 15:13, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> 
> > +                 for a in string.hexdigits for b in string.hexdigits
> Adding a u in front of '0123....' will work. This one also looks good.

Yeah. I went with this because it seems silly to have our own version of the constant when Python already has one that is the right type.
Martijn Pieters - Oct. 7, 2016, 1:31 p.m.
On 7 October 2016 at 15:14, Augie Fackler <raf@durin42.com> wrote:

> Yeah. I went with this because it seems silly to have our own version of
> the constant when Python already has one that is the right type.
>

There is more silliness.

The whole unquote function duplicates the one in the Python stdlib, from
2.4 somewhere, because it improved perf to not have to import urllib,
see c285bdb0572a48a34c9669a549793e60b5a469d4:

    util.url: copy urllib.unquote() into util to improve startup times

However, Python has since moved this function to urlparse in 2.6 (much
smaller module, so no perf reason anymore), the hex-digits dictionary is
build lazily in Python 3, and *most importantly*, those versions avoid the
quadratic performance behaviour of the 2.4 version we copied into Mercurial
(string building with += that evades the CPython optimisation for string
concatenation because multiple strings are being concatenated in each
expression).

So what *really* needs to happen is splitting out the `url` class out to a
new module (so it is used from fewer places and not always imported) and
then the stdlib unquote should be used again.

I'll see about creating a patch for this.

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -28,6 +28,7 @@  import re as remod
 import shutil
 import signal
 import socket
+import string
 import subprocess
 import sys
 import tempfile
@@ -2297,9 +2298,8 @@  def parsebool(s):
     """
     return _booleans.get(s.lower(), None)
 
-_hexdig = '0123456789ABCDEFabcdef'
 _hextochr = dict((a + b, chr(int(a + b, 16)))
-                 for a in _hexdig for b in _hexdig)
+                 for a in string.hexdigits for b in string.hexdigits)
 
 def _urlunquote(s):
     """Decode HTTP/HTML % encoding.
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
@@ -120,48 +120,35 @@ 
   mercurial/httpconnection.py: error importing: <TypeError> Can't mix strings and bytes in path components (error at i18n.py:*)
   mercurial/httppeer.py: error importing: <TypeError> Can't mix strings and bytes in path components (error at i18n.py:*)
   mercurial/i18n.py: error importing module: <TypeError> bytes expected, not str (line *)
-  mercurial/keepalive.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/localrepo.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/lock.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/mail.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/manifest.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/match.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/mdiff.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/merge.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/minirst.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/namespaces.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/obsolete.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/patch.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/pathutil.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/peer.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/profiling.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/pushkey.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/pvec.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/registrar.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/repair.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/repoview.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/revlog.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/revset.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/scmutil.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/scmwindows.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/similar.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/simplemerge.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/sshpeer.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/sshserver.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/sslutil.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/statichttprepo.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/store.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/streamclone.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/subrepo.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/tagmerge.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/tags.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/templatefilters.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/templatekw.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/templater.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/transaction.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/ui.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/unionrepo.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
-  mercurial/url.py: error importing: <TypeError> int() can't convert non-string with explicit base (error at util.py:*)
+  mercurial/keepalive.py: error importing module: <AttributeError> module 'mercurial.util' has no attribute 'httplib' (line *)
+  mercurial/localrepo.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/mail.py: error importing module: <AttributeError> module 'email' has no attribute 'Header' (line *)
+  mercurial/manifest.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/merge.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/namespaces.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/obsolete.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/patch.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/pushkey.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/pvec.py: error importing module: <NameError> name 'xrange' is not defined (line *)
+  mercurial/repair.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/repoview.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/revlog.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/revset.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/scmutil.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/scmwindows.py: error importing module: <ImportError> No module named 'winreg' (line *)
+  mercurial/simplemerge.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/sshpeer.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/sshserver.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/statichttprepo.py: error importing: <AttributeError> module 'mercurial.util' has no attribute 'urlerr' (error at byterange.py:*)
+  mercurial/store.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/streamclone.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/subrepo.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/templatefilters.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/templatekw.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/templater.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/ui.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/unionrepo.py: error importing: <TypeError> unorderable types: str() >= tuple() (error at util.py:*)
+  mercurial/url.py: error importing: <AttributeError> module 'mercurial.util' has no attribute 'urlerr' (error at httpconnection.py:*)
   mercurial/verify.py: error importing module: <TypeError> unorderable types: str() >= tuple() (line *)
   mercurial/win32.py: error importing module: <ImportError> No module named 'msvcrt' (line *)
   mercurial/windows.py: error importing module: <ImportError> No module named 'msvcrt' (line *)