On 03/24, Duy Nguyen wrote:
> On Sat, Mar 21, 2015 at 4:43 AM, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> > The split index extension uses ewah bitmaps to mark index entries as
> > deleted, instead of removing them from the index directly. This can
> > result in an on-disk index, in which entries of stage #0 and higher
> > stages appear, which are removed later when the index bases are merged.
> >
> > 15999d0 read_index_from(): catch out of order entries when reading an
> > index file introduces a check which checks if the entries are in order
> > after each index entry is read in do_read_index. This check may however
> > fail when a split index is read.
> >
> > Fix this by moving checking the index after we know there is no split
> > index or after the split index bases are successfully merged instead.
>
> Thank you for catching this. I was about to write "would be nice to
> point out what tests fail so the reviewer has easier time trying
> themselves", but whoa.. quite a few of them!
>
> May I suggest a slight modification. Even though stage info is messed
> up before the index is merged, I think we should still check that both
> front and base indexes have all the names sorted correctly (and even
> stronger, the base index should never contain staged entries). If
Hmm I just tried adding another check for that, but the base index
does seem to include staged entries sometimes.
I've tried with this, but there are quite a few test failures. For
example in t3600-rm.sh test #52 fails, and test-dump-split-index shows
the submodule with stages 1, 2 and 3 in the index.
own 74cd8e14a8fcc5df52e5c47a3ba0c30b29e5075a
base 0ff6ae43b1caa039c2a6262f07678b88314a5b4f
160000 6daff6d0fc4a9299deb0a51881e14cdbda16b88d 1 submod
160000 ee8321115a919c0607236124af886df2c9f16e2f 2 submod
160000 f3abce3ddcc2d68a8c113bd16767deeb376276f9 3 submod
replacements:
deletions: 3
diff --git a/read-cache.c b/read-cache.c
index 2ba67ce..b502290 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1528,6 +1528,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist,
struct cache_header *hdr;
void *mmap;
size_t mmap_size;
+ int fully_merged = 1;
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
if (istate->initialized)
@@ -1580,6 +1581,10 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist,
ce = create_from_disk(disk_ce, &consumed, previous_name);
set_index_entry(istate, i, ce);
+ if (ce_stage(ce)) {
+ fully_merged = 0;
+ }
+
if (i > 0)
if (check_ce_order(istate->cache[i - 1], ce, 1) > 0 &&
multiple_stage_entries)
@@ -1610,6 +1615,8 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist,
src_offset += extsize;
}
munmap(mmap, mmap_size);
+ if (!fully_merged && istate->split_index)
+ die("base index cannot contain staged entries");
return istate->cache_nr;
unmap:
> sorting order is messed up, it could lead to other problems. So
> instead of removing the test from do_read_index(), perhaps add a flag
> in check_ce_order() to optionally detect the stage problem, but
> print/do nothing, only set a flag so the caller know there may be a
> problem. In the two new call sites you added, we still call the new
> check_ce_order() again to make sure everything is in order. In the
> call site where split index is not active, if the previous
> check_ce_order() call from inside do_read_index() says "everything is
> ok", we could even skip the check.
> --
> Duy
--
Thomas Gummerer
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html