Re: [PATCH] read-cache: tighten checks for do_read_index

[Thomas Gummerer <t.gummerer@gmail.com>]

On 03/24, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > 03f15a7 read-cache: fix reading of split index moved the checks for the
> > correct order of index entries out of do_read_index.  This loosens the
> > checks more than necessary.  Re-introduce the checks for the order, but
> > don't error out when we have multiple stage-0 entries in the index.
> > Return a flag for the caller instead, if we have multiple stage-0
> > entries and let the caller decide if we need to error out.
> >
> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> > ---
> >
> > This is a patch on top of my previous patch, as that one has already
> > been merged to next.
>
> I am not convinced that this is a good change in the first place.
>
> The original before your fix was wrong exactly because it was too
> tightly tied to the implementation of the index file format where
> there was only one file whose contents must be sorted, and that is
> why it was a broken check in a new world with split-index.  And your
> fix in 'next' is the right fix---it makes the verification happen
> only on the result is given to the caller for its consumption.
>
> It may be true that entries may have to be sorted in a certain order
> when reading the original index file format and also reading some
> steps in reading the split-index, but that merely happens to be an
> imprementation detail of the two format currently supported, and as
> we improve these formats (or even introduce yet another one) in the
> longer term, this patch would re-introduce the same issue your
> earlier fix corrected, wouldn't it?

Yes, after looking at it again I completely agree.  Sorry for the noise.