builtin/repack.c: extract common cruft pack loop
When generating the list of packs to store in a MIDX (when given the `--write-midx` option), we include any cruft packs both during --geometric and non-geometric repacks. But the rules for when we do and don't have to check whether any of those cruft packs were queued for deletion differ slightly between the two cases. But the two can be unified, provided there is a little bit of extra detail added in the comment to clarify when it is safe to avoid checking for any pending deletions (and why it is OK to do so even when not required). Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau committed
Sep 13, 2023 at 15:18 UTC
c6a0468f824f458acea442450d204a3d01d1aa9b
1 file changed
+18
-13
builtin/repack.c
+18
-13
@@ -702,26 +702,31 @@ static void midx_included_packs(struct string_list *include,
702
703
string_list_insert(include, strbuf_detach(&buf, NULL));
704
}
705
-
706
- for_each_string_list_item(item, &existing->cruft_packs) {
707
- /*
708
- * no need to check DELETE_PACK, since we're not
709
- * doing an ALL_INTO_ONE repack
710
- */
711
- string_list_insert(include, xstrfmt("%s.idx", item->string));
712
- }
705
} else {
706
for_each_string_list_item(item, &existing->non_kept_packs) {
707
if (pack_is_marked_for_deletion(item))
708
continue;
709
string_list_insert(include, xstrfmt("%s.idx", item->string));
710
}
711
+ }
712
720
- for_each_string_list_item(item, &existing->cruft_packs) {
721
- if (pack_is_marked_for_deletion(item))
722
- continue;
723
- string_list_insert(include, xstrfmt("%s.idx", item->string));
724
- }
713
+ for_each_string_list_item(item, &existing->cruft_packs) {
714
+ /*
715
+ * When doing a --geometric repack, there is no need to check
716
+ * for deleted packs, since we're by definition not doing an
717
+ * ALL_INTO_ONE repack (hence no packs will be deleted).
718
+ * Otherwise we must check for and exclude any packs which are
719
+ * enqueued for deletion.
720
+ *
721
+ * So we could omit the conditional below in the --geometric
722
+ * case, but doing so is unnecessary since no packs are marked
723
+ * as pending deletion (since we only call
724
+ * `mark_packs_for_deletion()` when doing an all-into-one
725
+ * repack).
726
+ */
727
+ if (pack_is_marked_for_deletion(item))
728
+ continue;
729
+ string_list_insert(include, xstrfmt("%s.idx", item->string));
730
}
731
}
732