[PATCH] transaction: support for callbacks during abort

[FUJIWARA Katsunori]

At Thu, 13 Aug 2015 12:17:22 +0900,
FUJIWARA Katsunori wrote:
> 
> 
> At Sat, 18 Jul 2015 01:48:24 +0200,
> Pierre-Yves David wrote:
> > 
> > On 07/16/2015 04:43 PM, FUJIWARA Katsunori wrote:
> > >
> > > At Tue, 14 Jul 2015 13:26:14 +0200,
> > > Pierre-Yves David wrote:
> > >>
> > >> On 07/13/2015 07:44 AM, FUJIWARA Katsunori wrote:
> > >>>
> > >>>
> > >>> At Sat, 11 Jul 2015 20:47:00 +0100,
> > >>> Pierre-Yves David wrote:
> > >>>
> > >>>> On 07/11/2015 11:31 AM, FUJIWARA Katsunori wrote:
> > >>>>> Now, I'm working for dirstate consistency around inside and border of
> > >>>>> transaction, and planning to use this abort callback mechanism to know
> > >>>>> the end of transaction at failure.
> > >>>>
> > >>>> I'm a bit curious about what code you need to run in such case. If you
> > >>>> use the file generator mechanism you do not need to worries about
> > >>>> restoring backup.
> > >>>>
> > >>>> However, I'm not sure how rolling back the file would atomatically
> > >>>> invalide the in memory object content (probably not). Is this why you
> > >>>> need a callback? could we (would it make sense to) integrate that as
> > >>>> part of one of our current cache invalidation mechanism?
> > >>>
> > >>>
> > >>> I'm planning to use callback mechanism for purposes below:
> > >>>
> > >>> 1. to make existing 'dirstate.write()' invocations write in-memory
> > >>>      changes into appropriate file ('.hg/dirstate' or '.hg/dirstate.pending'),
> > >>>      'dirstate.write()' should know whether transaction is running or not
> > >>>
> > >>>      I'm planning changes below for this purpose:
> > >>>
> > >>>      - invoke (newly added) 'dirstate.begintransaction()' at the
> > >>>        beginning of transaction in 'localrepo.transaction()'
> > >>>
> > >>>      - invoke (newly added) 'dirstate.endtransaction()' at the end of
> > >>>        transaction via "the end of transaction" callback (instead of
> > >>>        finalize/abort callbacks)
> > >>>
> > >>>
> > >>>      There are some other ways:
> > >>>
> > >>>      1-1. instantiate dirstate with 'repo'
> > >>>           (or weak ref of repo to avoid circular reference ?)
> > >>>
> > >>>      1-2. make 'dirstate.write()' require 'localrepo' argument
> > >>>
> > >>>           This seems too large impact (especially for 3rd party
> > >>>           extensions, even though Mercurial doesn't ensure compatibility
> > >>>           of internal API at all :-))
> > >>>
> > >>>      In both cases, 'dirstate.write()' can determine appropriate output
> > >>>      file by examination of 'repo.currenttransaction()'.
> > >>
> > >> I would prefer one of this options. Using hooks for such core code looks
> > >> a bit hacky and fragile.
> > >
> > > How about something new like 'unlock' of 'lock' (or 'after' of
> > > transaction) ? Would you think this kind of 'call back' mechanism
> > > itself is also fragile for core code (if newly added) ?
> > 
> > I do not understand what you mean.
> > 
> > I'm worried about hook approach being "fragile" because it dirstate 
> > defines very important data for the understanding of the repo. Having it 
> > a the same place as other extensions and config hook will probably lead 
> > to some ordering hell.
> 
> (Oops, 'unlock' is just a name of a local variable in
                                      ^^^^^^^^^^^^^^
> 'localrepo.lock()'.  Correct name of constructor argument is
> 'releasefn')

"local variable" => "local function"

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp