[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