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

On Wed, Sep 01, 2021 at 05:40:22PM -0400, Taylor Blau wrote:

Hmm. As I was wondering about about, this is more complicated than meets
the eye. Consider t5616.36, which tests that repacking does not loosen
promisor objects.

In builtin/repack.c:repack_promisor_object(), the repack builtin tells
pack-objects about the pack that it just wrote with `--keep-pack` (and
we rely on that working in order to not loosen all of the objects that
we just wrote).

Except when we iterate through `get_all_packs()`, we don't see the keep
pack yet, because it is still prefixed with .tmp.

So, this does get kind of tricky. There are some internal callers that
do want to know about .tmp packs and a whole host of other callers that
don't or shouldn't. Maybe that should point us towards "we should be
more careful about the order we write packs in, even temporary ones".

So I happened to be looking at some packing stuff again today, and I
realized the situation is much less dire than I made it out to be.

On the reading side, we _won't_ look at tmp_pack_*, nor tmp_idx_*,
because they don't end in '.pack' or '.idx'. So really, the only
confusing case is the ".tmp-$$-etc.idx" that is generated by
pack-objects (because it thinks of that as the final name to use
itself), and then later rearranged by git-repack.

So I think we _are_ OK as long as the correct order is observed during
the rename-into-place steps.

I.e., your original patch (plus the extra post in index-pack) would mean
that we're fully covered. The .tmp-*.idx ones would appear in the
correct order due to pack-objects, and the final pack-*.idx ones due to
git-repack.

-Peff