[PATCH 2 of 3 STABLE] manifest: avoid corruption by dropping removed files with pure (issue5801)

[Matt Harbison]

On Sat, 25 May 2019 22:02:01 -0400, Yuya Nishihara <yuya at tcha.org> wrote:

> On Fri, 24 May 2019 00:01:39 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at yahoo.com>
>> # Date 1558662864 14400
>> #      Thu May 23 21:54:24 2019 -0400
>> # Branch stable
>> # Node ID 4cb54e688c12790fcf3cacff2b2fb3822e9f8e8f
>> # Parent  89c0c8edc9d40ccb707acbeccff25891c5456521
>> manifest: avoid corruption by dropping removed files with pure  
>> (issue5801)
>
> Queued for stable, thanks.
>
> Maybe we'll need some unit tests for this compaction method?
>
>> @@ -298,6 +302,16 @@ class _lazymanifest(object):
>>                      i += 1
>>                      if i == len(self.positions) or self.positions[i] <  
>> 0:
>>                          break
>> +
>> +                    # A removed file has no positions[] entry, but  
>> does have an
>> +                    # overwritten first byte.  Break out and find the  
>> end of the
>> +                    # current good entry/entries if there is a removed  
>> file
>> +                    # before the next position.
>> +                    if (self.hasremovals
>> +                        and self.data.find('\n\x00', cur,
>> +                                           self.positions[i]) != -1):
>> +                        break
>
> Just curious how slow it would be if we just did "if hasremovals: break".

I'd imagine that it depends on the repo size and the frequency of  
rename/remove, since it's basically short circuiting the loop that  
collects as many entries as possible.  The repo from timeless took about a  
second longer (out of ~4 min), so no big deal.  I'll test at work with a  
repo that has a lot more files, and a lot of renames.