Hi,

On Sun, Apr 22, 2018 at 5:38 AM, Duy Nguyen [off-list ref] wrote:

quoted

  - there's a better, more performant fix or there is some way to actually
    share a split_index between two independent index_state objects.

A cleaner way of doing this would be something to the line [1]

    move_index_extensions(&o->result, o->dst_index);

near the end of this function. This could be where we compare the
result index with the source index's shared file and see if it's worth
keeping the shared index or not. Shared index is designed to work with
huge index files though, any operations that go through all index
entries will usually not be cheap. But at least it's safer.

Yeah, it looks like move_index_extensions() currently has no logic for
the split_index.  Adding it sounds to me like a patch series of its
own, and I'm keen to limit additional changes since my patch series
already broke things pretty badly once already.

quoted

However, with this fix, all the tests pass both normally and under
GIT_TEST_SPLIT_INDEX=DareISayYes.  Without this patch, when
GIT_TEST_SPLIT_INDEX is set, my directory rename detection series will fail
several tests, as reported by SZEDER.

Yes, the change looks good.

Great, thanks for looking over it.

[1] To me the second parameter should be src_index, not dst_index.
We're copying entries from _source_ index to "result" and we should
also copy extensions from the source index. That line happens to work
only when dst_index is the same as src_index, which is the common use
case so far.

That makes sense; this sounds like another fix that should be
submitted.  Did you want to submit a patch making that change?  Do you
want me to?

Elijah