Subject: Re: [PATCH 0/2] pack-write,repack: prevent opening packs too early

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 07 Sep 2021 21:48:05 +0200

On Tue, Aug 31 2021, Taylor Blau wrote:

> This pair of patches fixes a race where the .idx is moved into place
> before the .rev file, allowing the pack to be in a state where it
> appears a .rev file wasn't generated.
>
> This can cause Git to inadvertently take the slow "generate the
> reverse index on-the-fly", which does not impact correctness, but is
> unnecessarily slow when compared to reading the .rev file.
>
> The race is fixed by moving the .idx into place only after all other
> pack-related files have already been written. The first patch fixes
> the direct `pack-objects` case, and the second patch fixes `repack`
> (which also renames pack files around).
>
> Thanks in advance for your review.
>
> Taylor Blau (2):
>   pack-write.c: rename `.idx` file into place last
>   builtin/repack.c: move `.idx` files into place last
>
>  builtin/repack.c |  2 +-
>  pack-write.c     | 12 ++++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)

I've reviewed this to the point of coming up with my own series on top
of it, but it can be considered separately:
https://lore.kernel.org/git/[email-masked]/

The only change (if any) would be to perhaps update the commit
message(s) to note that the *.bitmap case is left, and that the promise
of "closing the race" is still subject to the vagaries of our non-use of
fsync() here, so pedantically speaking the race is still there.

But even without those suggested changes:

Reviewed-by: Ævar Arnfjörð Bjarmason <[email-masked]>