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:
> @@ -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
