[PATCH 4 of 4 stable] PROOF OF CONCEPT: introduce lock validation

[Christian Ebert]

* Mads Kiilerich on Monday, May 14, 2012 at 11:08:22 +0200
> On 14/05/12 01:06, Christian Ebert wrote:
>> * Mads Kiilerich on Saturday, May 12, 2012 at 20:37:46 +0200
>>> diff --git a/hgext/keyword.py b/hgext/keyword.py
>>> --- a/hgext/keyword.py
>>> +++ b/hgext/keyword.py
>>> @@ -436,7 +436,11 @@
>>>    repo[None].add([fn])
>>>    ui.note(_('\nkeywords written to %s:\n') % fn)
>>>    ui.note(keywords)
>>> -    repo.dirstate.setbranch('demobranch')
>>> +    wlock = repo.wlock()
>>> +    try:
>>> +        repo.dirstate.setbranch('demobranch')
>>> +    finally:
>>> +        wlock.release()
>> 
>> Are you sure this is needed? I can't think of a situation where
>> the temporary kwdemo repo suffers from concurrent write access.
>> otoh, better safe than sorry.
> 
> This missing lock shows up when validating locking, and I had to spend
> time validating that the warning was correct - I don't want to do that
> every time I run the validation. It is not feasible to maintain that
> kwdemo is a low quality area of the Mercurial code base with known
> bugs.

As I said, better safe than sorry, completely fine with me. I was
just curious, not critical.

> But I really can't think of a situation where the kwdemo has any
> relevance as a built-in command at all. I think it would be more
> suitable as a .py test - and more useful for the user as a more
> verbose example in the documentation (which however already is quite
> long) or on the wiki.

You mean as a script in contrib?

The wiki currently already has a sample. And the one of the
purposes of having command or script is to actually see the
effect of the extension in its current or intended configuration
when you're inside the working directory - without actually
affecting it. Furthermore the idea was to make the barrier for
this kind of "live" inspection as low as possible to avoid nasty
surprises. But I agree that the command is not needed for the
actual functionality of the extension.

c
-- 
theatre - books - texts - movies
Black Trash Productions at home: http://www.blacktrash.org
Black Trash Productions on Facebook:
http://www.facebook.com/blacktrashproductions