Re: [PATCH 6/8] builtin/repack.c: store existing cruft packs separately
flat view
From: Jeff King <hidden>
Date: 2023-09-07 19:15:57
On Tue, Sep 05, 2023 at 04:36:54PM -0400, Taylor Blau wrote:
When repacking with the `--write-midx` option, we invoke the function `midx_included_packs()` in order to produce the list of packs we want to include in the resulting MIDX. This list is comprised of: - existing .keep packs - any pack(s) which were written earlier in the same process - any unchanged packs when doing a `--geometric` repack - any cruft packs Prior to this patch, we stored pre-existing cruft and non-cruft packs together (provided those packs are non-kept). This meant we needed an additional bit to indicate which non-kept pack(s) were cruft versus those that aren't. But alternatively we can store cruft packs in a separate list, avoiding the need for this extra bit, and simplifying the code below.
OK. Getting rid of the extra bit is nice. We shorten code that only
cares about cruft packs like:
for each pack
if (pack is cruft)
...
to just:
for each cruft_pack
...
which is good. But the flip side is that any existing code which looks
at the combined list now has to do:
for each pack
...
for each cruft_pack
...
I think there's just one case of that, here:
quoted hunk ↗ jump to hunk
@@ -707,6 +706,12 @@ static void midx_included_packs(struct string_list *include, continue; string_list_insert(include, xstrfmt("%s.idx", item->string)); } + + for_each_string_list_item(item, &existing->cruft_packs) { + if ((uintptr_t)item->util & DELETE_PACK) + continue; + string_list_insert(include, xstrfmt("%s.idx", item->string)); + } } }
It may be an OK price to pay if this lets us keep cleaning things up (especially if we could get rid of that util casting entirely!). Let's read on... -Peff