[PATCH 2 of 8 V7] bookmarks: introduce binary encoding

[Gregory Szorc]

On Wed, Nov 9, 2016 at 9:22 AM, Augie Fackler <raf at durin42.com> wrote:

> On Wed, Nov 02, 2016 at 07:06:44AM -0700, Stanislau Hlebik wrote:
> > # HG changeset patch
> > # User Stanislau Hlebik <stash at fb.com>
> > # Date 1478016027 25200
> > #      Tue Nov 01 09:00:27 2016 -0700
> > # Branch stable
> > # Node ID f3da841e3d47ccbb0be3892b521607c09fdeab13
> > # Parent  a56a624a8a42b93ec980d2c284756a38719dffe6
> > bookmarks: introduce binary encoding
> >
> > Bookmarks binary encoding will be used for `bookmarks` bundle2 part.
> > The format is: <4 bytes - bookmark size, big-endian><bookmark>
> >                <1 byte - 0 if node is empty 1 otherwise><20 bytes node>
>
> Enough other formats have 32 bytes of space for nodes (eg revlog) that
> I suspect we should do 32 bytes here too. Thoughts?
>

The wire protocol today nearly universally assumes a binary node is 20
bytes. I want 32 byte hashes as bad as the next person. But until we have
actual patches working towards that goal, I'm inclined to call YAGNI and
say 20 bytes is acceptable. As Pierre-Yves says, we can use a bundle part
parameter (or introduce a new bundle part) to handle >20 bytes nodes when
needed.


>
> > BookmarksEncodeError and BookmarksDecodeError maybe thrown if input is
> > incorrect.
> >
> > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> > --- a/mercurial/bookmarks.py
> > +++ b/mercurial/bookmarks.py
> > @@ -7,8 +7,10 @@
> >
> >  from __future__ import absolute_import
> >
> > +import StringIO
> >  import errno
> >  import os
> > +import struct
> >
> >  from .i18n import _
> >  from .node import (
> > @@ -23,6 +25,70 @@
> >      util,
> >  )
> >
> > +_NONEMPTYNODE = struct.pack('?', False)
> > +_EMPTYNODE = struct.pack('?', True)
> > +
> > +def _unpackbookmarksize(stream):
> > +    """Returns 0 if stream is empty.
> > +    """
> > +
> > +    expectedsize = struct.calcsize('>i')
> > +    encodedbookmarksize = stream.read(expectedsize)
> > +    if len(encodedbookmarksize) == 0:
> > +        return 0
> > +    if len(encodedbookmarksize) != expectedsize:
> > +        raise ValueError(
> > +            _('cannot decode bookmark size: '
> > +              'expected size: %d, actual size: %d') %
> > +            (expectedsize, len(encodedbookmarksize)))
> > +    return struct.unpack('>i', encodedbookmarksize)[0]
> > +
> > +def encodebookmarks(bookmarks):
> > +    """Encodes bookmark to node mapping to the byte string.
> > +
> > +    Format: <4 bytes - bookmark size><bookmark>
> > +            <1 byte - 0 if node is empty 1 otherwise><20 bytes node>
> > +    Node may be 20 byte binary string, 40 byte hex string or empty
> > +    """
> > +
> > +    for bookmark, node in bookmarks.iteritems():
> > +        yield struct.pack('>i', (len(bookmark)))
> > +        yield encoding.fromlocal(bookmark)
> > +        if node:
> > +            if len(node) != 20 and len(node) != 40:
> > +                raise ValueError(_('node must be 20 or bytes long'))
> > +            if len(node) == 40:
> > +                node = bin(node)
> > +            yield _NONEMPTYNODE
> > +            yield node
> > +        else:
> > +            yield _EMPTYNODE
> > +
> > +def decodebookmarks(buf):
> > +    """Decodes buffer into bookmark to node mapping.
> > +
> > +    Node is either 20 bytes or empty.
> > +    """
> > +
> > +    stream = StringIO.StringIO(buf)
> > +    bookmarks = {}
> > +    bookmarksize = _unpackbookmarksize(stream)
> > +    while bookmarksize:
> > +        bookmark = stream.read(bookmarksize)
> > +        if len(bookmark) != bookmarksize:
> > +            raise ValueError(
> > +                _('cannot decode bookmark: expected size: %d, '
> > +                'actual size: %d') % (bookmarksize, len(bookmark)))
> > +        bookmark = encoding.tolocal(bookmark)
> > +        packedemptynodeflag = stream.read(struct.calcsize('?'))
> > +        emptynode = struct.unpack('?', packedemptynodeflag)[0]
> > +        node = ''
> > +        if not emptynode:
> > +            node = stream.read(20)
> > +        bookmarks[bookmark] = node
> > +        bookmarksize = _unpackbookmarksize(stream)
> > +    return bookmarks
> > +
> >  def _getbkfile(repo):
> >      """Hook so that extensions that mess with the store can hook bm
> storage.
> >
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20161110/1f9482a6/attachment-0001.html>