[PATCH] {RFC} commands: switch to ctxmanager in phase

[Pierre-Yves David]

On 01/16/2016 04:15 AM, Yuya Nishihara wrote:
> A couple of issues found in "use context manager" series, in clowncopter.
>
> http://hg.netv6.net/clowncopter/rev/5f2bd1d7b292
>
>> --- a/mercurial/context.py	Fri Jan 15 13:14:46 2016 -0800
>> +++ b/mercurial/context.py	Fri Jan 15 13:14:46 2016 -0800
>> @@ -1548,17 +1548,15 @@
>>                   # so we don't wait on the lock
>>                   # wlock can invalidate the dirstate, so cache normal _after_
>>                   # taking the lock
>> -                wlock = self._repo.wlock(False)
>>                   normal = self._repo.dirstate.normal
>> -                try:
>> +                with self._repo.wlock(False):
>
> wlock must be taken before caching normal. See
> https://selenic.com/repo/hg/rev/48e32c2c499b

Nice catch, I should have spotted that. I'm not sure what the status of 
that series in Matt hands. We sould send a fix once that status is clearer.


> http://hg.netv6.net/clowncopter/rev/adf95f8332e6
>
>> --- a/mercurial/context.py	Fri Jan 15 13:14:46 2016 -0800
>> +++ b/mercurial/context.py	Fri Jan 15 13:14:46 2016 -0800
>> @@ -1415,9 +1415,8 @@
>>
>>       def add(self, list, prefix=""):
>>           join = lambda f: os.path.join(prefix, f)
>> -        wlock = self._repo.wlock()
>>           ui, ds = self._repo.ui, self._repo.dirstate
>> -        try:
>> +        with self._repo.wlock():
>
> Perhaps dirstate should be cached after taking wlock.

ditto.


>
>
> http://hg.netv6.net/clowncopter/rev/5eaee365f782
>
>> @@ -258,14 +257,12 @@
>>
>>   class histeditstate(object):
>>       def __init__(self, repo, parentctxnode=None, actions=None, keep=None,
>> -            topmost=None, replacements=None, lock=None, wlock=None):
>> +            topmost=None, replacements=None):
>>           self.repo = repo
>>           self.actions = actions
>>           self.keep = keep
>>           self.topmost = topmost
>>           self.parentctxnode = parentctxnode
>> -        self.lock = lock
>> -        self.wlock = wlock
>
> These variables are planned to be used?
>
> https://selenic.com/repo/hg/rev/e0b5f5e3afe8

Without a comment explaining why we have these unused variable, there is 
few chance that they survive. We should reinstall them with a such comment.

Cheers,

-- 
Pierre-Yves David