Patchwork [5,of,9,V6] bookmarks: add srchex param to updatefromremote

login
register
mail settings
Submitter Stanislau Hlebik
Date Oct. 11, 2016, 4:25 p.m.
Message ID <f781756b8de11a6f3e7d.1476203147@dev1918.lla1.facebook.com>
Download mbox | patch
Permalink /patch/17036/
State Changes Requested
Delegated to: Pierre-Yves David
Headers show

Comments

Stanislau Hlebik - Oct. 11, 2016, 4:25 p.m.
# HG changeset patch
# User Stanislau Hlebik <stash@fb.com>
# Date 1476197429 25200
#      Tue Oct 11 07:50:29 2016 -0700
# Node ID f781756b8de11a6f3e7dd5fd6354e9778defd8c3
# Parent  718ed86a3698631077a087efaf668d70513056f5
bookmarks: add srchex param to updatefromremote
Pierre-Yves David - Oct. 14, 2016, 1:28 a.m.
On 10/11/2016 06:25 PM, Stanislau Hlebik wrote:
> # HG changeset patch
> # User Stanislau Hlebik <stash@fb.com>
> # Date 1476197429 25200
> #      Tue Oct 11 07:50:29 2016 -0700
> # Node ID f781756b8de11a6f3e7dd5fd6354e9778defd8c3
> # Parent  718ed86a3698631077a087efaf668d70513056f5
> bookmarks: add srchex param to updatefromremote

I do not understand what the purpose and effect of this changeset is. 
Can you elaborate a little ?

>
> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> --- a/mercurial/bookmarks.py
> +++ b/mercurial/bookmarks.py
> @@ -508,11 +508,12 @@
>
>      return None
>
> -def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=()):
> +def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=(),
> +                     srchex=None):
>      ui.debug("checking for updated bookmarks\n")
>      localmarks = repo._bookmarks
>      (addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same
> -     ) = compare(repo, remotemarks, localmarks, dsthex=hex)
> +     ) = compare(repo, remotemarks, localmarks, srchex=srchex, dsthex=hex)
>
>      status = ui.status
>      warn = ui.warn
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Stanislau Hlebik - Nov. 2, 2016, 11:08 a.m.
I’m using srchex in bookmarks part handler to convert from bin nodes to hex nodes, because updatefromremote expect hex nodes.

On 10/14/16, 2:28 AM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:

    
    
    On 10/11/2016 06:25 PM, Stanislau Hlebik wrote:
    > # HG changeset patch

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

    > # Date 1476197429 25200

    > #      Tue Oct 11 07:50:29 2016 -0700

    > # Node ID f781756b8de11a6f3e7dd5fd6354e9778defd8c3

    > # Parent  718ed86a3698631077a087efaf668d70513056f5

    > bookmarks: add srchex param to updatefromremote

    
    I do not understand what the purpose and effect of this changeset is. 
    Can you elaborate a little ?
    
    >

    > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py

    > --- a/mercurial/bookmarks.py

    > +++ b/mercurial/bookmarks.py

    > @@ -508,11 +508,12 @@

    >

    >      return None

    >

    > -def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=()):

    > +def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=(),

    > +                     srchex=None):

    >      ui.debug("checking for updated bookmarks\n")

    >      localmarks = repo._bookmarks

    >      (addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same

    > -     ) = compare(repo, remotemarks, localmarks, dsthex=hex)

    > +     ) = compare(repo, remotemarks, localmarks, srchex=srchex, dsthex=hex)

    >

    >      status = ui.status

    >      warn = ui.warn

    > _______________________________________________

    > 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=8-TOccCZw4oFT0da5cQ-wA28AAvE6qKjZzPGBL8t-1k&s=V5DeajSgRhOwuad7CFu_QgJzUJxWLtqkhpa1BOyyA-k&e= 

    >

    
    -- 
    Pierre-Yves David
Pierre-Yves David - Nov. 10, 2016, 5:37 p.m.
On 11/02/2016 11:08 AM, Stanislau Hlebik wrote:
> I’m using srchex in bookmarks part handler to convert from bin nodes to hex nodes, because updatefromremote expect hex nodes.

This sounds wrong to me, low lever representation of node is binary, 
storage is binary, the network is now able to exchange it as binary. We 
should have the function along the path handle binary, old network 
protocol using hex will convert to binary themselve.

> On 10/14/16, 2:28 AM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:
>
>
>
>     On 10/11/2016 06:25 PM, Stanislau Hlebik wrote:
>     > # HG changeset patch
>     > # User Stanislau Hlebik <stash@fb.com>
>     > # Date 1476197429 25200
>     > #      Tue Oct 11 07:50:29 2016 -0700
>     > # Node ID f781756b8de11a6f3e7dd5fd6354e9778defd8c3
>     > # Parent  718ed86a3698631077a087efaf668d70513056f5
>     > bookmarks: add srchex param to updatefromremote
>
>     I do not understand what the purpose and effect of this changeset is.
>     Can you elaborate a little ?
>
>     >
>     > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
>     > --- a/mercurial/bookmarks.py
>     > +++ b/mercurial/bookmarks.py
>     > @@ -508,11 +508,12 @@
>     >
>     >      return None
>     >
>     > -def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=()):
>     > +def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=(),
>     > +                     srchex=None):
>     >      ui.debug("checking for updated bookmarks\n")
>     >      localmarks = repo._bookmarks
>     >      (addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same
>     > -     ) = compare(repo, remotemarks, localmarks, dsthex=hex)
>     > +     ) = compare(repo, remotemarks, localmarks, srchex=srchex, dsthex=hex)
>     >
>     >      status = ui.status
>     >      warn = ui.warn
>     > _______________________________________________
>     > 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=8-TOccCZw4oFT0da5cQ-wA28AAvE6qKjZzPGBL8t-1k&s=V5DeajSgRhOwuad7CFu_QgJzUJxWLtqkhpa1BOyyA-k&e=
>     >
>
>     --
>     Pierre-Yves David
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Stanislau Hlebik - Nov. 11, 2016, 11:29 a.m.
Not sure I fully understand your idea.
Do you suggest to rewrite bookmarks.compare and bookmarks.updatefromremote to accept only binary nodes?

