[PATCH 3 of 7] transaction: add onclose hook for pre-close logic

[Gregory Szorc]

On 3/25/14, 7:19 PM, Durham Goode wrote:
> On 3/25/14 6:53 PM, "Gregory Szorc" <gregory.szorc at gmail.com> wrote:
>
>> On 3/24/14, 7:33 PM, Durham Goode wrote:
>>> # HG changeset patch
>>> # User Durham Goode <durham at fb.com>
>>> # Date 1395701867 25200
>>> #      Mon Mar 24 15:57:47 2014 -0700
>>> # Node ID c84f51f8f92e0a3db4888ac9739f43fd866cac20
>>> # Parent  08595987c5b0e8af5aa8fec4debd7260f5a79e8f
>>> transaction: add onclose hook for pre-close logic
>>>
>>> Adds an optional onclose parameter to transactions that gets called
>>> just before
>>> the transaction is committed. This allows things that build up data
>>> over the
>>> course of the transaction (like the fncache) to commit their data.
>>>
>>> diff --git a/mercurial/transaction.py b/mercurial/transaction.py
>>> --- a/mercurial/transaction.py
>>> +++ b/mercurial/transaction.py
>>> @@ -42,12 +42,14 @@
>>>        opener.unlink(journal)
>>>
>>>    class transaction(object):
>>> -    def __init__(self, report, opener, journal, after=None,
>>> createmode=None):
>>> +    def __init__(self, report, opener, journal, after=None,
>>> createmode=None,
>>> +            onclose=None):
>>>            self.count = 1
>>>            self.usages = 1
>>>            self.report = report
>>>            self.opener = opener
>>>            self.after = after
>>> +        self.onclose = onclose
>>>            self.entries = []
>>>            self.map = {}
>>>            self.journal = journal
>>> @@ -126,6 +128,9 @@
>>>        @active
>>>        def close(self):
>>>            '''commit the transaction'''
>>> +        if self.count == 1 and self.onclose:
>>> +            self.onclose()
>>> +
>>>            self.count -= 1
>>>            if self.count != 0:
>>>                return
>>
>> I don't mean to scope bloat this patch, but appending extra operations
>> to transaction semantics is very appealing to me. We (Mozilla) have some
>> extensions performing SQL against a local SQLite database, for example
>> and would like the SQL to execute in a transaction tied to the Mercurial
>> transaction.
>>
>> I was wondering if a wrappable function is the best solution here. Given
>> we're dealing with transactions and atomicity is hard, I don't believe
>> wrapping functions is the most robust solution. For example, what
>> happens when onclose() is wrapped multiple times and there is an
>> exception in function 3 of 5? Will the commits of functions 1 and 2 have
>> the opportunity to be rewound? (No they won't.)
>>
>> I was curious if you'll consider a patch that makes things a bit more
>> robust by adding a "transaction operation" API that allows custom
>> objects to be registered to a transaction. They would have methods like
>> "commit," "abort," and "rollback" that would be called during
>> corresponding transaction events.
>
> As a first step, I could add an onabort handler to transactions that's
> called at the beginning of an abort. That would give extensions a way to
> hook in to the abort phase (so functions 1 and 2 could rollback).
>
> I'd rather not add a full transaction extensibility framework to this
> patch, mainly because of the amount of design involved. (How do you
> register? Statically at uisetup time? Dynamically as the transactions are
> being passed around? How do you serialize information to make hg recover
> work?  Assume the extension will handle that bit?)  And my need to get
> this series in by the end of the week.

I think onabort would suffice for the near term. A robust solution can 
come later, if it's needed.