[PATCH 2 of 6 V2] localrepo: run commithook only if transaction is closed successfully

[FUJIWARA Katsunori]

At Thu, 17 Dec 2015 02:50:50 +0900,
FUJIWARA Katsunori wrote:
> 
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> # Date 1450287754 -32400
> #      Thu Dec 17 02:42:34 2015 +0900
> # Node ID 4210557127a602a91ce9958bb957e2a32493856b
> # Parent  4c237821e543338dde6bfd395c28884f065399ec
> localrepo: run commithook only if transaction is closed successfully
> 
> Before this patch, internal function "commithook()" is executed even
> if transaction is aborted. Examination of 'changelog.hasnode()' in it
> avoids actual making "commit" hooks run at failure of transaction.
> 
> But if transaction scope is nested, transaction is still active even
> after 'transaction.close()' and may be aborted in subsequent
> processing. If failure of transaction is known, instantiation of
> 'repo.changelog' itself is useless.
> 
> This patch runs 'commithook()' only if transaction is closed
> successfully, by scheduling invocation of '_afterlock(commithook)' by
> 'transaction.addpostclose()'. This follows the style of
> 'cg1unpacker.apply()' for 'changegroup' hooks.
> 
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1616,18 +1616,21 @@ class localrepository(object):
>              bookmarks.update(self, [p1, p2], ret)
>              cctx.markcommitted(ret)
>              ms.reset()
> +
> +            def commithook(node=hex(ret), parent1=hookp1, parent2=hookp2):
> +                # hack for command that use a temporary commit (eg: histedit)
> +                # temporary commit got stripped before hook release
> +                if self.changelog.hasnode(ret):
> +                    self.hook("commit", node=node, parent1=parent1,
> +                              parent2=parent2)
> +            tr.addpostclose('run-hook-commit-%020i' % len(self.changelog),
> +                            lambda tr: self._afterlock(commithook))
> +
>              tr.close()

Oops, I should pass weakref-ed calling chain to 'tr.addpostclose()' in
this case, shouldn't I ?

Please drop this patch, if so (others in this series can intentionally
work without this).

BTW, passing 'self._refreshfilecachestats' directly to
'tr.addpostclose()' below in 'localrepository.transaction()' should be
also fixed, shouldn't it ?

    https://selenic.com/repo/hg/file/2916ebaef312/mercurial/localrepo.py#l1060


>  
>          finally:
>              lockmod.release(tr, lock, wlock)
>  
> -        def commithook(node=hex(ret), parent1=hookp1, parent2=hookp2):
> -            # hack for command that use a temporary commit (eg: histedit)
> -            # temporary commit got stripped before hook release
> -            if self.changelog.hasnode(ret):
> -                self.hook("commit", node=node, parent1=parent1,
> -                          parent2=parent2)
> -        self._afterlock(commithook)
>          return ret
>  
>      @unfilteredmethod
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

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