From: Taylor Blau Date: Tue, 27 Jul 2021 17:36:34 GMT Subject: Re: [PATCH v2 08/24] midx: respect 'core.multiPackIndex' when writing Message-ID: In-Reply-To: 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