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

Laurent Charignon lcharignon at fb.com
Tue May 12 14:46:45 CDT 2015



On 5/12/15, 12:34 PM, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org>
wrote:

>
>
>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
>]

Ok, looks good
>
>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)

You suggested a week ago to change it to two levels:
- direct access without-warning applied to a list of commands "whitelisted"
- visible (stays as is but provides direct access with warning)

Do you want to go back to the three levels of the previous patch?

>
>
>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')

Ok
>
>
>> +
>>   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

See V2 sent right after
>
>>               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)

idem
>
>-- 
>Pierre-Yves David



More information about the Mercurial-devel mailing list