[PATCH 3 of 4 evolve-ext] inhibit: don't leave any obsolete commit visible after closing transaction

[Pierre-Yves David]

On 04/02/2015 07:09 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon at fb.com>
> # Date 1427921481 25200
> #      Wed Apr 01 13:51:21 2015 -0700
> # Node ID efcfb550ae9a38972af97d25c841b1d1d132f499
> # Parent  89b46ecc6bcfec588667eec38e6ce85ea002b82b
> inhibit: don't leave any obsolete commit visible after closing transaction
>
> We add a callback to wrap the transaction closure to identify commits that are
> obsolete and visible. We inhibit them to prevent the user to see instability in the UI.
>
> diff --git a/hgext/inhibit.py b/hgext/inhibit.py
> --- a/hgext/inhibit.py
> +++ b/hgext/inhibit.py
> @@ -21,6 +21,7 @@
>   to XXX.
>   """
>   from mercurial import localrepo
> +from mercurial import transaction

unused import ?

>   from mercurial import obsolete
>   from mercurial import extensions
>   from mercurial import cmdutil
> @@ -47,6 +48,10 @@
>                   obsinhibit.add(raw[i:i+20])
>               return obsinhibit
>
> +    # Wrapping this to inhibit obsolete revs resulting from a transaction
> +    extensions.wrapfunction(localrepo.localrepository,
> +                            'transaction', transactioncallback)
> +
>       repo.__class__ = obsinhibitedrepo
>       repo._explicitaccess = set()
>
> @@ -136,6 +141,20 @@
>       finally:
>           tr.release()
>
> +
> +def transactioncallback(orig, repo, *args, **kwargs):
> +    """ This wraps localrepo.transaction to inhibit any obsolete changes
> +    resulting from any transaction """

small nits: python docstring are usually in the form

   short summary

   Longer explanation
   on multiple line

(just like commit message actually ;-) )

> +    def callback(transaction):

Small nits: callback is going to be a terrible name in a traceback. 
Using something more informative will give more clue to the poor soul 
that will get to debug something in there.
(no need to change it, just pointing that out for next time)

> +        """ At the end of the transaction we catch all the new visible and
> +        obsolete commit to inhibit them """
> +        visibleobsolete = repo.revs('(not hidden()) and obsolete()')
> +        if visibleobsolete:
> +            _inhibitmarkers(repo, [repo[r].node() for r in visibleobsolete])
> +    transaction = orig(repo, *args, **kwargs)
> +    transaction.addpostclose('inhibitposttransaction', callback)

I though you had some specific needs in terms of ordering?

-- 
Pierre-Yves David