Patchwork [7,of,8,V5] exchange: use `bookmarks` bundle2 part

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

Comments

Stanislau Hlebik - Sept. 16, 2016, 11:10 a.m.
# HG changeset patch
# User Stanislau Hlebik <stash@fb.com>
# Date 1473954485 25200
#      Thu Sep 15 08:48:05 2016 -0700
# Node ID 837f2f1650ebf323b46924040f180fc966e80fbf
# Parent  4ace3cd7d2d2e1d63f0811034467e7b7e9719861
exchange: use `bookmarks` bundle2 part

Apply changes from `bookmarks` part.
Initial idea was to add a special mode to the bookmarks part handler that will
apply bookmark changes differently (create divergent bookmarks etc).
But it would've required significant code changes, so I decided to stick
with old approach.
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 1473954485 25200
> #      Thu Sep 15 08:48:05 2016 -0700
> # Node ID 837f2f1650ebf323b46924040f180fc966e80fbf
> # Parent  4ace3cd7d2d2e1d63f0811034467e7b7e9719861
> exchange: use `bookmarks` bundle2 part

exchange here is too vague, are we talking about push or pull ?

> Apply changes from `bookmarks` part.
> Initial idea was to add a special mode to the bookmarks part handler that will
> apply bookmark changes differently (create divergent bookmarks etc).
> But it would've required significant code changes, so I decided to stick
> with old approach.

Having the part handler able to handle this will be very useful for 
on-disk bundle. Can you elaborate on the difficulties you face to have 
this handled on the handler side?

> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -1370,9 +1370,13 @@
>      kwargs['cg'] = pullop.fetch
>      if 'listkeys' in pullop.remotebundle2caps:
>          kwargs['listkeys'] = ['phases']
> -        if pullop.remotebookmarks is None:
> -            # make sure to always includes bookmark data when migrating
> -            # `hg incoming --bundle` to using this function.
> +
> +    if pullop.remotebookmarks is None:
> +        # make sure to always includes bookmark data when migrating
> +        # `hg incoming --bundle` to using this function.
> +        if 'bookmarks' in pullop.remotebundle2caps:
> +            kwargs['bookmarks'] = True
> +        elif 'listkeys' in pullop.remotebundle2caps:
>              kwargs['listkeys'].append('bookmarks')
>
>      # If this is a full pull / clone and the server supports the clone bundles
> @@ -1414,6 +1418,10 @@
>              _pullapplyphases(pullop, value)
>
>      # processing bookmark update
> +    for remotebookmarks in op.records['bookmarks']:
> +        pullop.remotebookmarks = remotebookmarks
> +
> +    # processing bookmark update
>      for namespace, value in op.records['listkeys']:
>          if namespace == 'bookmarks':
>              pullop.remotebookmarks = value
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Stanislau Hlebik - Oct. 5, 2016, 9:53 a.m.
Hey.

Sorry for late response.
To run `bookmod.updatefromremote()` from part handler we need to pass url of the remote and explicitly requested bookmarks
to `bookmarks` part handler. That means that we either need to create bundleoperation() in _pullbundle2() method and
pass it to processbundle() or pass remoteurl and explicitbookmarks as parameters to processbundle. The former is fine but according to the comment in processbundle() - `this ability will probably go away in the process`, the latter adds parameters that are only related to 
`bookmarks` part.

