facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-06 06:47:41 +03:00
Code Issues Projects Releases Wiki Activity

parsers: fix invariant bug in find_deepest (issue5623)

Browse Source 
find_deepest is used to find the "best" ancestors given a list. In the main
loop it keeps an invariant called 'ninteresting' which is supposed to contain
the number of non-zero entries in the 'interesting' array. This invariant is
incorrectly maintained, however, which leads the the algorithm returning an
empty result for certain graphs. This has been fixed.

Also, the 'interesting' array is supposed to fit 2^ancestors values, but is
incorrectly allocated to twice that size. This has been fixed as well.

The tests in test-ancestor.py compare the Python and C versions of the code,
and report the error correctly, since the Python version works correct. Even
so, I have added an additional test against the expected result, in the event
that both algorithms have an identical error in the future.

This fixes issue5623.
This commit is contained in:
Sune Foldager 2017-07-14 13:48:17 +02:00
parent 15d613159e
commit dca90c364e
2 changed files with 19 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
mercurial/cext/revlog.c
View File

@ -1464,7 +1464,7 @@ static PyObject *find_deepest(indexObject *self, PyObject *revs)
goto bail;
}
interesting = calloc(sizeof(*interesting), 2 << revcount);
interesting = calloc(sizeof(*interesting), 1 << revcount);
if (interesting == NULL) {
PyErr_NoMemory();
goto bail;
@ -1481,6 +1481,8 @@ static PyObject *find_deepest(indexObject *self, PyObject *revs)
interesting[b] = 1;
}
/* invariant: ninteresting is the number of non-zero entries in
* interesting. */
ninteresting = (int)revcount;
for (v = maxrev; v >= 0 && ninteresting > 1; v--) {
@ -1523,8 +1525,10 @@ static PyObject *find_deepest(indexObject *self, PyObject *revs)
continue;
seen[p] = nsp;
interesting[sp] -= 1;
if (interesting[sp] == 0 && interesting[nsp] > 0)
if (interesting[sp] == 0)
ninteresting -= 1;
if (interesting[nsp] == 0)
ninteresting += 1;
interesting[nsp] += 1;
}
}

18
tests/test-ancestor.py
View File

@ -217,14 +217,16 @@ def test_lazyancestors():
# The C gca algorithm requires a real repo. These are textual descriptions of
# DAGs that have been known to be problematic.
# DAGs that have been known to be problematic, and, optionally, known pairs
# of revisions and their expected ancestor list.
dagtests = [
'+2*2*2/*3/2',
'+3*3/*2*2/*4*4/*4/2*4/2*2',
('+2*2*2/*3/2', {}),
('+3*3/*2*2/*4*4/*4/2*4/2*2', {}),
('+2*2*/2*4*/4*/3*2/4', {(6, 7): [3, 5]}),
]
def test_gca():
u = uimod.ui.load()
for i, dag in enumerate(dagtests):
for i, (dag, tests) in enumerate(dagtests):
repo = hg.repository(u, b'gca%d' % i, create=1)
cl = repo.changelog
if not util.safehasattr(cl.index, 'ancestors'):
@ -235,15 +237,21 @@ def test_gca():
# Compare the results of the Python and C versions. This does not
# include choosing a winner when more than one gca exists -- we make
# sure both return exactly the same set of gcas.
# Also compare against expected results, if available.
for a in cl:
for b in cl:
cgcas = sorted(cl.index.ancestors(a, b))
pygcas = sorted(ancestor.ancestors(cl.parentrevs, a, b))
if cgcas != pygcas:
expected = None
if (a, b) in tests:
expected = tests[(a, b)]
if cgcas != pygcas or (expected and cgcas != expected):
print("test_gca: for dag %s, gcas for %d, %d:"
% (dag, a, b))
print(" C returned: %s" % cgcas)
print(" Python returned: %s" % pygcas)
if expected:
print(" expected: %s" % expected)
def main():
seed = None