Re: [PATCH v2 2/8] builtin/multi-pack-index.c: support `--stdin-packs` mode

On Wed, Sep 22, 2021 at 04:11:37PM -0700, Jonathan Tan wrote:

quoted

An alternative approach
would have been to call that function from the `git repack` builtin
directly, but this introduces awkward problems around closing and
reopening the object store, so the MIDX will be written out-of-process.

I'm not sure if the implementation direction started by this patch
(eventually, running "git multi-pack-index write --stdin-packs" to
replace the midx of a repository while "git repack" is running) would
work on platforms in which mmap-ing a file means that other processes
can't delete it, but if it works on a Windows CI, this should be fine.
(I don't have a Windows CI handy to test it on, though.)

Assuming it works on platforms like Windows, this patch looks fine.

It definitely passes CI :-). But special care is taken to handle this
case, and it works for a couple of reasons. Notably that we only call
`write_midx_included_packs()` (which in turn invokes the
multi-pack-index builtin as a sub-process) *after* repack has called
close_object_store(). That means that `repack` won't be holding the MIDX
open while the sub-process is trying to write a new one in its place.

The other reason is that the multi-pack-index process also makes sure to
close the old MIDX before writing the new one, so we can be certain that
neither of these spots are mapping the file or have it opened when
trying to write over it.

Thanks,
Taylor