Thanks,
Stas.

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

    
    
    On 09/16/2016 01:10 PM, Stanislau Hlebik wrote:
    > # HG changeset patch

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

    > # Date 1473954485 25200

    > #      Thu Sep 15 08:48:05 2016 -0700

    > # Node ID 837f2f1650ebf323b46924040f180fc966e80fbf

    > # Parent  4ace3cd7d2d2e1d63f0811034467e7b7e9719861

    > exchange: use `bookmarks` bundle2 part

    
    exchange here is too vague, are we talking about push or pull ?
    
    > Apply changes from `bookmarks` part.

    > Initial idea was to add a special mode to the bookmarks part handler that will

    > apply bookmark changes differently (create divergent bookmarks etc).

    > But it would've required significant code changes, so I decided to stick

    > with old approach.

    
    Having the part handler able to handle this will be very useful for 
    on-disk bundle. Can you elaborate on the difficulties you face to have 
    this handled on the handler side?
    
    > diff --git a/mercurial/exchange.py b/mercurial/exchange.py

    > --- a/mercurial/exchange.py

    > +++ b/mercurial/exchange.py

    > @@ -1370,9 +1370,13 @@

    >      kwargs['cg'] = pullop.fetch

    >      if 'listkeys' in pullop.remotebundle2caps:

    >          kwargs['listkeys'] = ['phases']

    > -        if pullop.remotebookmarks is None:

    > -            # make sure to always includes bookmark data when migrating

    > -            # `hg incoming --bundle` to using this function.

    > +

    > +    if pullop.remotebookmarks is None:

    > +        # make sure to always includes bookmark data when migrating

    > +        # `hg incoming --bundle` to using this function.

    > +        if 'bookmarks' in pullop.remotebundle2caps:

    > +            kwargs['bookmarks'] = True

    > +        elif 'listkeys' in pullop.remotebundle2caps:

    >              kwargs['listkeys'].append('bookmarks')

    >

    >      # If this is a full pull / clone and the server supports the clone bundles

    > @@ -1414,6 +1418,10 @@

    >              _pullapplyphases(pullop, value)

    >

    >      # processing bookmark update

    > +    for remotebookmarks in op.records['bookmarks']:

    > +        pullop.remotebookmarks = remotebookmarks

    > +

    > +    # processing bookmark update

    >      for namespace, value in op.records['listkeys']:

    >          if namespace == 'bookmarks':

    >              pullop.remotebookmarks = value

    > _______________________________________________

    > Mercurial-devel mailing list

    > Mercurial-devel@mercurial-scm.org

    > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DQICaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=1EQ58Dmb5uX1qHujcsT1Mg&m=fk9qadfDXhIpfXeTTV1agLi_-MdXHfIxjtrdPwM06LU&s=3YWaziuI9b5UjzfYUpnlQnKXOyV1_msZVpSaS8I_Cwo&e= 

    >

    
    -- 
    Pierre-Yves David
Pierre-Yves David - Oct. 6, 2016, 11:36 a.m.
On 10/05/2016 11:53 AM, Stanislau Hlebik wrote:
> Hey.
>
> Sorry for late response.
> To run `bookmod.updatefromremote()` from part handler we need to pass url of the remote and explicitly requested bookmarks
> to `bookmarks` part handler. That means that we either need to create bundleoperation() in _pullbundle2() method and
> pass it to processbundle() or pass remoteurl and explicitbookmarks as parameters to processbundle. The former is fine but according to the comment in processbundle() - `this ability will probably go away in the process`, the latter adds parameters that are only related to
> `bookmarks` part.

Creating the bundleoperation beforehand is fine. It is possible to do so 
to handle this kind of case. This kind of data seems to belong to the 
bundleoperation object (as they are part of the unbundle-wide context).

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1370,9 +1370,13 @@ 
     kwargs['cg'] = pullop.fetch
     if 'listkeys' in pullop.remotebundle2caps:
         kwargs['listkeys'] = ['phases']
-        if pullop.remotebookmarks is None:
-            # make sure to always includes bookmark data when migrating
-            # `hg incoming --bundle` to using this function.
+
+    if pullop.remotebookmarks is None:
+        # make sure to always includes bookmark data when migrating
+        # `hg incoming --bundle` to using this function.
+        if 'bookmarks' in pullop.remotebundle2caps:
+            kwargs['bookmarks'] = True
+        elif 'listkeys' in pullop.remotebundle2caps:
             kwargs['listkeys'].append('bookmarks')
 
     # If this is a full pull / clone and the server supports the clone bundles
@@ -1414,6 +1418,10 @@ 
             _pullapplyphases(pullop, value)
 
     # processing bookmark update
+    for remotebookmarks in op.records['bookmarks']:
+        pullop.remotebookmarks = remotebookmarks
+
+    # processing bookmark update
     for namespace, value in op.records['listkeys']:
         if namespace == 'bookmarks':
             pullop.remotebookmarks = value