Patchwork [5,of,7,V4] bundle2: add `pushbookmarks` part handler

login
register
mail settings
Submitter Stanislau Hlebik
Date Sept. 4, 2016, 10:46 p.m.
Message ID <6d2fbc0ed52b54dfe525.1473029217@dev1918.lla1.facebook.com>
Download mbox | patch
Permalink /patch/16546/
State Accepted
Headers show

Comments

Stanislau Hlebik - Sept. 4, 2016, 10:46 p.m.
# HG changeset patch
# User Stanislau Hlebik <stash@fb.com>
# Date 1473012368 25200
#      Sun Sep 04 11:06:08 2016 -0700
# Node ID 6d2fbc0ed52b54dfe525063da90973f426f2bcbe
# Parent  db526b94eeb678879c2faceb40c64aebbe1054af
bundle2: add `pushbookmarks` part handler
Pierre-Yves David - Sept. 6, 2016, 1:01 p.m.
On 09/05/2016 12:46 AM, Stanislau Hlebik wrote:
> # HG changeset patch
> # User Stanislau Hlebik <stash@fb.com>
> # Date 1473012368 25200
> #      Sun Sep 04 11:06:08 2016 -0700
> # Node ID 6d2fbc0ed52b54dfe525063da90973f426f2bcbe
> # Parent  db526b94eeb678879c2faceb40c64aebbe1054af
> bundle2: add `pushbookmarks` part handler

The fact we need two different part  handler with the very same 
transport is a good hint that something is wrong. This get in 
perspective with my comment about record in patch 3.

Without thinking too much about it I don't see any reason why we would 
need different logic for each direction. Am I missing something?

Cheers,
Stanislau Hlebik - Sept. 6, 2016, 3:33 p.m.
But in case of push we also send ‘old’ positions of the bookmarks (they are necessary for bookmarks.pushbookmarks).
So the part handlers should be different, don’t they?

On 9/6/16, 2:01 PM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:

    
    
    On 09/05/2016 12:46 AM, Stanislau Hlebik wrote:
    > # HG changeset patch

    > # User Stanislau Hlebik <stash@fb.com>

    > # Date 1473012368 25200

    > #      Sun Sep 04 11:06:08 2016 -0700

    > # Node ID 6d2fbc0ed52b54dfe525063da90973f426f2bcbe

    > # Parent  db526b94eeb678879c2faceb40c64aebbe1054af

    > bundle2: add `pushbookmarks` part handler

    
    The fact we need two different part  handler with the very same 
    transport is a good hint that something is wrong. This get in 
    perspective with my comment about record in patch 3.
    
    Without thinking too much about it I don't see any reason why we would 
    need different logic for each direction. Am I missing something?
    
    Cheers,
    
    -- 
    Pierre-Yves David
Pierre-Yves David - Sept. 6, 2016, 3:35 p.m.
On 09/06/2016 05:33 PM, Stanislau Hlebik wrote:
> But in case of push we also send ‘old’ positions of the bookmarks (they are necessary for bookmarks.pushbookmarks).
> So the part handlers should be different, don’t they?

The "old" value is related to the way pushkey works (and detect race 
condition). I'm sure we can build a part in a way it fit both usage in a 
simple way.

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,
     encoding,
     error,
@@ -1623,3 +1624,32 @@ 
         book, node = bookmarknode.rsplit(' ', 1)
         bookmarks[dec(book)] = dec(node)
     op.records.add('bookmarks', bookmarks)
+
+@parthandler('pushbookmarks')
+def handlepushbookmarks(op, inpart):
+    bookmarksresults = []
+    # Grab the transaction to ensure that we have the lock before performing the
+    # pushbookmark.
+    if op.ui.configbool('experimental', 'bundle2lazylocking'):
+        op.gettransaction()
+    for bookmarkinfo in inpart.read().splitlines():
+        dec = encoding.tolocal
+        book, old, new = bookmarkinfo.rsplit(' ', 2)
+        book = dec(book)
+        old = dec(old)
+        new = dec(new)
+        ret = bookmarks.pushbookmark(op.repo, book, old, new)
+        bookmarksresults.append('%s %i' % (book, ret))
+    if op.reply is not None:
+        rpart = op.reply.newpart(
+            'reply:pushbookmarks', data='\n'.join(bookmarksresults))
+        rpart.addparam('in-reply-to', str(inpart.id), mandatory=False)
+
+@parthandler('reply:pushbookmarks')
+def handlepushbookmarksreply(op, inpart):
+    partid = int(inpart.params['in-reply-to'])
+    for bookmarksresult in inpart.read().splitlines():
+        bookmark, ret = bookmarksresult.rsplit(' ', 1)
+        op.records.add('pushbookmarks',
+                       {'bookmark': bookmark, 'return': ret},
+                       partid)