On Wed, Jun 22, 2022 at 12:58:42PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@xxxxxxxxxxxx> writes:
>
> > - the MIDX file itself is written using a lock_file, so it is
> > atomically moved into place, and the temporary file is either
> > removed, or cleaned up automatically with a sigchain handler on
> > process death
>
> Good.
>
> > - the bitmap (written in bitmap_writer_finish(), which is the path for
> > both single- and multi-pack bitmaps) is written to a temporary file
> > and moved into place after the bitmaps are written.
> >
> > ...but this temporary file isn't automatically cleaned up, so it
> > could stick around after process death. Luckily the race window here
> > is pretty small, since all of the bitmaps have been computed already
> > and are held in memory.
> >
> > This is probably worth a cleanup on its own, too.
>
> As long as the "temporary file" is clearly a temporary file that
> "gc" can recognize and get rid of, it would be OK, I would think.
Yeah, I think either is fine (though it would be slightly nicer to have
the sigchain code clean it up ahead of time when possible). In the
meantime, nothing is broken, though.
> > - unless GIT_TEST_MIDX_WRITE_REV=1 is in your environment, we won't
> > *write* a .rev file, hence this is pretty rare to deal with in
> > practice.
>
> OK, but if we were to write one, we should do the same "write into a
> temporary, rename it in place" dance, right? Or is a separate .rev
> file is pretty much a thing of last decade that we do not have to
> worry too much about?
Right. Technically we link() it into place, cf. our discussion about it
here:
https://lore.kernel.org/git/xmqq5yqeghck.fsf@gitster.g/
But in general this is not really at all considered common anymore,
since we expect all new writes to have the reverse index embedded in the
MIDX itself.
Thanks,
Taylor
