Re: [PATCH 0/4] repack: various corner cases for cruft-less MIDXs

From: Taylor Blau <ttaylorr@openai.com>
Date: Oct 1, 2026, 03:37 UTC
Message-ID: <ar3VXavSoKS3xaiT@com-79390>
In-Reply-To: <20260930205535.GD747209@coredump.intra.peff.net>

On Wed, Sep 30, 2026 at 04:55:35PM -0400, Jeff King wrote:

Show 14 quoted lines

> On Tue, Sep 29, 2026 at 08:28:34PM -0500, Taylor Blau wrote:
>
> > This patch series fixes a few bugs I spotted while investigating the
> > cruft-less MIDX feature.
> >
> > The bugs addressed are found in various corner cases, and, when
> > triggered, may result in a MIDX being written whose objects are not
> > closed under reachability. When this happens while the caller is trying
> > to write reachability bitmaps, bitmap generation may fail if one or more
> > selected commits are descendants of the open portion of the MIDX.
>
> I think all of these are making things strictly better, but I did find a
> few spots where the fixes might be incomplete. I'm not sure if that
> argues for a re-roll or for punting those to future work. ;)

Thanks for the review. Like I wrote in my response to your review, leaving it half-fixed felt dishonest, so fixes are included in the subsequent round, though they do make the series a little longer.

There is a separate, pre-existing bug that I would like to fix outside of this series, but only because it (a) is independent of this series, and (b) I estimate that the fix is considerably more complex.

Show 6 quoted lines

> I agree with Stolee that an oidset is perhaps a better data structure
> for storing the extra roots (which are in a kind-of random order anyway,
> since we're pulling them in pack order from various packs). But it also
> probably doesn't make that big a difference in practice (we'll skip
> duplicates during the traversal, and you probably don't have that many
> duplicate objects in a repo in the first place).

Yup.

Thanks, Taylor