[PATCH 03 of 15] bookmark: add a 'check:bookmarks' bundle2 part

[Gregory Szorc]

On Wed, Oct 18, 2017 at 6:09 PM, Boris Feld <boris.feld at octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld at octobus.net>
> # Date 1508072463 -7200
> #      Sun Oct 15 15:01:03 2017 +0200
> # Node ID 5eec60fda2b053ca9f6527b4dc86631adac3ca91
> # Parent  f27feea4fe4c313aa87c85aabd1ed8c3c4b31296
> # EXP-Topic b2.bookmarks
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 5eec60fda2b0
> bookmark: add a 'check:bookmarks' bundle2 part
>
> This part checks that bookmarks are still at the node they are expected to
> be.
> This allows a pushing client to detect push race where the repository was
> updated between the time it discovered the server state and the time it
> managed
> to finish its push.
>
> Such checking already exists when pushing bookmark through pushkey. This
> new
> part can be inserted at the beginning of the bundle, triggering abort
> earlier.
> In addition, we would like to move away from pushey to push bookmark. A
> step
> useful to solve issue5165.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -155,6 +155,7 @@
>
>  from .i18n import _
>  from . import (
> +    bookmarks,
>      changegroup,
>      error,
>      node as nodemod,
> @@ -1702,6 +1703,34 @@
>      replyto = int(inpart.params['in-reply-to'])
>      op.records.add('changegroup', {'return': ret}, replyto)
>
> + at parthandler('check:bookmarks')
> +def handlecheckbookmarks(op, inpart):
> +    """check location of bookmarks
> +
> +    This part is to be used to detect push race regarding bookmark, it
> +    contains binary encoded (bookmark, node) tuple. If the local state
> does
> +    not marks the one in the part, a PushRaced exception is raised
> +    """
> +    bookdata = bookmarks.binarydecode(inpart)
> +
> +    msgstandard = ('repository changed while pushing - please try again '
> +                   '(bookmark "%s" move from %s to %s)')
> +    msgmissing = ('repository changed while pushing - please try again '
> +                  '(bookmark "%s" is missing, expected %s)')
> +    msgexist = ('repository changed while pushing - please try again '
> +                '(bookmark "%s" set on %s, expected missing)')
> +    for book, node in bookdata:
> +        currentnode = op.repo._bookmarks.get(book)
> +        if currentnode != node:
> +            if node is None:
> +                finalmsg = msgexist % (book, nodemod.short(currentnode))
> +            elif currentnode is None:
> +                finalmsg = msgmissing % (book, nodemod.short(node))
> +            else:
> +                finalmsg = msgstandard % (book, nodemod.short(node),
> +                                          nodemod.short(currentnode))
> +            raise error.PushRaced(finalmsg)
> +
>

+1 to the feature!

Currently, the error message only captures the first failure. Do you think
it should capture all failures? (This could be changed as a follow-up of
course.)


>  @parthandler('check:heads')
>  def handlecheckheads(op, inpart):
>      """check that head of the repo did not change
> _______________________________________________
> 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/20171018/54d8cdd5/attachment.html>