# HG changeset patch # User Georges Racinet # Date 2019-12-05 19:41:23 # Node ID 49fa0b31ee1d84c44eab951f36fb7386e3fced22 # Parent d5ce99a6db52a858ea850206af39886dc597ab11 cext-revlog: fixed __delitem__ for uninitialized nodetree This is a bug in a code path that's seldom used, because in practice (at least in the whole test suite), calls to `del index[i:j]` currently just don't happen before the nodetree has been initialized. However, in our current work to replace the nodetree by a Rust implementation, this is of course systematic. In `index_slice_del()`, if the slice start is smaller than `self->length`, the whole of `self->added` has to be cleared. Before this change, the clearing was done only by the call to `index_invalidate_added(self, 0)`, that happens only for initialized nodetrees. Hence the removal was effective only from `start` to `self->length`. The consequence is index corruption, with bogus results in subsequent calls, and in particular errors such as `ValueError("parent out of range")`, due to the fact that parents of entries in `self->added` are now just invalid. This is detected by the rebase tests, under conditions that the nodetree of revlog.c is never initialized. The provided specific test is more direct. Differential Revision: https://phab.mercurial-scm.org/D7603 diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c --- a/mercurial/cext/revlog.c +++ b/mercurial/cext/revlog.c @@ -2522,7 +2522,10 @@ static int index_slice_del(indexObject * index_invalidate_added(self, 0); if (self->ntrev > start) self->ntrev = (int)start; + } else if (self->added) { + Py_CLEAR(self->added); } + self->length = start; if (start < self->raw_length) { if (self->cache) { diff --git a/tests/test-parseindex2.py b/tests/test-parseindex2.py --- a/tests/test-parseindex2.py +++ b/tests/test-parseindex2.py @@ -247,6 +247,32 @@ class parseindex2tests(unittest.TestCase got = index[-1] self.assertEqual(want, got) # no inline data + def testdelitemwithoutnodetree(self): + index, _junk = parsers.parse_index2(data_non_inlined, False) + + def hexrev(rev): + if rev == nullrev: + return b'\xff\xff\xff\xff' + else: + return nodemod.bin('%08x' % rev) + + def appendrev(p1, p2=nullrev): + # node won't matter for this test, let's just make sure + # they don't collide. Other data don't matter either. + node = hexrev(p1) + hexrev(p2) + b'.' * 12 + index.append((0, 0, 12, 1, 34, p1, p2, node)) + + appendrev(4) + appendrev(5) + appendrev(6) + self.assertEqual(len(index), 7) + + del index[1:7] + + # assertions that failed before correction + self.assertEqual(len(index), 1) # was 4 + self.assertEqual(index.headrevs(), [0]) # gave ValueError + if __name__ == '__main__': import silenttestrunner