builtin/repack.c: store existing cruft packs separately

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. 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:17 UTC eabfaf8e8db27dd76ec1f1d4e0a2a124374475ab
1 file changed +23 -16
builtin/repack.c
+23 -16
@@ -27,7 +27,6 @@
27 #define PACK_CRUFT 4
28
29 #define DELETE_PACK 1
30 -#define CRUFT_PACK 2
30
31 static int pack_everything;
32 static int delta_base_offset = 1;
@@ -98,16 +97,18 @@ static int repack_config(const char *var, const char *value,
97 struct existing_packs {
98 struct string_list kept_packs;
99 struct string_list non_kept_packs;
100 + struct string_list cruft_packs;
101 };
102
103 #define EXISTING_PACKS_INIT { \
104 .kept_packs = STRING_LIST_INIT_DUP, \
105 .non_kept_packs = STRING_LIST_INIT_DUP, \
106 + .cruft_packs = STRING_LIST_INIT_DUP, \
107 }
108
109 static int has_existing_non_kept_packs(const struct existing_packs *existing)
110 {
110 - return existing->non_kept_packs.nr;
111 + return existing->non_kept_packs.nr || existing->cruft_packs.nr;
112 }
113
114 static void mark_packs_for_deletion_1(struct string_list *names,
@@ -138,6 +139,7 @@ static void mark_packs_for_deletion(struct existing_packs *existing,
139
140 {
141 mark_packs_for_deletion_1(names, &existing->non_kept_packs);
142 + mark_packs_for_deletion_1(names, &existing->cruft_packs);
143 }
144
145 static void remove_redundant_pack(const char *dir_name, const char *base_name)
@@ -165,12 +167,14 @@ static void remove_redundant_packs_1(struct string_list *packs)
167 static void remove_redundant_existing_packs(struct existing_packs *existing)
168 {
169 remove_redundant_packs_1(&existing->non_kept_packs);
170 + remove_redundant_packs_1(&existing->cruft_packs);
171 }
172
173 static void existing_packs_release(struct existing_packs *existing)
174 {
175 string_list_clear(&existing->kept_packs, 0);
176 string_list_clear(&existing->non_kept_packs, 0);
177 + string_list_clear(&existing->cruft_packs, 0);
178 }
179
180 /*
@@ -204,12 +208,10 @@ static void collect_pack_filenames(struct existing_packs *existing,
208
209 if ((extra_keep->nr > 0 && i < extra_keep->nr) || p->pack_keep)
210 string_list_append(&existing->kept_packs, buf.buf);
207 - else {
208 - struct string_list_item *item;
209 - item = string_list_append(&existing->non_kept_packs, buf.buf);
210 - if (p->is_cruft)
211 - item->util = (void*)(uintptr_t)CRUFT_PACK;
212 - }
211 + else if (p->is_cruft)
212 + string_list_append(&existing->cruft_packs, buf.buf);
213 + else
214 + string_list_append(&existing->non_kept_packs, buf.buf);
215 }
216
217 string_list_sort(&existing->kept_packs);
@@ -691,14 +693,11 @@ static void midx_included_packs(struct string_list *include,
693 string_list_insert(include, strbuf_detach(&buf, NULL));
694 }
695
694 - for_each_string_list_item(item, &existing->non_kept_packs) {
695 - if (!((uintptr_t)item->util & CRUFT_PACK)) {
696 - /*
697 - * no need to check DELETE_PACK, since we're not
698 - * doing an ALL_INTO_ONE repack
699 - */
700 - continue;
701 - }
696 + for_each_string_list_item(item, &existing->cruft_packs) {
697 + /*
698 + * no need to check DELETE_PACK, since we're not
699 + * doing an ALL_INTO_ONE repack
700 + */
701 string_list_insert(include, xstrfmt("%s.idx", item->string));
702 }
703 } else {
@@ -707,6 +706,12 @@ static void midx_included_packs(struct string_list *include,
706 continue;
707 string_list_insert(include, xstrfmt("%s.idx", item->string));
708 }
709 +
710 + for_each_string_list_item(item, &existing->cruft_packs) {
711 + if ((uintptr_t)item->util & DELETE_PACK)
712 + continue;
713 + string_list_insert(include, xstrfmt("%s.idx", item->string));
714 + }
715 }
716 }
717
@@ -836,6 +841,8 @@ static int write_cruft_pack(const struct pack_objects_args *args,
841 fprintf(in, "%s-%s.pack\n", pack_prefix, item->string);
842 for_each_string_list_item(item, &existing->non_kept_packs)
843 fprintf(in, "-%s.pack\n", item->string);
844 + for_each_string_list_item(item, &existing->cruft_packs)
845 + fprintf(in, "-%s.pack\n", item->string);
846 for_each_string_list_item(item, &existing->kept_packs)
847 fprintf(in, "%s.pack\n", item->string);
848 fclose(in);