[PATCH 02 of 15 V2] bookmark: add methods to binary encode and decode bookmark values

Augie Fackler raf at durin42.com
Fri Nov 10 17:35:52 EST 2017


(+indygreg, who also is a formats enthusiast)

On Thu, Nov 02, 2017 at 02:17:59PM +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld at octobus.net>
> # Date 1508072395 -7200
> #      Sun Oct 15 14:59:55 2017 +0200
> # Node ID 4d0c6772a81aa1e2b25f32f944563db1f33fd327
> # Parent  8c9a9eecdcd61401a1604a08a5272f7dabd4b912
> # EXP-Topic b2.bookmarks
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 4d0c6772a81a
> bookmark: add methods to binary encode and decode bookmark values
>
> Coming new bundle2 parts related to bookmark will use a binary encoding. It
> encodes a series of '(bookmark, node)' pairs. Bookmark name has a high enough
> size limit to not be affected by issue5165. (64K length, we are well covered)

I'm not thrilled here. Could we do some sort of varint encoding, which
would be generally useful going forward for a variety of things,
rather than just shrugging and setting a limit which we (today) deem
absurdly large?

I agree that practically speaking, nobody should have a 64k bookmark,
but we can do better. We also know that \0 shouldn't appear in a
bookmark name, so we could just make the format be
null-terminated. I'd much rather we get in the practice of
deliberately crafting formats that are maximally flexible and
reusable.

IOW, encode something like this:

struct {
  node [20]byte
  name [] byte // continues until you see the first NUL
}

Greg, am I talking crazy?


>
> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> --- a/mercurial/bookmarks.py
> +++ b/mercurial/bookmarks.py
> @@ -8,12 +8,14 @@
>  from __future__ import absolute_import
>
>  import errno
> +import struct
>
>  from .i18n import _
>  from .node import (
>      bin,
>      hex,
>      short,
> +    wdirid,
>  )
>  from . import (
>      encoding,
> @@ -550,6 +552,60 @@ def unhexlifybookmarks(marks):
>          binremotemarks[name] = bin(node)
>      return binremotemarks
>
> +_binaryentry = struct.Struct('>20sH')
> +
> +def binaryencode(bookmarks):
> +    """encode a '(bookmark, node)' iterable into a binary stream
> +
> +    the binary format is:
> +
> +        <node><bookmark-length><bookmark-name>
> +
> +    :node: is a 20 bytes binary node,
> +    :bookmark-length: an unsigned short,
> +    :bookmark-name: the name of the bookmark (of length <bookmark-length>)
> +
> +    wdirid (all bits set) will be used as a special value for "missing"
> +    """
> +    binarydata = []
> +    for book, node in bookmarks:
> +        if not node: # None or ''
> +            node = wdirid
> +        binarydata.append(_binaryentry.pack(node, len(book)))
> +        binarydata.append(book)
> +    return ''.join(binarydata)
> +
> +def binarydecode(stream):
> +    """decode a binary stream into an '(bookmark, node)' iterable
> +
> +    the binary format is:
> +
> +        <node><bookmark-length><bookmark-name>
> +
> +    :node: is a 20 bytes binary node,
> +    :bookmark-length: an unsigned short,
> +    :bookmark-name: the name of the bookmark (of length <bookmark-length>))
> +
> +    wdirid (all bits set) will be used as a special value for "missing"
> +    """
> +    entrysize = _binaryentry.size
> +    books = []
> +    while True:
> +        entry = stream.read(entrysize)
> +        if len(entry) < entrysize:
> +            if entry:
> +                raise error.Abort(_('bad bookmark stream'))
> +            break
> +        node, length = _binaryentry.unpack(entry)
> +        bookmark = stream.read(length)
> +        if len(bookmark) < length:
> +            if entry:
> +                raise error.Abort(_('bad bookmark stream'))
> +        if node == wdirid:
> +            node = None
> +        books.append((bookmark, node))
> +    return books
> +
>  def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=()):
>      ui.debug("checking for updated bookmarks\n")
>      localmarks = repo._bookmarks
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list