Patchwork [3,of,4] bundle2.processbundle: let callers request default behavior

login
register
mail settings
Submitter Eric Sumner
Date Nov. 25, 2014, 6:26 p.m.
Message ID <46994128747aa4bc3057.1416939988@dev911.prn1.facebook.com>
Download mbox | patch
Permalink /patch/6857/
State Accepted
Delegated to: Pierre-Yves David
Headers show

Comments

Eric Sumner - Nov. 25, 2014, 6:26 p.m.
# HG changeset patch
# User Eric Sumner <ericsumner@fb.com>
# Date 1416873884 28800
#      Mon Nov 24 16:04:44 2014 -0800
# Node ID 46994128747aa4bc3057c7fbdbe19f4734cd48ab
# Parent  ed01cf6d09c8682dfd8292f1333bce11be02bc1f
bundle2.processbundle: let callers request default behavior

This patch series is intended to allow bundle2 push reply part handlers to
make changes to the local repository; it has been developed in parallel with
an extension that allows the server to rebase incoming changesets while applying
them.

The default transaction getter for processbundle is a private function that
raises an exception; this diff lets calling code pass None as the transaction
getter to explicitly request this default behavior.

The next diff will check a config option to determine whether to provide a
transaction to the reply bundle processor.  If one shouldn't be provided, the
code needs a way to specify that the default behavior should be used.
Augie Fackler - Dec. 2, 2014, 3:38 p.m.
On Tue, Nov 25, 2014 at 10:26:28AM -0800, Eric Sumner wrote:
> # HG changeset patch
> # User Eric Sumner <ericsumner@fb.com>
> # Date 1416873884 28800
> #      Mon Nov 24 16:04:44 2014 -0800
> # Node ID 46994128747aa4bc3057c7fbdbe19f4734cd48ab
> # Parent  ed01cf6d09c8682dfd8292f1333bce11be02bc1f
> bundle2.processbundle: let callers request default behavior
>
> This patch series is intended to allow bundle2 push reply part handlers to
> make changes to the local repository; it has been developed in parallel with
> an extension that allows the server to rebase incoming changesets while applying
> them.
>
> The default transaction getter for processbundle is a private function that
> raises an exception; this diff lets calling code pass None as the transaction
> getter to explicitly request this default behavior.
>
> The next diff will check a config option to determine whether to provide a
> transaction to the reply bundle processor.  If one shouldn't be provided, the
> code needs a way to specify that the default behavior should be used.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -277,7 +277,7 @@
>      to be created"""
>      raise TransactionUnavailable()
>
> -def processbundle(repo, unbundler, transactiongetter=_notransaction):
> +def processbundle(repo, unbundler, transactiongetter=None):

I think (but am not sure) that after this patch _notransaction is
unused and could be deleted in a followup.

>      """This function process a bundle, apply effect to/from a repo
>
>      It iterates over each part then searches for and uses the proper handling
> @@ -288,6 +288,8 @@
>
>      Unknown Mandatory part will abort the process.
>      """
> +    if transactiongetter is None:
> +        transactiongetter = _notransaction
>      op = bundleoperation(repo, transactiongetter)
>      # todo:
>      # - replace this is a init function soon.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Dec. 2, 2014, 8:30 p.m.
On 12/02/2014 07:38 AM, Augie Fackler wrote:
> On Tue, Nov 25, 2014 at 10:26:28AM -0800, Eric Sumner wrote:
>> # HG changeset patch
>> # User Eric Sumner <ericsumner@fb.com>
>> # Date 1416873884 28800
>> #      Mon Nov 24 16:04:44 2014 -0800
>> # Node ID 46994128747aa4bc3057c7fbdbe19f4734cd48ab
>> # Parent  ed01cf6d09c8682dfd8292f1333bce11be02bc1f
>> bundle2.processbundle: let callers request default behavior
>>
>> This patch series is intended to allow bundle2 push reply part handlers to
>> make changes to the local repository; it has been developed in parallel with
>> an extension that allows the server to rebase incoming changesets while applying
>> them.
>>
>> The default transaction getter for processbundle is a private function that
>> raises an exception; this diff lets calling code pass None as the transaction
>> getter to explicitly request this default behavior.
>>
>> The next diff will check a config option to determine whether to provide a
>> transaction to the reply bundle processor.  If one shouldn't be provided, the
>> code needs a way to specify that the default behavior should be used.
>>
>> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>> --- a/mercurial/bundle2.py
>> +++ b/mercurial/bundle2.py
>> @@ -277,7 +277,7 @@
>>       to be created"""
>>       raise TransactionUnavailable()
>>
>> -def processbundle(repo, unbundler, transactiongetter=_notransaction):
>> +def processbundle(repo, unbundler, transactiongetter=None):
>
> I think (but am not sure) that after this patch _notransaction is
> unused and could be deleted in a followup.

