[PATCH 07 of 10 V10] exchange: getbundle `bookmarks` part generator
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Wed Nov 23 17:01:07 EST 2016
On 11/22/2016 04:29 AM, Gregory Szorc wrote:
> On Sun, Nov 20, 2016 at 4:14 AM, Stanislau Hlebik <stash at fb.com
> <mailto:stash at fb.com>> wrote:
>
> # HG changeset patch
> # User Stanislau Hlebik <stash at fb.com <mailto:stash at fb.com>>
> # Date 1479373181 28800
> # Thu Nov 17 00:59:41 2016 -0800
> # Node ID 5af41d2c5226c36d5a1f999ff3d99d8694ae68b9
> # Parent 866281dae2407308c19c7c3109bb5501b940ee67
> exchange: getbundle `bookmarks` part generator
>
> This generator will be used during pull operation.
>
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -1680,6 +1680,21 @@
> if chunks:
> bundler.newpart('hgtagsfnodes', data=''.join(chunks))
>
> + at getbundle2partsgenerator('bookmarks')
> +def _getbundlebookmarkspart(bundler, repo, source, bundlecaps=None,
> + b2caps=None, heads=None, common=None,
> + **kwargs):
> + if not kwargs.get('bookmarks'):
> + return
> + if 'bookmarks' not in b2caps:
> + raise ValueError(
> + _('bookmarks are requested but client is not capable '
> + 'of receiving it'))
>
>
> I fully support the principle of failing fast. However, AFAICT no other
> @getbundle2partsgenerator raises a ValueError like this.
> _getbundletagsfnodes() for example silently no-ops.
>
> I like the idea of failing here. But at the same time, this is breaking
> an apparent convention. I'd like a 2nd opinion on whether we should fail
> or just no-op.
Failing seems to the the right behavior here. 'getbundletagsfnodes' is
providing the client with a cache entry. So if we fail to provide it,
the client will behave properly but have to do extra work. In addition
the client do not explicitly request the tagsfnodes entry. It request
changesets and we kindly provide the cache entry along if the client
support it.
On the other hand in this case, the client is explicitly requesting
bookmark data, if we do not provide any the client will see a valid
reply with no bookmark and think the server has no bookmark. This lead
to an incorrect behavior. So crashing on bogus request is the way to go
here.
>
>
> +
> + bookmarks = _getbookmarks(repo, **kwargs)
> + encodedbookmarks = bookmod.encodebookmarks(bookmarks)
> + bundler.newpart('bookmarks', data=encodedbookmarks)
> +
> def _getbookmarks(repo, **kwargs):
> """Returns bookmark to node mapping.
>
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -228,7 +228,9 @@
> 'bundlecaps': 'scsv',
> 'listkeys': 'csv',
> 'cg': 'boolean',
> - 'cbattempted': 'boolean'}
> + 'cbattempted': 'boolean',
> + 'bookmarks': 'boolean',
> + }
>
> # client side
>
>
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list