Patchwork [6,of,8,V5] exchange: add `pushbookmarks` part generator

login
register
mail settings
Submitter Stanislau Hlebik
Date Sept. 16, 2016, 11:10 a.m.
Message ID <4ace3cd7d2d2e1d63f08.1474024234@dev1918.lla1.facebook.com>
Download mbox | patch
Permalink /patch/16645/
State Accepted
Headers show

Comments

Stanislau Hlebik - Sept. 16, 2016, 11:10 a.m.
# HG changeset patch
# User Stanislau Hlebik <stash@fb.com>
# Date 1473954507 25200
#      Thu Sep 15 08:48:27 2016 -0700
# Node ID 4ace3cd7d2d2e1d63f0811034467e7b7e9719861
# Parent  b20ca1e0104fe34775f581295a0d0112dad80cb5
exchange: add `pushbookmarks` part generator

New generator instead of the old `bookmarks` generator.
It creates `bookmarks` and `check:bookmarks` bundle2 part2
instead of `pushkey` parts.
Pierre-Yves David - Sept. 16, 2016, 3:53 p.m.
On 09/16/2016 01:10 PM, Stanislau Hlebik wrote:
> # HG changeset patch
> # User Stanislau Hlebik <stash@fb.com>
> # Date 1473954507 25200
> #      Thu Sep 15 08:48:27 2016 -0700
> # Node ID 4ace3cd7d2d2e1d63f0811034467e7b7e9719861
> # Parent  b20ca1e0104fe34775f581295a0d0112dad80cb5
> exchange: add `pushbookmarks` part generator
>
> New generator instead of the old `bookmarks` generator.
> It creates `bookmarks` and `check:bookmarks` bundle2 part2
> instead of `pushkey` parts.
>
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -806,6 +806,54 @@
>          markers = sorted(pushop.outobsmarkers)
>          buildobsmarkerspart(bundler, markers)
>
> +def _pushb2ctxcheckbookmarks(pushop, bundler):
> +    """Generate race condition checking parts
> +
> +    Exists as an independent function to aid extensions
> +    """
> +    enc = encoding.fromlocal
> +    if not pushop.force:
> +        bookmarks = []
> +        for book, old, new in pushop.outbookmarks:
> +            if old:
> +                bookmarks.append(enc(book) + ' ' + old)
> +        bundler.newpart('check:bookmarks', data='\n'.join(bookmarks))

I'm certain how the existing --force option affect bookmark. but I 
suspect your new semantic is not matching it.

> +
> +@b2partsgenerator('pushbookmarks')
> +def _pushb2bookmarksnew(pushop, bundler):
> +    if 'bookmarks' in pushop.stepsdone:
> +        return
> +    b2caps = bundle2.bundle2caps(pushop.remote)
> +    if 'bookmarks' not in b2caps:
> +        return
> +    pushop.stepsdone.add('bookmarks')
> +    if not pushop.outbookmarks:
> +        return
> +    _pushb2ctxcheckbookmarks(pushop, bundler)
> +    booktoaction = {}
> +    bookmarksdata = []
> +    enc = encoding.fromlocal
> +    for book, old, new in pushop.outbookmarks:
> +        action = 'update'
> +        if not old:
> +            action = 'export'
> +        elif not new:
> +            action = 'delete'
> +        booktoaction[book] = action
> +        bookmarksdata.append('%s %s' % (enc(book), new))
> +    bundler.newpart('bookmarks', data='\n'.join(bookmarksdata))
> +
> +    def handlereply(op):
> +        ui = pushop.ui
> +        for book, old, new in pushop.outbookmarks:
> +            action = 'update'
> +            if not old:
> +                action = 'export'
> +            elif not new:
> +                action = 'delete'
> +            ui.status(bookmsgmap[action][0] % book)

The reply handling does not seems to actually handled any reply, (which 
highlight the fact than in patch 3, no reply is produced). This suspicious.

We either don't need a reply, and then need explanation of why we don't 
need it and we should not provide an handlereply callback. Or we still 
need a reply and we should actually handle a reply.

> +    return handlereply
> +
>  @b2partsgenerator('bookmarks')
>  def _pushb2bookmarks(pushop, bundler):
>      """handle bookmark push through bundle2"""
> @@ -1747,7 +1795,8 @@
>                      gettransaction()
>
>                  op = bundle2.bundleoperation(repo, gettransaction,
> -                                             captureoutput=captureoutput)
> +                                             captureoutput=captureoutput,
> +                                             applybookmarks=True)
>                  try:
>                      op = bundle2.processbundle(repo, cg, op=op)
>                  finally:

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -806,6 +806,54 @@ 
         markers = sorted(pushop.outobsmarkers)
         buildobsmarkerspart(bundler, markers)
 
+def _pushb2ctxcheckbookmarks(pushop, bundler):
+    """Generate race condition checking parts
+
+    Exists as an independent function to aid extensions
+    """
+    enc = encoding.fromlocal
+    if not pushop.force:
+        bookmarks = []
+        for book, old, new in pushop.outbookmarks:
+            if old:
+                bookmarks.append(enc(book) + ' ' + old)
+        bundler.newpart('check:bookmarks', data='\n'.join(bookmarks))
+
+@b2partsgenerator('pushbookmarks')
+def _pushb2bookmarksnew(pushop, bundler):
+    if 'bookmarks' in pushop.stepsdone:
+        return
+    b2caps = bundle2.bundle2caps(pushop.remote)
+    if 'bookmarks' not in b2caps:
+        return
+    pushop.stepsdone.add('bookmarks')
+    if not pushop.outbookmarks:
+        return
+    _pushb2ctxcheckbookmarks(pushop, bundler)
+    booktoaction = {}
+    bookmarksdata = []
+    enc = encoding.fromlocal
+    for book, old, new in pushop.outbookmarks:
+        action = 'update'
+        if not old:
+            action = 'export'
+        elif not new:
+            action = 'delete'
+        booktoaction[book] = action
+        bookmarksdata.append('%s %s' % (enc(book), new))
+    bundler.newpart('bookmarks', data='\n'.join(bookmarksdata))
+
+    def handlereply(op):
+        ui = pushop.ui
+        for book, old, new in pushop.outbookmarks:
+            action = 'update'
+            if not old:
+                action = 'export'
+            elif not new:
+                action = 'delete'
+            ui.status(bookmsgmap[action][0] % book)
+    return handlereply
+
 @b2partsgenerator('bookmarks')
 def _pushb2bookmarks(pushop, bundler):
     """handle bookmark push through bundle2"""
@@ -1747,7 +1795,8 @@ 
                     gettransaction()
 
                 op = bundle2.bundleoperation(repo, gettransaction,
-                                             captureoutput=captureoutput)
+                                             captureoutput=captureoutput,
+                                             applybookmarks=True)
                 try:
                     op = bundle2.processbundle(repo, cg, op=op)
                 finally: