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

login
register
mail settings
Submitter Boris Feld
Date Oct. 18, 2017, 4:09 p.m.
Message ID <5eec60fda2b053ca9f65.1508342995@FB>
Download mbox | patch
Permalink /patch/25185/
State Accepted
Headers show

Comments

Boris Feld - Oct. 18, 2017, 4:09 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@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.
Gregory Szorc - Oct. 18, 2017, 5:01 p.m.
On Wed, Oct 18, 2017 at 6:09 PM, Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@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)
>
> +@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@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Patch

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)
 
+@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)
+
 @parthandler('check:heads')
 def handlecheckheads(op, inpart):
     """check that head of the repo did not change