Subject: Re: [PATCH] fixup! midx: implement writing incremental MIDX bitmaps

From: Taylor Blau <[email-masked]>
Date: Thu, 29 Aug 2024 14:57:08 -0400

On Wed, Aug 28, 2024 at 02:33:56PM -0400, Jeff King wrote:
> Is that right, though? It looks like the caller might pass in a
> tempfile name like .../pack/multi-pack-index.d/tmp_midx_XXXXXX,
> if we're in incremental mode. But we'll write directly to
> "multi-pack-index-$hash.bitmap" in the same directory. I'm not sure to
> what degree it matters, since that's the name we want in the long run.
> But would we possibly overwrite an active-in-use file rather than doing
> the atomic rename-into-place if we happened to generate the same midx?
>
> It feels like we should still respect the name the caller is using for
> tempfiles, and then rename it into the correct spot at the end.

In either case, we're going to write to a temporary file initialized by
the pack-bitmap machinery and then rename() it into place at the end of
bitmap_writer_finish().

On the caller side, in the non-incremental mode, we'll pass
$GIT_DIR/objects/pack/multi-pack-index-$hash.bitmap as the location,
write its contents into a temporary file, and then rename() it there.

But in the incremental mode this series introduces, I think it would be
a bug to pass a tmp_midx_XXXXXX file path there, since nobody would move
it from tmp_midx_XXXXX-$HASH.bitmap into its final location.

So I think what's written here with the fixup! patch is right (and
should be squashed into 13/13 in the next round), but let me know if I'm
missing something.

Thanks,
Taylor