Subject: Re: [PATCH v2 08/24] midx: respect 'core.multiPackIndex' when writing

From: Taylor Blau <[email-masked]>
Date: Tue, 27 Jul 2021 13:36:34 -0400

On Tue, Jul 27, 2021 at 01:29:49PM -0400, Jeff King wrote:
> On Mon, Jul 26, 2021 at 06:14:09PM -0400, Taylor Blau wrote:
> > So in this scenario, we have two copies of the same MIDX open, and the
> > repository's single pack is opened in one of the MIDXs, but not both.
> > One copy of the pack is pointed at via r->objects->packed_git. Then when
> > we fall back to open_pack_bitmap(), we call get_all_packs(), which calls
> > prepare_midx_pack(), which installs the second MIDX's copy of the same
> > pack into the r->objects->packed_git, and we have a cycle.
>
> Right, I understand how that ends up with duplicate structs for each
> pack. But how do we get a cycle out of that?

Sorry, it isn't a true cycle where p->next == p.

> But now the internal midx writing code can never call close_midx() on
> that, because it does not own it to close. Can we simply drop the
> close_midx() call there?
>
> This would all make much more sense to me if write_midx_internal()
> simply took a conceptually read-only midx as a parameter, and the caller
> passed in the appropriate one (probably even using
> prepare_multi_pack_index_one() to get it).

No, we can't drop the close_midx() call there because we must close the
MIDX file on Windows before moving a new one into place. My feeling is
we should always be working on the r->objects->multi_pack_index pointer,
and calling close_object_store() there instead of close_midx().

Does that seem like a reasonable approach to you?

Thanks,
Taylor