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

[Pierre-Yves David]

On 09/16/2016 01:10 PM, Stanislau Hlebik wrote:
> # HG changeset patch
> # User Stanislau Hlebik <stash at 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.

> +
> + at 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:


-- 
Pierre-Yves David