[PATCH V2 STABLE] serve: move hg-ssh readonly logic into hg serve

[Durham Goode]

On 4/28/17 6:27 AM, Yuya Nishihara wrote:
> On Thu, 27 Apr 2017 10:13:21 -0400, Augie Fackler wrote:
>> On Wed, Apr 26, 2017 at 06:24:52PM -0700, Durham Goode wrote:
>>> # HG changeset patch
>>> # User Durham Goode <durham at fb.com>
>>> # Date 1493255976 25200
>>> #      Wed Apr 26 18:19:36 2017 -0700
>>> # Branch stable
>>> # Node ID b1964bbc387fb53b4152f19d6e929309e3f21ac6
>>> # Parent  6e0368b6e0bb2aa5210daec091c0200583553a78
>>> serve: move hg-ssh readonly logic into hg serve
>>
>> This looks good, but I'm very hesitant to add a new feature like this
>> 4 days before a release. Does anyone feel strongly that this should be
>> in 4.2?
>
> Agreed. The feature seems good, but no need to hurry to pick it up.
>
>> +        if realcmd == 'serve' and '--read-only' in req.args:
>> +            req.args.remove('--read-only')
>
> This increases the risk of wrong command parsing. We can't say --read-only
> is always a flag.
>
>> +            req.ui.setconfig('hooks', 'pretxnopen.readonlyrejectpush',
>> +                             rejectpush, 'dispatch')
>> +            req.ui.setconfig('hooks', 'prepushkey.readonlyrejectpush',
>> +                             rejectpush, 'dispatch')
>
> And --read-only won't work as expected in command server since there are write
> operations other than push.

Are there write operations that wouldn't trigger the pretxnopen hook?

Alternatively, I could rename this flag to be 
--add-hgssh-push-blocking-hooks, remove it from the hg server command 
definition entirely (just watch for it at the dispatch level and strip 
it like I currently do). That way we can maintain the current pattern 
while letting it be a special path for hg-ssh only.

And if someone passes --add-hgssh-push-blocking-hooks to hg serve in a 
position that isn't a flag, well it'll just break for them.

Thoughts?