Re: [PATCH v5 0/8] Repack objects into separate packfiles based on a filter

From: Taylor Blau <me@ttaylorr.com>
Date: Aug 16, 2023, 00:38 UTC
Message-ID: <ZNwaoGBNrpdzlPUq@nand.local>
In-Reply-To: <xmqqfs4jhp3p.fsf@gitster.g>

On Tue, Aug 15, 2023 at 04:18:02PM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> Taylor Blau <me@ttaylorr.com> writes:
>
> > But why is that tree marked in the first place? It's because we attempt
> > to rebuild the bitmap from the existing .bitmap file, but fail part of
> > the way through (when we look up the first blob object in the reposition
> > table). But that happens *after* we see the tree object, so its bit
> > position is marked, even though we didn't rebuild a complete bitmap.
>
> So, there is another bug lurking, other than the lack of "combining
> filtered repack and bitmaps are explicitly forbidden" logic?

I think that there is a bug lurking in the sense of trying to reuse bitmaps when covering a pack that doesn't have reachability closure in this particular scenario.

But there are no "blessed" use-cases for doing this. So I think that we should indeed fix this, but I am not immediately concerned here.

> We see the tree object, we immediately mark it as "done" even we are
> not, then we finish in failure and the "done" mark is left behind?  Do
> we need two bits, "under review" and "done", or something then?

No; we can either reuse a complete bitmap or not. So it's fine to OR all of the (permuted) bits into ent->bitmap, but it's not OK to fill in just part of them.

Thanks, Taylor