[PATCH 2 of 2 v2] acl: support revsets [RFC]

Pierre-Yves David pierre-yves.david at ens-lyon.org
Wed May 11 08:33:33 EDT 2016



On 05/11/2016 02:23 PM, Augie Fackler wrote:
>> On May 11, 2016, at 6:12 AM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>
>>>> Afaict, it does not help w/ bookmarks at all,
>>>> as bookmarks aren't applied until after the hook runs.
>>>> And I don't think one can roll back a bookmark by that point,
>>>> as I think the previous information would be lost.
>>> Hm, you might well be right. Sounds like maybe we should consider both
>>> patches? Being able to use revsets here seems about right to me feel-wise.
>> This series creates the following though flow on my side:
>>
>> 1) Why do user need to setup hook by hand that seems like something acl should do by itself
> I don’t know what this means.

If you look at patch 1 you can see that in addition to adding the acl 
extension, the config need to be updated:

    $ echo 'pretxnchangegroup.acl = python:hgext.acl.hook' >> $config
    $ echo 'prepushkey.acl = python:hgext.acl.hook' >> $config


>> 2) Bookmark handling would be better/safer at the transaction level, but we don't have good hook data at that point
> I think the problem is not the transaction-level business for bookmarks, but instead the fact that old clients send bookmarks in a separate transaction.

No, the problem is that we cannot implement that hook at the pretnxclose 
level, because the data of which hook is not available here. This is an 
issue that need to be tackled at some point anyway. But doing the a 
pretnxclose hook is the right point to handle acl request as this is 
when we'll be able to have a view of the full operation.

The fact old client use a different transaction is independant and 
"unsolvable" but have no effect on the approach used here.

>
>> 3) If we improve hooks used, user will have to adjust config to take advantage of it (because of (1))
>>
>> This create a tickling temptation to rework acl to register hooks and improve transaction to track bookmark event before moving forward here.
> I understand that temptation, but I think that’s out of scope for what this series is trying to accomplish (and not entirely feasible anyway with old clients potentially in play.)

But I'm not super eager to move forward on this bookmark things with 
such a brittle implementation, especially, fixes to issue5165 
<https://bz.mercurial-scm.org/show_bug.cgi?id=5165> might break the 
current approach.

as a remindier, changing the hooks used will not have any change on the 
challenge.


More information about the Mercurial-devel mailing list