Re: [PATCH v2 5/8] repack: follow kept packs when omitting cruft from the MIDX
From: Jeff King <hidden>
Date: 2026-10-02 23:25:30
On Wed, Sep 30, 2026 at 11:11:51PM -0500, Taylor Blau wrote:
quoted hunk ↗ jump to hunk
diff --git a/builtin/repack.c b/builtin/repack.c index 88b05e96b5b..27d6668a4ab 100644 --- a/builtin/repack.c +++ b/builtin/repack.c@@ -476,9 +476,11 @@ int cmd_repack(int argc, show_progress = !po_args.quiet && isatty(2); strvec_push(&cmd.args, "--keep-true-parents"); - for (i = 0; i < keep_pack_list.nr; i++) - strvec_pushf(&cmd.args, "--keep-pack=%s", - keep_pack_list.items[i].string); + /* Geometric follow walks exclude these packs through stdin instead. */ + if (!(geometry.split_factor && !midx_must_contain_cruft)) + for (i = 0; i < keep_pack_list.nr; i++) + strvec_pushf(&cmd.args, "--keep-pack=%s", + keep_pack_list.items[i].string);
This conditional makes my head hurt because of the double-negation. By De Morgan's it is just: if (!geometry.split_factor || midx_must_contain_cruft) which at least untangles it. The comment makes sense to say "we do not need to do this in geometric" mode, which matches the first half. But why does midx_must_contain_cruft trigger it? I guess it is "we do not need to bother doing the "^"-exclusion later in that mode", but I wonder if there is any advantage to suppressing it. I don't remember enough of the details here about why we were treating keep packs specially in the first place.
quoted hunk ↗ jump to hunk
@@ -593,6 +595,29 @@ int cmd_repack(int argc, fprintf(in, "%c%s\n", marker, basename); } + if (!midx_must_contain_cruft) {
OK, and this is the flip side of the earlier conditional. We are in geometric mode if we get here, and we kick in only in non-midx-cruft mode. IMHO the De Morgan untangling above makes it more clear, but you could probably even further with: /* explanatory comment here */ int handle_keep_packs_via_follow = geometry.split_factor && !midx_must_contain_cruft; And then use that in both spots. That might be overkill, though (and the name I proposed certainly sucks). -Peff