[PATCH evolve-ext] 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 14:34:37 CDT 2015



On 05/11/2015 04:56 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon at fb.com>
> # Date 1428086461 25200
> #      Fri Apr 03 11:41:01 2015 -0700
> # Node ID 23a0b73d771fe6b675e52f000ffb47768c4ad474
> # 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,10 @@
>   cmdtable = {}
>   command = cmdutil.command(cmdtable)
>
> +# List of commands where no warning is shown for direct access
> +directaccessnowarning = ['update', 'export']
> +directaccessnowarningevolve = ['prune']

We will need to do that for an arbitrary number of extensions.

I suggest the following format:

directaccesslevel = [
     (False, None. 'update'), # no warning, core command, command name
     (True,  'rebase'. 'rebase'), # warning, rebase extensions, command name
]

remember we have three level:

- no direct access (suggestion: not in the list)
- direct access with warning (suggestion: first item to True)
- direct access without warning (sugguestion: first item to False)


The current suggestion is conservative, we can changes the behavior once 
we have all in place.

There is also obvious missing commands in my and your list. But it is 
okay, lets focus on the mechanism first them work on the list content.

>   def reposetup(ui, repo):
>
>       class obsinhibitedrepo(repo.__class__):
> @@ -62,7 +66,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})
> +    for cmd in directaccessnowarning:
> +        extensions.wrapcommand(commands.table, cmd, wrapwithoutwarning)
> +        evolve = extensions.find("evolve")
> +    for cmd in directaccessnowarningevolve:
> +        try:
> +            extensions.wrapcommand(evolve.cmdtable, cmd, wrapwithoutwarning)
> +        except error.UnknownCommand:
> +            # Command not loaded, in that case we don't need to override it
> +            pass
>   def _update(orig, ui, repo, *args, **kwargs):
>       """
>       When moving to a commit we want to inhibit any obsolete commit affecting
> @@ -192,6 +209,14 @@
>       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

eeerrrk, We should not change the filtername in flight.

use repo = repo.filtered('visible-directaccess-nowarn')


> +
>   def extsetup(ui):
>       # lets wrap the computation of the obsolete set
>       # We apply inhibition there
> @@ -214,6 +239,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,8 +276,10 @@
>       # 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':
> +    if repo and repo.filtername and \
> +        repo.filtername.startswith('visible'):

1) if you want to check against None, explicitly check "is not None".
2) we also only want allow unfiltered access for filtername that starts 
with 'visible-directaccess' to ensure we can have stuff like push and 
pull still requires --hidden
3) do not use \ for line continuation it is hard to read and error 
prone. use either parentheses or temporary variable.

>           prelength = len(repo._explicitaccess)
> +        accessbefore = set(repo._explicitaccess)
>           repo.symbols = gethashsymbols(tree)
>           cl = repo.unfiltered().changelog
>           for node in repo.symbols:
> @@ -264,6 +292,12 @@
>                   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(l) for l in unhiddencommits])))
> +

We should display hash because people used hash to refer to these commit

>               repo.invalidatevolatilesets()
>
>   @command('debugobsinhibit', [], '')
> 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 3 for write operation
> +  Warning: accessing hidden changesets 10 for write operation
> +  Warning: accessing hidden changesets 11 for write operation

(We should display hash because people used hash to refer to these commit)

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list