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
