From: Derrick Stolee Date: Tue, 09 Oct 2018 14:11:32 GMT Subject: Re: [PATCH 2/3] midx: close multi-pack-index on repack Message-ID: <8b5dbe3d-b382-bf48-b524-d9e8a074ac4d@gmail.com> In-Reply-To: On 10/9/2018 5:10 AM, Junio C Hamano wrote: > "Derrick Stolee via GitGitGadget" writes: > >> diff --git a/builtin/repack.c b/builtin/repack.c >> index c6a7943d5c..7925bb976e 100644 >> --- a/builtin/repack.c >> +++ b/builtin/repack.c >> @@ -432,6 +432,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix) >> >> if (!midx_cleared) { >> /* if we move a packfile, it will invalidated the midx */ >> + if (the_repository->objects) { >> + close_midx(the_repository->objects->multi_pack_index); >> + the_repository->objects->multi_pack_index = NULL; >> + } >> clear_midx_file(get_object_directory()); >> midx_cleared = 1; > It somehow looks like a bit of layering violation, doesn't it? When > we are clearing a midx file, don't we always want to do this as well? You're right. It did feel a bit wrong. In v2, I'll replace this commit with a refactor of clear_midx_file() to do that. One tricky part is that we need to clear the file even if the in-memory struct hasn't been initialized, but I think passing a repository will suffice for that. CC Stefan: Is there a plan to make get_object_directory() take a repository parameter? Thanks, -Stolee