On 11/10/16, 5:37 PM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:

    
    
    On 11/02/2016 11:08 AM, Stanislau Hlebik wrote:
    > I’m using srchex in bookmarks part handler to convert from bin nodes to hex nodes, because updatefromremote expect hex nodes.

    
    This sounds wrong to me, low lever representation of node is binary, 
    storage is binary, the network is now able to exchange it as binary. We 
    should have the function along the path handle binary, old network 
    protocol using hex will convert to binary themselve.
    
    > On 10/14/16, 2:28 AM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:

    >

    >

    >

    >     On 10/11/2016 06:25 PM, Stanislau Hlebik wrote:

    >     > # HG changeset patch

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

    >     > # Date 1476197429 25200

    >     > #      Tue Oct 11 07:50:29 2016 -0700

    >     > # Node ID f781756b8de11a6f3e7dd5fd6354e9778defd8c3

    >     > # Parent  718ed86a3698631077a087efaf668d70513056f5

    >     > bookmarks: add srchex param to updatefromremote

    >

    >     I do not understand what the purpose and effect of this changeset is.

    >     Can you elaborate a little ?

    >

    >     >

    >     > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py

    >     > --- a/mercurial/bookmarks.py

    >     > +++ b/mercurial/bookmarks.py

    >     > @@ -508,11 +508,12 @@

    >     >

    >     >      return None

    >     >

    >     > -def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=()):

    >     > +def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=(),

    >     > +                     srchex=None):

    >     >      ui.debug("checking for updated bookmarks\n")

    >     >      localmarks = repo._bookmarks

    >     >      (addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same

    >     > -     ) = compare(repo, remotemarks, localmarks, dsthex=hex)

    >     > +     ) = compare(repo, remotemarks, localmarks, srchex=srchex, dsthex=hex)

    >     >

    >     >      status = ui.status

    >     >      warn = ui.warn

    >     > _______________________________________________

    >     > 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=8-TOccCZw4oFT0da5cQ-wA28AAvE6qKjZzPGBL8t-1k&s=V5DeajSgRhOwuad7CFu_QgJzUJxWLtqkhpa1BOyyA-k&e=

    >     >

    >

    >     --

    >     Pierre-Yves David

    >

    >

    > _______________________________________________

    > 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=DQIDaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=1EQ58Dmb5uX1qHujcsT1Mg&m=1J4zA7UH314_fpivdIc-MtVNMMrDFnFrMs3GQgAriME&s=shmUZvEsh8vOpaMl3fpSo83yBZlgnswdngRwTg1gXAQ&e= 

    >

    
    -- 
    Pierre-Yves David
Pierre-Yves David - Nov. 11, 2016, 11:59 a.m.
On 11/11/2016 11:29 AM, Stanislau Hlebik wrote:
> Not sure I fully understand your idea.
> Do you suggest to rewrite bookmarks.compare and bookmarks.updatefromremote to accept only binary nodes?

Yes, it would like it would make more sense.

If I understand your series correctly, we otherwise get in a situation 
where a caller with the binary data has to convert them to hex and the 
function internal have to convert thing again because internal storage 
is node. If everybody were talking node the situation would be simpler.

Does this make sense ?
Stanislau Hlebik - Nov. 11, 2016, 12:01 p.m.
That should be fine

On 11/11/16, 11:59 AM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:

    
    
    On 11/11/2016 11:29 AM, Stanislau Hlebik wrote:
    > Not sure I fully understand your idea.

    > Do you suggest to rewrite bookmarks.compare and bookmarks.updatefromremote to accept only binary nodes?

    
    Yes, it would like it would make more sense.
    
    If I understand your series correctly, we otherwise get in a situation 
    where a caller with the binary data has to convert them to hex and the 
    function internal have to convert thing again because internal storage 
    is node. If everybody were talking node the situation would be simpler.
    
    Does this make sense ?
    
    -- 
    Pierre-Yves David

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -508,11 +508,12 @@ 
 
     return None
 
-def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=()):
+def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=(),
+                     srchex=None):
     ui.debug("checking for updated bookmarks\n")
     localmarks = repo._bookmarks
     (addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same
-     ) = compare(repo, remotemarks, localmarks, dsthex=hex)
+     ) = compare(repo, remotemarks, localmarks, srchex=srchex, dsthex=hex)
 
     status = ui.status
     warn = ui.warn