builtin/repack.c: extract structure to store existing packs

The repack machinery needs to keep track of which packfiles were present in the repository at the beginning of a repack, segmented by whether or not each pack is marked as kept. The names of these packs are stored in two `string_list`s, corresponding to kept- and non-kept packs, respectively. As a consequence, many functions within the repack code need to take both `string_list`s as arguments, leading to code like this: ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix, cruft_expiration, &names, &existing_nonkept_packs, /* <- */ &existing_kept_packs); /* <- */ Wrap up this pair of `string_list`s into a single structure that stores both. This saves us from having to pass both string lists separately, and prepares for adding additional fields to this structure. 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 e2b43831a5e4b482be19746d2257a309b51ba5fe
1 file changed +49 -41
builtin/repack.c
+49 -41
@@ -95,14 +95,29 @@ static int repack_config(const char *var, const char *value,
95 return git_default_config(var, value, ctx, cb);
96 }
97
98 +struct existing_packs {
99 + struct string_list kept_packs;
100 + struct string_list non_kept_packs;
101 +};
102 +
103 +#define EXISTING_PACKS_INIT { \
104 + .kept_packs = STRING_LIST_INIT_DUP, \
105 + .non_kept_packs = STRING_LIST_INIT_DUP, \
106 +}
107 +
108 +static void existing_packs_release(struct existing_packs *existing)
109 +{
110 + string_list_clear(&existing->kept_packs, 0);
111 + string_list_clear(&existing->non_kept_packs, 0);
112 +}
113 +
114 /*
99 - * Adds all packs hex strings (pack-$HASH) to either fname_nonkept_list
100 - * or fname_kept_list based on whether each pack has a corresponding
115 + * Adds all packs hex strings (pack-$HASH) to either packs->non_kept
116 + * or packs->kept based on whether each pack has a corresponding
117 * .keep file or not. Packs without a .keep file are not to be kept
118 * if we are going to pack everything into one file.
119 */
104 -static void collect_pack_filenames(struct string_list *fname_nonkept_list,
105 - struct string_list *fname_kept_list,
120 +static void collect_pack_filenames(struct existing_packs *existing,
121 const struct string_list *extra_keep)
122 {
123 struct packed_git *p;
@@ -126,16 +141,16 @@ static void collect_pack_filenames(struct string_list *fname_nonkept_list,
141 strbuf_strip_suffix(&buf, ".pack");
142
143 if ((extra_keep->nr > 0 && i < extra_keep->nr) || p->pack_keep)
129 - string_list_append(fname_kept_list, buf.buf);
144 + string_list_append(&existing->kept_packs, buf.buf);
145 else {
146 struct string_list_item *item;
132 - item = string_list_append(fname_nonkept_list, buf.buf);
147 + item = string_list_append(&existing->non_kept_packs, buf.buf);
148 if (p->is_cruft)
149 item->util = (void*)(uintptr_t)CRUFT_PACK;
150 }
151 }
152
138 - string_list_sort(fname_kept_list);
153 + string_list_sort(&existing->kept_packs);
154 strbuf_release(&buf);
155 }
156
@@ -327,7 +342,7 @@ static int geometry_cmp(const void *va, const void *vb)
342 }
343
344 static void init_pack_geometry(struct pack_geometry *geometry,
330 - struct string_list *existing_kept_packs,
345 + struct existing_packs *existing,
346 const struct pack_objects_args *args)
347 {
348 struct packed_git *p;
@@ -344,23 +359,24 @@ static void init_pack_geometry(struct pack_geometry *geometry,
359
360 if (!pack_kept_objects) {
361 /*
347 - * Any pack that has its pack_keep bit set will appear
348 - * in existing_kept_packs below, but this saves us from
349 - * doing a more expensive check.
362 + * Any pack that has its pack_keep bit set will
363 + * appear in existing->kept_packs below, but
364 + * this saves us from doing a more expensive
365 + * check.
366 */
367 if (p->pack_keep)
368 continue;
369
370 /*
355 - * The pack may be kept via the --keep-pack option;
356 - * check 'existing_kept_packs' to determine whether to
357 - * ignore it.
371 + * The pack may be kept via the --keep-pack
372 + * option; check 'existing->kept_packs' to
373 + * determine whether to ignore it.
374 */
375 strbuf_reset(&buf);
376 strbuf_addstr(&buf, pack_basename(p));
377 strbuf_strip_suffix(&buf, ".pack");
378
363 - if (string_list_has_string(existing_kept_packs, buf.buf))
379 + if (string_list_has_string(&existing->kept_packs, buf.buf))
380 continue;
381 }
382 if (p->is_cruft)
@@ -565,14 +581,13 @@ static void midx_snapshot_refs(struct tempfile *f)
581 }
582
583 static void midx_included_packs(struct string_list *include,
568 - struct string_list *existing_nonkept_packs,
569 - struct string_list *existing_kept_packs,
584 + struct existing_packs *existing,
585 struct string_list *names,
586 struct pack_geometry *geometry)
587 {
588 struct string_list_item *item;
589
575 - for_each_string_list_item(item, existing_kept_packs)
590 + for_each_string_list_item(item, &existing->kept_packs)
591 string_list_insert(include, xstrfmt("%s.idx", item->string));
592 for_each_string_list_item(item, names)
593 string_list_insert(include, xstrfmt("pack-%s.idx", item->string));
@@ -600,7 +615,7 @@ static void midx_included_packs(struct string_list *include,
615 string_list_insert(include, strbuf_detach(&buf, NULL));
616 }
617
603 - for_each_string_list_item(item, existing_nonkept_packs) {
618 + for_each_string_list_item(item, &existing->non_kept_packs) {
619 if (!((uintptr_t)item->util & CRUFT_PACK)) {
620 /*
621 * no need to check DELETE_PACK, since we're not
@@ -611,7 +626,7 @@ static void midx_included_packs(struct string_list *include,
626 string_list_insert(include, xstrfmt("%s.idx", item->string));
627 }
628 } else {
614 - for_each_string_list_item(item, existing_nonkept_packs) {
629 + for_each_string_list_item(item, &existing->non_kept_packs) {
630 if ((uintptr_t)item->util & DELETE_PACK)
631 continue;
632 string_list_insert(include, xstrfmt("%s.idx", item->string));
@@ -700,8 +715,7 @@ static int write_cruft_pack(const struct pack_objects_args *args,
715 const char *pack_prefix,
716 const char *cruft_expiration,
717 struct string_list *names,
703 - struct string_list *existing_packs,
704 - struct string_list *existing_kept_packs)
718 + struct existing_packs *existing)
719 {
720 struct child_process cmd = CHILD_PROCESS_INIT;
721 struct strbuf line = STRBUF_INIT;
@@ -744,9 +758,9 @@ static int write_cruft_pack(const struct pack_objects_args *args,
758 in = xfdopen(cmd.in, "w");
759 for_each_string_list_item(item, names)
760 fprintf(in, "%s-%s.pack\n", pack_prefix, item->string);
747 - for_each_string_list_item(item, existing_packs)
761 + for_each_string_list_item(item, &existing->non_kept_packs)
762 fprintf(in, "-%s.pack\n", item->string);
749 - for_each_string_list_item(item, existing_kept_packs)
763 + for_each_string_list_item(item, &existing->kept_packs)
764 fprintf(in, "%s.pack\n", item->string);
765 fclose(in);
766
@@ -778,8 +792,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
792 struct child_process cmd = CHILD_PROCESS_INIT;
793 struct string_list_item *item;
794 struct string_list names = STRING_LIST_INIT_DUP;
781 - struct string_list existing_nonkept_packs = STRING_LIST_INIT_DUP;
782 - struct string_list existing_kept_packs = STRING_LIST_INIT_DUP;
795 + struct existing_packs existing = EXISTING_PACKS_INIT;
796 struct pack_geometry geometry = { 0 };
797 struct strbuf line = STRBUF_INIT;
798 struct tempfile *refs_snapshot = NULL;
@@ -915,13 +928,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
928 packtmp_name = xstrfmt(".tmp-%d-pack", (int)getpid());
929 packtmp = mkpathdup("%s/%s", packdir, packtmp_name);
930
918 - collect_pack_filenames(&existing_nonkept_packs, &existing_kept_packs,
919 - &keep_pack_list);
931 + collect_pack_filenames(&existing, &keep_pack_list);
932
933 if (geometry.split_factor) {
934 if (pack_everything)
935 die(_("options '%s' and '%s' cannot be used together"), "--geometric", "-A/-a");
924 - init_pack_geometry(&geometry, &existing_kept_packs, &po_args);
936 + init_pack_geometry(&geometry, &existing, &po_args);
937 split_pack_geometry(&geometry);
938 }
939
@@ -965,7 +977,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
977 if (pack_everything & ALL_INTO_ONE) {
978 repack_promisor_objects(&po_args, &names);
979
968 - if (existing_nonkept_packs.nr && delete_redundant &&
980 + if (existing.non_kept_packs.nr && delete_redundant &&
981 !(pack_everything & PACK_CRUFT)) {
982 for_each_string_list_item(item, &names) {
983 strvec_pushf(&cmd.args, "--keep-pack=%s-%s.pack",
@@ -1054,8 +1066,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1066
1067 ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix,
1068 cruft_expiration, &names,
1057 - &existing_nonkept_packs,
1058 - &existing_kept_packs);
1069 + &existing);
1070 if (ret)
1071 goto cleanup;
1072
@@ -1086,8 +1097,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1097 pack_prefix,
1098 NULL,
1099 &names,
1089 - &existing_nonkept_packs,
1090 - &existing_kept_packs);
1100 + &existing);
1101 if (ret)
1102 goto cleanup;
1103 }
@@ -1133,7 +1143,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1143
1144 if (delete_redundant && pack_everything & ALL_INTO_ONE) {
1145 const int hexsz = the_hash_algo->hexsz;
1136 - for_each_string_list_item(item, &existing_nonkept_packs) {
1146 + for_each_string_list_item(item, &existing.non_kept_packs) {
1147 char *sha1;
1148 size_t len = strlen(item->string);
1149 if (len < hexsz)
@@ -1152,8 +1162,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1162
1163 if (write_midx) {
1164 struct string_list include = STRING_LIST_INIT_NODUP;
1155 - midx_included_packs(&include, &existing_nonkept_packs,
1156 - &existing_kept_packs, &names, &geometry);
1165 + midx_included_packs(&include, &existing, &names, &geometry);
1166
1167 ret = write_midx_included_packs(&include, &geometry,
1168 refs_snapshot ? get_tempfile_path(refs_snapshot) : NULL,
@@ -1172,7 +1181,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1181
1182 if (delete_redundant) {
1183 int opts = 0;
1175 - for_each_string_list_item(item, &existing_nonkept_packs) {
1184 + for_each_string_list_item(item, &existing.non_kept_packs) {
1185 if (!((uintptr_t)item->util & DELETE_PACK))
1186 continue;
1187 remove_redundant_pack(packdir, item->string);
@@ -1193,7 +1202,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1202 strbuf_strip_suffix(&buf, ".pack");
1203
1204 if ((p->pack_keep) ||
1196 - (string_list_has_string(&existing_kept_packs,
1205 + (string_list_has_string(&existing.kept_packs,
1206 buf.buf)))
1207 continue;
1208
@@ -1224,8 +1233,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1233
1234 cleanup:
1235 string_list_clear(&names, 1);
1227 - string_list_clear(&existing_nonkept_packs, 0);
1228 - string_list_clear(&existing_kept_packs, 0);
1236 + existing_packs_release(&existing);
1237 free_pack_geometry(&geometry);
1238
1239 return ret;