[PATCH evolve-ext-V3] inhibit: direct access with and without warning on a per command basis

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue May 12 17:03:48 CDT 2015



On 05/12/2015 02:11 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon at fb.com>
> # Date 1431463949 25200
> #      Tue May 12 13:52:29 2015 -0700
> # Node ID 5abc61cbb35e368185a535444d1d67f3651a7109
> # Parent  5e82d78f5872c9503d7b6691c594a13794a9b4a4
> inhibit: direct access with and without warning on a per command basis
>
> We introduce a new filtername visibile-directaccess-nowarn to enable direct
> access with no warning on a per command basis.
> The motivation behing this change is to display warning when attempting direct
> access in destructive commands.
>
> diff --git a/hgext/inhibit.py b/hgext/inhibit.py
> --- a/hgext/inhibit.py
> +++ b/hgext/inhibit.py
> @@ -37,6 +37,15 @@
>   cmdtable = {}
>   command = cmdutil.command(cmdtable)
>
> +# List of commands where no warning is shown for direct access
> +directaccesslevel = [
> +    # warning or not, extension (None if core), command name
> +    (False, None, 'update'),
> +    (False, None, 'export'),
> +    (True,  'rebase', 'rebase'),
> +    (False,  'evolve', 'prune'),
> +]
> +
>   def reposetup(ui, repo):
>
>       class obsinhibitedrepo(repo.__class__):
> @@ -62,6 +71,20 @@
>       repo.__class__ = obsinhibitedrepo
>       repo._explicitaccess = set()
>
> +def setupdirectaccess():
> +    """ Add two new filtername that behave like visible to provide direct access
> +    and direct access with warning. Wraps the commands to setup direct access """
> +    visiblefilter = repoview.computehidden
> +    repoview.filtertable.update({'visible-directaccess-nowarn': visiblefilter})
> +    repoview.filtertable.update({'visible-directaccess-warn': visiblefilter})


1) As the idea to still have a level where --hidden is still needed, we 
should stop using direct wrapping of 'repoview.computehidden' and have a 
distinct function we use for the 'visible-directaccess-*' (reusing the 
visible logic + what was in the wrapping). This function is then to be 
used in 'filtertable.

2) In "mercurial/branchmap.py" there is a subsettable dictionnary that 
indicate what filter are super set of each other (for cache 
collaboration). You want to populate that so that 'visible' is marked as 
a subset of 'visible-directaccess-nowarn' and 
'visible-directaccess-nowarn' as a subset of 'visible-directaccess-warn'.


> +    for warn, ext, cmd in directaccesslevel:
> +        try:
> +            cmdtable = extensions.find(ext).cmdtable if ext else commands.table
> +            wrapper = wrapwithwarning if warn else wrapwithoutwarning
> +            extensions.wrapcommand(cmdtable, cmd, wrapper)
> +        except error.UnknownCommand:
> +            # Command not loaded, in that case we don't need to override it
> +            pass

What about using a `command in cmdtable` instead of try/except.

>   def _update(orig, ui, repo, *args, **kwargs):
>       """
> @@ -192,6 +215,22 @@
>       transaction.addpostclose('inhibitposttransaction', inhibitposttransaction)
>       return transaction
>
> +def wrapwithoutwarning(orig, *args, **kwargs):
> +    ui, repo, = args[0], args[1]
> +    if repo and repo.filtername == 'visible':
> +        object.__setattr__(repo, 'filtername',
> +                          'visible-directaccess-nowarn')
> +    k = orig(*args, **kwargs)
> +    return k

I insist that you need to retrieve the new repoview with 
'repo.filtered("visible-directaccess-nowarn")'.

For that to work, you must then pass the repo object to the orig function:

def wrapwithoutwarning(orig, ui, repo, *args, **kwargs):
     repo = repo.filtered('visible-directaccess-nowarn')
     …
    return orig(ui, repo, *args, **kwargs)

> @@ -214,6 +253,7 @@
>       extensions.wrapfunction(obsolete, 'createmarkers', _createmarkers)
>       extensions.wrapfunction(repoview, '_getdynamicblockers', _accessvisible)
>       extensions.wrapfunction(revset, 'posttreebuilthook', _posttreebuilthook)
> +    setupdirectaccess()
>       # wrap update to make sure that no obsolete commit is visible after an
>       # update
>       extensions.wrapcommand(commands.table, 'update', _update)
> @@ -250,21 +290,30 @@
>       # We extract the symbols that look like hashes and add them to the
>       # explicitaccess set
>       orig(tree, repo)
> -    if repo is not None and repo.filtername == 'visible':
> -        prelength = len(repo._explicitaccess)
> -        repo.symbols = gethashsymbols(tree)
> -        cl = repo.unfiltered().changelog
> -        for node in repo.symbols:
> -            try:
> -                node = cl._partialmatch(node)
> -            except error.LookupError:
> -                node = None
> -            if node is not None:
> -                rev = cl.rev(node)
> -                if rev not in repo.changelog:
> -                    repo._explicitaccess.add(rev)
> -        if prelength != len(repo._explicitaccess):
> -            repo.invalidatevolatilesets()
> +    if repo is not None and repo.filtername:

"repo.filtername is not None" ;-)

> +        hasdirectaccess = repo.filtername.startswith('visible-directaccess')
> +        if hasdirectaccess:

Not sure why we have two if? is this to avoid the line wrap?

> +            prelength = len(repo._explicitaccess)
> +            accessbefore = set(repo._explicitaccess)
> +            repo.symbols = gethashsymbols(tree)
> +            cl = repo.unfiltered().changelog
> +            for node in repo.symbols:
> +                try:
> +                    node = cl._partialmatch(node)
> +                except error.LookupError:
> +                    node = None
> +                if node is not None:
> +                    rev = cl.rev(node)
> +                    if rev not in repo.changelog:
> +                        repo._explicitaccess.add(rev)
> +            if prelength != len(repo._explicitaccess):
> +                if repo.filtername != 'visible-directaccess-nowarn':
> +                    unhiddencommits = repo._explicitaccess - accessbefore
> +                    repo.ui.warn( _("Warning: accessing hidden changesets %s "
> +                                    "for write operation\n") %
> +                                    (",".join([str(repo.unfiltered()[l])
> +                                        for l in unhiddencommits])))
> +                repo.invalidatevolatilesets()
>
>   @command('debugobsinhibit', [], '')
>   def cmddebugobsinhibit(ui, repo, *revs):
> diff --git a/tests/test-inhibit.t b/tests/test-inhibit.t
> --- a/tests/test-inhibit.t
> +++ b/tests/test-inhibit.t
> @@ -397,6 +397,9 @@
>     (use --hidden to access hidden revisions)
>     [255]
>     $ hg rebase -r ad78ff7d621f -r 53a94305e133 -d  2db36d8066ff
> +  Warning: accessing hidden changesets 2db36d8066ff for write operation
> +  Warning: accessing hidden changesets ad78ff7d621f for write operation
> +  Warning: accessing hidden changesets 53a94305e133 for write operation
>     rebasing 10:ad78ff7d621f "add cK"
>     rebasing 11:53a94305e133 "add cL"

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list