Patchwork [3,of,9] bundle2: add a devel option controling bundle version used for exchange

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 3, 2016, 2:54 p.m.
Message ID <2a9b825514377c8486b8.1470236088@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/16056/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 3, 2016, 2:54 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1470142101 -7200
#      Tue Aug 02 14:48:21 2016 +0200
# Node ID 2a9b825514377c8486b8dbd3dfbdd865efeba1fc
# Parent  2d61469ee8d3af037aa4e2730576777bbe12ff57
# EXP-Topic bundle2.devel
bundle2: add a devel option controling bundle version used for exchange

We need an official way to force bundle1 to be used in test. We introduce a new
option 'devel.legacy.exchange' to control this. When specified, this option
will control the list of bundle version Mercurial consider when exchanging with
a peer. Current valid value are 'bundle1' and 'bundle2'.

Using this option in all tests will allow use to remove the
'experimental.bundle2-exp' option. We will simplify the code once the
experimental option is dropped.
Gregory Szorc - Aug. 3, 2016, 3:24 p.m.
> On Aug 3, 2016, at 07:54, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1470142101 -7200
> #      Tue Aug 02 14:48:21 2016 +0200
> # Node ID 2a9b825514377c8486b8dbd3dfbdd865efeba1fc
> # Parent  2d61469ee8d3af037aa4e2730576777bbe12ff57
> # EXP-Topic bundle2.devel
> bundle2: add a devel option controling bundle version used for exchange
> 
> We need an official way to force bundle1 to be used in test. We introduce a new
> option 'devel.legacy.exchange' to control this. When specified, this option
> will control the list of bundle version Mercurial consider when exchanging with
> a peer. Current valid value are 'bundle1' and 'bundle2'.

As a follow up, you may want to hook up a way to disable the getbundle capability entirely as a way to test the ancient wire protocol commands for retrieving changegroups. I'm pretty sure our testing of those code paths is lacking.

> 
> Using this option in all tests will allow use to remove the
> 'experimental.bundle2-exp' option. We will simplify the code once the
> experimental option is dropped.
> 
> diff -r 2d61469ee8d3 -r 2a9b82551437 mercurial/exchange.py
> --- a/mercurial/exchange.py    Wed Aug 03 15:01:23 2016 +0200
> +++ b/mercurial/exchange.py    Tue Aug 02 14:48:21 2016 +0200
> @@ -260,10 +260,23 @@ def buildobsmarkerspart(bundler, markers
> def _forcebundle1(op):
>     """return true if a pull/push can use bundle2
> 
> -    Feel free to nuke this function when we drop the experimental option"""
> -    return not (op.repo.ui.configbool('experimental', 'bundle2-exp', True)
> -                and op.remote.capable('bundle2'))
> +    This function is used to allow testing of the older bundle version"""
> +    ui = op.repo.ui
> +    forcebundle1 = False
> +    # The goal is this config is to allow developper to choose the bundle
> +    # version used during exchanged. This is especially handy during test.
> +    # Value is a list of bundle version to be picked from, highest version
> +    # should be used.
> +    #
> +    # developer config: devel.legacy.exchange
> +    exchange = ui.configlist('devel', 'legacy.exchange')
> +    if not exchange:
> +        forcebundle1 = not ui.configbool('experimental', 'bundle2-exp', True)
> +        # developer config: devel.legacy.exchange
> +    else:
> +        forcebundle1 = 'bundle2' not in exchange and 'bundle1' in exchange
> 
> +    return forcebundle1 or not op.remote.capable('bundle2')
> 
> class pushoperation(object):
>     """A object that represent a single push operation
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - Aug. 3, 2016, 4:20 p.m.
On 08/03/2016 05:24 PM, Gregory Szorc wrote:
>
>
>> On Aug 3, 2016, at 07:54, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>>
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1470142101 -7200
>> #      Tue Aug 02 14:48:21 2016 +0200
>> # Node ID 2a9b825514377c8486b8dbd3dfbdd865efeba1fc
>> # Parent  2d61469ee8d3af037aa4e2730576777bbe12ff57
>> # EXP-Topic bundle2.devel
>> bundle2: add a devel option controling bundle version used for exchange
>>
>> We need an official way to force bundle1 to be used in test. We introduce a new
>> option 'devel.legacy.exchange' to control this. When specified, this option
>> will control the list of bundle version Mercurial consider when exchanging with
>> a peer. Current valid value are 'bundle1' and 'bundle2'.
>
> As a follow up, you may want to hook up a way to disable the getbundle capability entirely as a way to test the ancient wire protocol commands for retrieving changegroups. I'm pretty sure our testing of those code paths is lacking.

That is the spirit of this 'legacy.xxx' series of variable. (that I 
expect to be bikesheded a bit. I'll try to remember about this getbundle 
thingy (but also feel free to jump in). But I think we have some other 
hack somewhere to test it. However, We probably wants need some 
test-runner integrated way to test variants.

Cheers,

Patch

diff -r 2d61469ee8d3 -r 2a9b82551437 mercurial/exchange.py
--- a/mercurial/exchange.py	Wed Aug 03 15:01:23 2016 +0200
+++ b/mercurial/exchange.py	Tue Aug 02 14:48:21 2016 +0200
@@ -260,10 +260,23 @@  def buildobsmarkerspart(bundler, markers
 def _forcebundle1(op):
     """return true if a pull/push can use bundle2
 
-    Feel free to nuke this function when we drop the experimental option"""
-    return not (op.repo.ui.configbool('experimental', 'bundle2-exp', True)
-                and op.remote.capable('bundle2'))
+    This function is used to allow testing of the older bundle version"""
+    ui = op.repo.ui
+    forcebundle1 = False
+    # The goal is this config is to allow developper to choose the bundle
+    # version used during exchanged. This is especially handy during test.
+    # Value is a list of bundle version to be picked from, highest version
+    # should be used.
+    #
+    # developer config: devel.legacy.exchange
+    exchange = ui.configlist('devel', 'legacy.exchange')
+    if not exchange:
+        forcebundle1 = not ui.configbool('experimental', 'bundle2-exp', True)
+        # developer config: devel.legacy.exchange
+    else:
+        forcebundle1 = 'bundle2' not in exchange and 'bundle1' in exchange
 
+    return forcebundle1 or not op.remote.capable('bundle2')
 
 class pushoperation(object):
     """A object that represent a single push operation