Re: [PATCH 07 of 10 V9] exchange: getbundle `bookmarks` part generator

[Gregory Szorc]

On Sun, Nov 13, 2016 at 2:30 AM, Stanislau Hlebik  wrote:

> # HG changeset patch
> # User Stanislau Hlebik 
> # Date 1479032456 28800
> #  Sun Nov 13 02:20:56 2016 -0800
> # Branch stable
> # Node ID 606bb4a7fb818f24d52e764828ba0d0a7921f999
> # Parent  bf21586f26e5a41f7d8bf342d4b4c16d71dbc6d2
> 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))
>
> +@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'))
> +
> +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',
> +}
>

IIRC there were patches submitted last cycle around bookmark filtering -
clients saying which bookmarks they were interested in. This implies there
is advanced functionality around bookmarks exchange around the corner. So I
have reservations about a "bookmarks" argument that is simply a boolean and
doesn't allow more advanced uses later.

Anyway, does this argument to the "getbundle" wire protocol command need to
exist at all? Today, clients can ask for bookmarks data by adding a
"bookmarks" value to the "listkeys" getbundle wire protocol argument. If
the client sent a bundle2 capability that indicated it can receive the
"bookmarks" dedicated bundle2 part, then we wouldn't need to introduce a
new argument to "getbundle."

That being said, I can go both ways on this. Sometimes having an explicit
argument for "send me X" is nice to have for clarity. My final opinion
likely hinges on what other bookmarks exchange features are planned. If
"send a subset of bookmarks" or a similar feature is in the works, I'd
prefer to shoehorn that into the same getbundle argument so there is only 1
variable controlling how bookmarks are sent.



>
>  # client side
>
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 07 of 10 V9] exchange: getbundle `bookmarks` part generator

[Stanislau Hlebik]

# HG changeset patch
# User Stanislau Hlebik 
# Date 1479032456 28800
#  Sun Nov 13 02:20:56 2016 -0800
# Branch stable
# Node ID 606bb4a7fb818f24d52e764828ba0d0a7921f999
# Parent  bf21586f26e5a41f7d8bf342d4b4c16d71dbc6d2
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))
 
+@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'))
+
+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
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel