Re: [PATCH v4 0/9] repack: avoid MIDX'ing cruft pack(s) where possible

>
> On Wed, May 28, 2025 at 07:20:07PM -0400, Taylor Blau wrote:
> > Range-diff against v3:
>
> Hmm. My tool for submitting patches botched the range-diff here. The
> correct range-diff is:
>
> 1:  986bef29b5 ! 1:  2753e29648 pack-objects: limit scope in 'add_object_entry_from_pack()'
>     @@ builtin/pack-objects.c: static int add_object_entry_from_pack(const struct objec
>         if (p) {
>      -          struct rev_info *revs = _data;
>                 struct object_info oi = OBJECT_INFO_INIT;
>     --
>     +
>                 oi.typep = &type;
>     -+
>     -           if (packed_object_info(the_repository, p, ofs, &oi) < 0) {
>     +@@ builtin/pack-objects.c: static int add_object_entry_from_pack(const struct object_id *oid,
>                         die(_("could not get type of object %s in pack %s"),
>                             oid_to_hex(oid), p->pack_name);
>                 } else if (type == OBJ_COMMIT) {
> 2:  6f8fe8a4e1 = 2:  32b49d9073 pack-objects: factor out handling '--stdin-packs'
> 3:  2a235461a6 = 3:  a797ff3a83 pack-objects: declare 'rev_info' for '--stdin-packs' earlier
> 4:  240e90b68d = 4:  29bf05633a pack-objects: perform name-hash traversal for unpacked objects
> 5:  9a18fa2e52 = 5:  0696fa1736 pack-objects: fix typo in 'show_object_pack_hint()'
> 6:  6c997853f1 = 6:  1cc45b4472 pack-objects: swap 'show_{object,commit}_pack_hint'
> 7:  0ff699f056 = 7:  3e3d929bd0 pack-objects: introduce '--stdin-packs=follow'
> 8:  58891101f3 ! 8:  52a069ef48 repack: exclude cruft pack(s) from the MIDX where possible
>     @@ Commit message
>          MIDX with '--write-midx' to ensure that the resulting MIDX was always
>          closed under reachability in order to generate reachability bitmaps.
>
>     -    Suppose (prior to this patch) you have a once-unreachable object packed
>     -    in a cruft pack, which later on becomes reachable from one or more
>     -    objects in a geometrically repacked pack. That once-unreachable object
>     -    *won't* appear in the new pack, since the cruft pack was specified as
>     -    neither included nor excluded to 'pack-objects --stdin-packs'. If the
>     +    While the previous patch added the '--stdin-packs=follow' option to
>     +    pack-objects, it is not yet on by default. Given that, suppose you have
>     +    a once-unreachable object packed in a cruft pack, which later becomes
>     +    reachable from one or more objects in a geometrically repacked pack.
>     +    That once-unreachable object *won't* appear in the new pack, since the
>     +    cruft pack was not specified as included or excluded when the
>     +    geometrically repacked pack was created with 'pack-objects
>     +    --stdin-packs' (*not* '--stdin-packs=follow', which is not on). If that
>          new pack is included in a MIDX without the cruft pack, then trying to
>          generate bitmaps for that MIDX may fail. This happens when the bitmap
>          selection process picks one or more commits which reach the
>     -    once-unreachable objects, commit ddee3703b3 ensures that the MIDX will
>     -    be closed under reachability. Without it, we would fail to generate a
>     -    MIDX bitmap.
>     +    once-unreachable objects.
>
>     +    To mitigate this failure mode, commit ddee3703b3 ensures that the MIDX
>     +    will be closed under reachability by including cruft pack(s). If cruft
>     +    pack(s) were not included, we would fail to generate a MIDX bitmap. But
>          ddee3703b3 alludes to the fact that this is sub-optimal by saying
>
>              [...] it's desirable to avoid including cruft packs in the MIDX
>
> Thanks,
> Taylor