Re: [PATCH v3 09/25] midx: avoid opening multiple MIDXs when writing

On Thu, Aug 12, 2021 at 04:22:29PM -0400, Jeff King wrote:

On Thu, Aug 12, 2021 at 04:15:32PM -0400, Jeff King wrote:

quoted

I think we'd need something like this:

diff --git a/midx.c b/midx.c
index 6dfafe7a8c..bfb6afea2e 100644
--- a/midx.c
+++ b/midx.c

@@ -1123,8 +1123,7 @@ static int write_midx_internal(const char *object_dir,
 	hold_lock_file_for_update(&lk, midx_name, LOCK_DIE_ON_ERROR);
 	f = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk));

-	if (ctx.m)
-		close_midx(ctx.m);
+	close_object_store(the_repository->objects);

 	if (ctx.nr - dropped_packs == 0) {
 		error(_("no pack files to index."));

though I'm not sure:

 - if this should be unconditional or dependent on ctx.m (I think the
   latter, because if we are renaming over any open midx, we would have
   filled in ctx.m earlier).

 - if this should go below the "no pack files to index" check (i.e., is
   there any point in closing if we know we will not write?). In fact,
   its purpose might be more obvious right before finalize_hashfile(),
   but I am OK either way on that.

Ah, this close_midx() actually gets moved and made unconditional later
in the series.  But it still needs to be close_object_store() instead.

Exactly; this first patch should read:

    if (ctx.m)
      close_object_store(the_repository->objects);

and then the latter patch (15/25) we drop the conditional and move our
call down until after the MIDX bitmap is written, but before we call
commit_lock_file().

Thanks,
Taylor