Re: [PATCH v2] builtin/repack.c: avoid making cruft packs preferred
From: Patrick Steinhardt <hidden>
Date: 2023-10-05 14:43:15
On Tue, Oct 03, 2023 at 05:54:19PM -0400, Taylor Blau wrote: [snip]
quoted hunk ↗ jump to hunk
@@ -801,6 +814,38 @@ static int write_midx_included_packs(struct string_list *include, if (preferred) strvec_pushf(&cmd.args, "--preferred-pack=%s", pack_basename(preferred)); + else if (names->nr) { + /* The largest pack was repacked, meaning that either + * one or two packs exist depending on whether the + * repository has a cruft pack or not.
Nit: this comment will grow stale soonish once your patch series lands that introduces a maximum packfile size for cruft packs as there can be arbitrarily many cruft packs from thereon.
+ * Select the non-cruft one as preferred to encourage + * pack-reuse among packs containing reachable objects + * over unreachable ones. + * + * (Note we could write multiple packs here if + * `--max-pack-size` was given, but any one of them + * will suffice, so pick the first one.) + */
Well, okay, you kind of acknowledge this here. The rest of this patch series looks good to me and makes sense. I don't really think that this comment here is worth a reroll. Patrick
Attachments
- signature.asc [application/pgp-signature] 833 bytes