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

[Durham Goode]

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.