It is used in the next chunk…

>> @@ -288,6 +288,8 @@
>>
>>       Unknown Mandatory part will abort the process.
>>       """
>> +    if transactiongetter is None:
>> +        transactiongetter = _notransaction
>>       op = bundleoperation(repo, transactiongetter)
>>       # todo:
>>       # - replace this is a init function soon.
Augie Fackler - Dec. 2, 2014, 8:41 p.m.
On Tue, Dec 2, 2014 at 3:30 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 12/02/2014 07:38 AM, Augie Fackler wrote:
>>
>> On Tue, Nov 25, 2014 at 10:26:28AM -0800, Eric Sumner wrote:
>>>
>>> # HG changeset patch
>>> # User Eric Sumner <ericsumner@fb.com>
>>> # Date 1416873884 28800
>>> #      Mon Nov 24 16:04:44 2014 -0800
>>> # Node ID 46994128747aa4bc3057c7fbdbe19f4734cd48ab
>>> # Parent  ed01cf6d09c8682dfd8292f1333bce11be02bc1f
>>> bundle2.processbundle: let callers request default behavior
>>>
>>> This patch series is intended to allow bundle2 push reply part handlers
>>> to
>>> make changes to the local repository; it has been developed in parallel
>>> with
>>> an extension that allows the server to rebase incoming changesets while
>>> applying
>>> them.
>>>
>>> The default transaction getter for processbundle is a private function
>>> that
>>> raises an exception; this diff lets calling code pass None as the
>>> transaction
>>> getter to explicitly request this default behavior.
>>>
>>> The next diff will check a config option to determine whether to provide
>>> a
>>> transaction to the reply bundle processor.  If one shouldn't be provided,
>>> the
>>> code needs a way to specify that the default behavior should be used.
>>>
>>> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>>> --- a/mercurial/bundle2.py
>>> +++ b/mercurial/bundle2.py
>>> @@ -277,7 +277,7 @@
>>>       to be created"""
>>>       raise TransactionUnavailable()
>>>
>>> -def processbundle(repo, unbundler, transactiongetter=_notransaction):
>>> +def processbundle(repo, unbundler, transactiongetter=None):
>>
>>
>> I think (but am not sure) that after this patch _notransaction is
>> unused and could be deleted in a followup.
>
>
> It is used in the next chunk…


That'll inspire in my code reviews for today. Sigh.

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -277,7 +277,7 @@ 
     to be created"""
     raise TransactionUnavailable()
 
-def processbundle(repo, unbundler, transactiongetter=_notransaction):
+def processbundle(repo, unbundler, transactiongetter=None):
     """This function process a bundle, apply effect to/from a repo
 
     It iterates over each part then searches for and uses the proper handling
@@ -288,6 +288,8 @@ 
 
     Unknown Mandatory part will abort the process.
     """
+    if transactiongetter is None:
+        transactiongetter = _notransaction
     op = bundleoperation(repo, transactiongetter)
     # todo:
     # - replace this is a init function soon.