Patchwork [08,of,10,py3] localrepo: ensure transaction id is fully bytes on py3

login
register
mail settings
Submitter Augie Fackler
Date March 19, 2017, 6:26 p.m.
Message ID <9e6b585465b7ec033160.1489947981@imladris.local>
Download mbox | patch
Permalink /patch/19459/
State Accepted
Headers show

Comments

Augie Fackler - March 19, 2017, 6:26 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1489900139 14400
#      Sun Mar 19 01:08:59 2017 -0400
# Node ID 9e6b585465b7ec033160326a4eb049329a2f83a9
# Parent  d5e20f704fd722dbf8b0a011b3c9ab7437b4ac82
localrepo: ensure transaction id is fully bytes on py3
via Mercurial-devel - March 20, 2017, 11:52 p.m.
On Sun, Mar 19, 2017 at 11:26 AM, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1489900139 14400
> #      Sun Mar 19 01:08:59 2017 -0400
> # Node ID 9e6b585465b7ec033160326a4eb049329a2f83a9
> # Parent  d5e20f704fd722dbf8b0a011b3c9ab7437b4ac82
> localrepo: ensure transaction id is fully bytes on py3
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -49,6 +49,7 @@ from . import (
>      peer,
>      phases,
>      pushkey,
> +    pycompat,
>      repoview,
>      revset,
>      revsetlang,
> @@ -1082,7 +1083,10 @@ class localrepository(object):
>                  hint=_("run 'hg recover' to clean up transaction"))
>
>          idbase = "%.40f#%f" % (random.random(), time.time())
> -        txnid = 'TXN:' + hashlib.sha1(idbase).hexdigest()
> +        ha = hashlib.sha1(idbase).hexdigest()

Just for my understanding, what makes idbase bytes on py3 (as
hashlib.sha1() requires)?

> +        if pycompat.ispy3:
> +            ha = ha.encode('latin1')

Every time I see "if pycompat.ispy3:", I feel like we should have
extracted a function instead. Do others agree? What could this method
be called? pycompat.latin1bytes()?

> +        txnid = 'TXN:' + ha
>          self.hook('pretxnopen', throw=True, txnname=desc, txnid=txnid)
>
>          self._writejournal(desc)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - March 21, 2017, 12:15 a.m.
On Mar 20, 2017 19:52, "Martin von Zweigbergk" <martinvonz@google.com>
wrote:

On Sun, Mar 19, 2017 at 11:26 AM, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1489900139 14400
> #      Sun Mar 19 01:08:59 2017 -0400
> # Node ID 9e6b585465b7ec033160326a4eb049329a2f83a9
> # Parent  d5e20f704fd722dbf8b0a011b3c9ab7437b4ac82
> localrepo: ensure transaction id is fully bytes on py3
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -49,6 +49,7 @@ from . import (
>      peer,
>      phases,
>      pushkey,
> +    pycompat,
>      repoview,
>      revset,
>      revsetlang,
> @@ -1082,7 +1083,10 @@ class localrepository(object):
>                  hint=_("run 'hg recover' to clean up transaction"))
>
>          idbase = "%.40f#%f" % (random.random(), time.time())
> -        txnid = 'TXN:' + hashlib.sha1(idbase).hexdigest()
> +        ha = hashlib.sha1(idbase).hexdigest()

Just for my understanding, what makes idbase bytes on py3 (as
hashlib.sha1() requires)?


Our module loader thing, by turning the un-marked string constant into
bytes.


> +        if pycompat.ispy3:
> +            ha = ha.encode('latin1')

Every time I see "if pycompat.ispy3:", I feel like we should have
extracted a function instead. Do others agree? What could this method
be called? pycompat.latin1bytes()?


Maybe. In this case a sha1 wrapper likely makes more sense.


> +        txnid = 'TXN:' + ha
>          self.hook('pretxnopen', throw=True, txnname=desc, txnid=txnid)
>
>          self._writejournal(desc)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - March 21, 2017, 1:18 a.m.
On Mon, 20 Mar 2017 20:15:31 -0400, Augie Fackler wrote:
> On Mar 20, 2017 19:52, "Martin von Zweigbergk" <martinvonz@google.com>
> wrote:
> On Sun, Mar 19, 2017 at 11:26 AM, Augie Fackler <raf@durin42.com> wrote:
> > +        if pycompat.ispy3:
> > +            ha = ha.encode('latin1')
> 
> Every time I see "if pycompat.ispy3:", I feel like we should have
> extracted a function instead. Do others agree? What could this method
> be called? pycompat.latin1bytes()?
> 
> Maybe. In this case a sha1 wrapper likely makes more sense.

or it could be node.hex(sha1(idbase).digest()).
Augie Fackler - March 21, 2017, 2:05 a.m.
> On Mar 20, 2017, at 21:18, Yuya Nishihara <yuya@tcha.org> wrote:
> 
>> Maybe. In this case a sha1 wrapper likely makes more sense.
> 
> or it could be node.hex(sha1(idbase).digest()).

Wow. I can't believe I missed that. Patch on the way momentarily.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -49,6 +49,7 @@  from . import (
     peer,
     phases,
     pushkey,
+    pycompat,
     repoview,
     revset,
     revsetlang,
@@ -1082,7 +1083,10 @@  class localrepository(object):
                 hint=_("run 'hg recover' to clean up transaction"))
 
         idbase = "%.40f#%f" % (random.random(), time.time())
-        txnid = 'TXN:' + hashlib.sha1(idbase).hexdigest()
+        ha = hashlib.sha1(idbase).hexdigest()
+        if pycompat.ispy3:
+            ha = ha.encode('latin1')
+        txnid = 'TXN:' + ha
         self.hook('pretxnopen', throw=True, txnname=desc, txnid=txnid)
 
         self._writejournal(desc)