Patchwork [03,of,10,py3] obsolete: use bytestr() instead of str() so the node is bytes on py3

login
register
mail settings
Submitter Augie Fackler
Date Aug. 1, 2017, 8:34 p.m.
Message ID <f50da59f7977c7d79685.1501619671@augie-macbookpro2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/22613/
State Accepted
Headers show

Comments

Augie Fackler - Aug. 1, 2017, 8:34 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1500907059 14400
#      Mon Jul 24 10:37:39 2017 -0400
# Node ID f50da59f7977c7d796850b95ac7c80a4fd92bd18
# Parent  8b22554199879f80a13ddee556bc540ddcb65a94
obsolete: use bytestr() instead of str() so the node is bytes on py3

I'm not sure this is right, since this should either be bytes or str
to match what's going on in the revlog layer.
Yuya Nishihara - Aug. 2, 2017, 3:11 p.m.
On Tue, 01 Aug 2017 16:34:31 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1500907059 14400
> #      Mon Jul 24 10:37:39 2017 -0400
> # Node ID f50da59f7977c7d796850b95ac7c80a4fd92bd18
> # Parent  8b22554199879f80a13ddee556bc540ddcb65a94
> obsolete: use bytestr() instead of str() so the node is bytes on py3
> 
> I'm not sure this is right, since this should either be bytes or str
> to match what's going on in the revlog layer.

Sounds like a bug of revlog. 'prec' should be a binary node id, which can't
be a unicode string.

> -        marker = (str(prec), tuple(succs), int(flag), metadata, date, parents)
> +        prec = bytes(pycompat.bytestr(prec))
> +        marker = (prec, tuple(succs), int(flag), metadata, date, parents)
Augie Fackler - Aug. 2, 2017, 3:37 p.m.
> On Aug 2, 2017, at 11:11, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> On Tue, 01 Aug 2017 16:34:31 -0400, Augie Fackler wrote:
>> # HG changeset patch
>> # User Augie Fackler <augie@google.com>
>> # Date 1500907059 14400
>> #      Mon Jul 24 10:37:39 2017 -0400
>> # Node ID f50da59f7977c7d796850b95ac7c80a4fd92bd18
>> # Parent  8b22554199879f80a13ddee556bc540ddcb65a94
>> obsolete: use bytestr() instead of str() so the node is bytes on py3
>> 
>> I'm not sure this is right, since this should either be bytes or str
>> to match what's going on in the revlog layer.
> 
> Sounds like a bug of revlog. 'prec' should be a binary node id, which can't
> be a unicode string.

I think in some cases prec coming in as something that __str__ or __repr__s down to a valid node id, which is why the str() is here. It definitely breaks if you just remove the str().

> 
>> -        marker = (str(prec), tuple(succs), int(flag), metadata, date, parents)
>> +        prec = bytes(pycompat.bytestr(prec))
>> +        marker = (prec, tuple(succs), int(flag), metadata, date, parents)
Yuya Nishihara - Aug. 2, 2017, 3:52 p.m.
On Wed, 2 Aug 2017 11:37:45 -0400, Augie Fackler wrote:
> 
> > On Aug 2, 2017, at 11:11, Yuya Nishihara <yuya@tcha.org> wrote:
> > 
> > On Tue, 01 Aug 2017 16:34:31 -0400, Augie Fackler wrote:
> >> # HG changeset patch
> >> # User Augie Fackler <augie@google.com>
> >> # Date 1500907059 14400
> >> #      Mon Jul 24 10:37:39 2017 -0400
> >> # Node ID f50da59f7977c7d796850b95ac7c80a4fd92bd18
> >> # Parent  8b22554199879f80a13ddee556bc540ddcb65a94
> >> obsolete: use bytestr() instead of str() so the node is bytes on py3
> >> 
> >> I'm not sure this is right, since this should either be bytes or str
> >> to match what's going on in the revlog layer.
> > 
> > Sounds like a bug of revlog. 'prec' should be a binary node id, which can't
> > be a unicode string.
> 
> I think in some cases prec coming in as something that __str__ or __repr__s down to a valid node id, which is why the str() is here. It definitely breaks if you just remove the str().

Ah, got it. Perhaps it's a changectx. In which case, I think bytes() should
work. If prec doesn't implement __bytes__(), it's probably wrong.

> >> -        marker = (str(prec), tuple(succs), int(flag), metadata, date, parents)
> >> +        prec = bytes(pycompat.bytestr(prec))
> >> +        marker = (prec, tuple(succs), int(flag), metadata, date, parents)
>

Patch

diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -79,6 +79,7 @@  from . import (
     obsutil,
     phases,
     policy,
+    pycompat,
     util,
 )
 
@@ -583,7 +584,8 @@  class obsstore(object):
 
         metadata = tuple(sorted(metadata.iteritems()))
 
-        marker = (str(prec), tuple(succs), int(flag), metadata, date, parents)
+        prec = bytes(pycompat.bytestr(prec))
+        marker = (prec, tuple(succs), int(flag), metadata, date, parents)
         return bool(self.add(transaction, [marker]))
 
     def add(self, transaction, markers):