[PATCH 02 of 10 V2] revlog: fix pure python slicing code when chain contains nullrev

[Boris FELD]

On 22/12/2018 04:20, Yuya Nishihara wrote:
> On Fri, 21 Dec 2018 12:47:05 +0100, Boris Feld wrote:
>> # HG changeset patch
>> # User Boris Feld <boris.feld at octobus.net>
>> # Date 1545296356 -3600
>> #      Thu Dec 20 09:59:16 2018 +0100
>> # Node ID df9b79e863d6e5215175487330b1469067e20bbd
>> # Parent  d51d82a46d9545235be727b875deeffd9de324e9
>> # EXP-Topic sparse-revlog
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r df9b79e863d6
>> revlog: fix pure python slicing code when chain contains nullrev
>>
>> We fixed the C code, but the python code still misbehaved.
>>
>> diff --git a/mercurial/revlogutils/deltas.py b/mercurial/revlogutils/deltas.py
>> --- a/mercurial/revlogutils/deltas.py
>> +++ b/mercurial/revlogutils/deltas.py
>> @@ -116,6 +116,12 @@ def slicechunk(revlog, revs, targetsize=
>>      [[0], [11], [13], [15]]
>>      >>> list(slicechunk(revlog, [0, 11, 13, 15], targetsize=20))
>>      [[0], [11], [13, 15]]
>> +
>> +    Slicing involving nullrev
>> +    >>> list(slicechunk(revlog, [-1, 0, 11, 13, 15], targetsize=20))
>> +    [[-1, 0], [11], [13, 15]]
>> +    >>> list(slicechunk(revlog, [-1, 13, 15], targetsize=5))
>> +    [[-1, 13, 15]]
>>      """
>>      if targetsize is not None:
>>          targetsize = max(targetsize, revlog._srmingapsize)
>> @@ -363,6 +369,8 @@ def _slicechunktodensity(revlog, revs, t
>>      gaps = []
>>      prevend = None
>>      for i, rev in enumerate(revs):
>> +        if rev == nullrev:
>> +            continue
>>          revstart = start(rev)
>>          revlen = length(rev)
>>  
>> @@ -490,7 +498,11 @@ def segmentspan(revlog, revs):
>>      if not revs:
>>          return 0
>>      end = revlog.end(revs[-1])
>> -    return end - revlog.start(revs[0])
>> +    i = 0
>> +    while revs[i] == nullrev:
>> +        i += 1
>> +    startrev = revs[i]
>> +    return end - revlog.start(startrev)
> The C implementation appears not doing that IIUC. Doesn't it a bug of
> the _testrevlog? I think it will misbehave if nullrev is passed in.
Ha! good catch, I'll fix _testrevlog instead.
>
> Can you turn these tests into a unittest to cover both pure and C
> implementations?
Turning this specific test is a bit more complicated than we would like
as we are taking advantage of _testrevlog flexibility to produce a
simple but clear test.

However, we've already added some unittest using real revlog in patches
you have already taken. Is that enough for you?
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel