Re: [PATCH v3 06/13] pack-bitmap.c: support bitmap pack-reuse with incremental MIDXs

From: Taylor Blau <me@ttaylorr.com>
Date: Mar 1, 2025, 00:16 UTC
Message-ID: <Z8JRx1eY0YrTvNEJ@nand.local>
In-Reply-To: <Z8GJc6iDqueVgc67@pks.im>

On Fri, Feb 28, 2025 at 11:01:23AM +0100, Patrick Steinhardt wrote:

Show 23 quoted lines

> On Tue, Nov 19, 2024 at 05:07:35PM -0500, Taylor Blau wrote:
> > diff --git a/pack-bitmap.c b/pack-bitmap.c
> > index c2c824347a6..1dddb242434 100644
> > --- a/pack-bitmap.c
> > +++ b/pack-bitmap.c
> > @@ -2347,14 +2348,18 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
> >  		uint32_t pack_int_id;
> >
> >  		if (bitmap_is_midx(bitmap_git)) {
> > +			struct multi_pack_index *m = bitmap_git->midx;
> >  			uint32_t preferred_pack_pos;
> >
> > -			if (midx_preferred_pack(bitmap_git->midx, &preferred_pack_pos) < 0) {
> > +			while (m->base_midx)
> > +				m = m->base_midx;
> > +
> > +			if (midx_preferred_pack(m, &preferred_pack_pos) < 0) {
> >  				warning(_("unable to compute preferred pack, disabling pack-reuse"));
> >  				return;
> >  			}
>
> Instead of completely disabling preferred packs, should we maybe fall
> back to the preferred pack of the next-higher layer?

The upper layers aren't really supposed to have a notion of a preferred pack, just whatever pack happens to be first in that layer's order (which by definition of the pseudo-pack order makes it preferred, so to speak).

But the base layer definitely can have a preferred pack, and failing to find it usually means that there is other corruption or something else wrong.

Thanks, Taylor