[PATCH] journal: properly check for held lock (issue5349)

[Pierre-Yves David]

On 09/13/2016 08:54 PM, Augie Fackler wrote:
> On Tue, Sep 13, 2016 at 08:39:30PM +0200, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>> # Date 1473791419 -7200
>> #      Tue Sep 13 20:30:19 2016 +0200
>> # Node ID 61784f683c494f547565122716bdb2234d5360ae
>> # Parent  be16091ac14d03f3cc038b2fb26efe46f785f8d7
>> # EXP-Topic pypy.journal
>> journal: properly check for held lock (issue5349)
>
> Queued, but please see a comment below.
>
> [...]
>
>> diff --git a/hgext/journal.py b/hgext/journal.py
>> --- a/hgext/journal.py
>> +++ b/hgext/journal.py
>> @@ -267,9 +267,21 @@ class journalstorage(object):
>>          # with a non-local repo (cloning for example).
>>          cls._currentcommand = fullargs
>>
>> +    def _currentlock(self, lockref):
>
> Why is this method inside the journalstorage class? Could it not be at
> module-level and therefore invite fewer questions about its
> implementation when being read?
>
> (It also strikes me this is probably a pattern we need to employ on
> other weakrefs to locks, so there's probably room for some sort of
> lockutil package eventually.)

This is exactly copied from the local repository class (So it is already 
employ for other weakrefs to core locks).
You are right,  It could (and most probably should) be a utility 
function instead of an object method. This time I went for the 
sub-optimal but consistent codebase instead of doing the cleanup detour. 
I was mostly aiming at getting the pypy test  green again using the 
shortest path as they were failing for over a month without much 
reaction from people with ownership of pypy or journal code.

Cheers,

-- 
Pierre-Yves David