[PATCH 1 of 2 STABLE] mq: qpush fails with infinite recursion in _findtags when status file is wrong (issue2664)
Tuukka Tolvanen
tuukka.tolvanen at gmail.com
Mon Feb 28 12:04:32 CST 2011
28.2.2011 10:07, Matt Mackall kirjoitti:
> On Mon, 2011-02-28 at 08:57 +0200, timeless wrote:
>> On Mon, 2011-02-28 at 06:23 +0200, timeless wrote:
>>> i did:
>>>
>>> d) add a test which will prevent people from doing it again (if they run tests).
>>
>> On Mon, Feb 28, 2011 at 8:29 AM, Matt Mackall<mpm at selenic.com> wrote:
>>> Oops, so you did.
>>>
>>> Ok, I guess the ball is in my court to fix this all up.
>>
>> yeah, sorry. i managed a bad workaround (not submitted) for the crash.
>> tuukka tracked down the regression and suggested the backout as a
>> solution (which was much more elegant than my fix -- although
>> admittedly not ideal).
>>
>> as to oscillation, your commit message was "mq: avoid using
>> revlog.nodemap unnecessarily". which to me seemed like an attempt at
>> an optimization instead of some other bug fix.
>
> Indeed, a very successful optimization attempt except for your
> interesting test case.
Re optimization -- I can't tell offhand what the caveats for cl.rev are
exactly vs checking cl.nodemap, but the first fix (for this testcase,
and other tests pass) before finding the regressing cset was to take an
exception instead of the check that looks potentially redundant here:
- if qbasenode not in self:
+ try:
+ qbase = cl.rev(qbasenode)
+ except LookupError:
self.ui.warn(_('mq status file refers to unknown node
%s\n')
% short(qbasenode))
return super(mqrepo, self)._branchtags(partial, lrev)
- qbase = cl.rev(qbasenode)
(That doesn't address the other part, _findtags called from __contains__
potentially calling __contains__ recursively, but it won't get hit by
this testcase.)
't.
More information about the Mercurial-devel
mailing list