Re: [PATCH 4/5] repack-promisor: extract function to remove redundant packs

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 9, 2026, 23:40 UTC
Message-ID: <aWGR1r5PlLL3rWWd@nand.local>
In-Reply-To: <20260105-pks-geometric-repack-with-promisors-v1-4-c4660573437e@pks.im>

On Mon, Jan 05, 2026 at 02:16:44PM +0100, Patrick Steinhardt wrote:

Show 13 quoted lines

> @@ -226,6 +227,15 @@ void pack_geometry_remove_redundant(struct pack_geometry *geometry,
>  	strbuf_release(&buf);
>  }
>
> +void pack_geometry_remove_redundant(struct pack_geometry *geometry,
> +				    struct string_list *names,
> +				    struct existing_packs *existing,
> +				    const char *packdir)
> +{
> +	remove_redundant_packs(geometry->pack, geometry->split,
> +			       names, existing, packdir);
> +}
> +

The refactoring up to this point looks all good to me.

As a side-note, I would love to get rid of the pack_geometry_remove_redundant() function altogether. It is kind of a hack that we handle determining which packs are made redundant by a repacking operation in two different ways depending on whether or not we are doing a geometric repack.

I have some patches to do this in a series that implements the "reachability-guided" geometric repacking technique that I have talked above[^1] previously. I think (having skimmed the next patch but not yet fully reviewed it) that this should still all be doable with your patches. We just have to take two passes (once through the existing non-promisor packs and then another pass through the promisor ones).

So I think that this all looks good to me and shouldn't interfere with that effort, though I'll make a note to rebase those patches on top of these to make it easier for the maintainer to queue both of them.

Thanks, Taylor

[^1]: Well, I was pretty sure that I had mentioned it on the list, but
  can't seem to find anything corresponding to it in my "sent" folder.
  In case I haven't talked above it before, the gist is a special mode
  of --stdin-packs that only packs objects from the included set of
  packs which are reachable. If repack generates a cruft pack after the
  fact, that allows us to "incrementally" build up the cruft pack over
  time.