Re: [PATCH 2/4] pack-objects: ensure tree/tag closure with '--stdin-packs=follow'

From: Taylor Blau <ttaylorr@openai.com>
Date: Oct 2, 2026, 00:51 UTC
Message-ID: <ar8AJLYZVb6sCIO-@com-79390>
In-Reply-To: <CABPp-BHE662t9aaNcZ4DZ+2AU_C7jR7_VyHZwt2Tm8JSPE3JZw@mail.gmail.com>

On Thu, Oct 01, 2026 at 04:22:11PM -0700, Elijah Newren wrote:

Show 10 quoted lines

> With the oid_array and parent-first pack order, the blobs are visited
> as sub/a and sub/b, so sub/* -delta applies. With the oidset, the
> subtree is visited first and the blobs are seen as a and b, so one is
> delta-compressed. When the root is processed later, the subtree is
> already marked SEEN and is not revisited with the sub/ prefix.
>
> The oid_array does not manufacture parent-before-child ordering if the
> input pack itself has the subtree first; this path information is
> explicitly best-effort. But it preserves a useful order when one
> exists, whereas an oidset discards it.

Sure, though as Peff and I discussed elsewhere in the thread, there are also situations where you can produce a sub-optimal pack even with oid_array. That's because the namehash you get for a given tree object depends on the path you took to get there.

So you can certainly come up with examples where the ordering of tree objects in an array of extra roots produces a lesser-quality delta selection than the same objects permuted into some different order.

The other thing to keep in mind is that, while there are clearly trade-offs as we have discussed here, the oidset ensures that we don't allocate memory wastefully when the same object is listed multiple times as an extra root.

The other other thing to keep in mind is that the size of this set is almost always going to be puny compared to the size of the overall pack. These objects are merely meant to pull in the (likely) few objects that need refreshed out of the cruft pack in order to ensure reachability closure.

So I think it's clear that this is a trade-off, and neither decision (oidset vs oid_array) is absolutely perfect for all cases. But on balance I think that the trade-offs push us towards oidset much more than they do towards oid_array.

Thanks, Taylor