[PATCH 1/8] builtin/repack.c: extract structure to store existing packs
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Sep 5, 2023, 20:36 UTC
- Message-ID
- <5b48b7e3cc03c83465a3dcecaa98b9d2e9667084.1693946195.git.me@ttaylorr.com>
- In-Reply-To
- <cover.1693946195.git.me@ttaylorr.com>
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> --- builtin/repack.c | 90 ++++++++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 41 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c index 2b43a5be08..c3ab89912e 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -95,14 +95,29 @@ static int repack_config(const char *var, const char *value, return git_default_config(var, value, ctx, cb); } +struct existing_packs { + struct string_list kept_packs; + struct string_list non_kept_packs; +}; + +#define EXISTING_PACKS_INIT { \ + .kept_packs = STRING_LIST_INIT_DUP, \ + .non_kept_packs = STRING_LIST_INIT_DUP, \ +} + +static void existing_packs_release(struct existing_packs *existing) +{ + string_list_clear(&existing->kept_packs, 0); + string_list_clear(&existing->non_kept_packs, 0); +} + /* - * Adds all packs hex strings (pack-$HASH) to either fname_nonkept_list - * or fname_kept_list based on whether each pack has a corresponding + * Adds all packs hex strings (pack-$HASH) to either packs->non_kept + * or packs->kept based on whether each pack has a corresponding * .keep file or not. Packs without a .keep file are not to be kept * if we are going to pack everything into one file. */ -static void collect_pack_filenames(struct string_list *fname_nonkept_list, - struct string_list *fname_kept_list, +static void collect_pack_filenames(struct existing_packs *existing, const struct string_list *extra_keep) { struct packed_git *p; @@ -126,16 +141,16 @@ static void collect_pack_filenames(struct string_list *fname_nonkept_list, strbuf_strip_suffix(&buf, ".pack"); if ((extra_keep->nr > 0 && i < extra_keep->nr) || p->pack_keep) - string_list_append(fname_kept_list, buf.buf); + string_list_append(&existing->kept_packs, buf.buf); else { struct string_list_item *item; - item = string_list_append(fname_nonkept_list, buf.buf); + item = string_list_append(&existing->non_kept_packs, buf.buf); if (p->is_cruft) item->util = (void*)(uintptr_t)CRUFT_PACK; } } - string_list_sort(fname_kept_list); + string_list_sort(&existing->kept_packs); strbuf_release(&buf); } @@ -327,7 +342,7 @@ static int geometry_cmp(const void *va, const void *vb) } static void init_pack_geometry(struct pack_geometry *geometry, - struct string_list *existing_kept_packs, + struct existing_packs *existing, const struct pack_objects_args *args) { struct packed_git *p; @@ -344,23 +359,24 @@ static void init_pack_geometry(struct pack_geometry *geometry, if (!pack_kept_objects) { /* - * Any pack that has its pack_keep bit set will appear - * in existing_kept_packs below, but this saves us from - * doing a more expensive check. + * Any pack that has its pack_keep bit set will + * appear in existing->kept_packs below, but + * this saves us from doing a more expensive + * check. */ if (p->pack_keep) continue; /* - * The pack may be kept via the --keep-pack option; - * check 'existing_kept_packs' to determine whether to - * ignore it. + * The pack may be kept via the --keep-pack + * option; check 'existing->kept_packs' to + * determine whether to ignore it. */ strbuf_reset(&buf); strbuf_addstr(&buf, pack_basename(p)); strbuf_strip_suffix(&buf, ".pack"); - if (string_list_has_string(existing_kept_packs, buf.buf)) + if (string_list_has_string(&existing->kept_packs, buf.buf)) continue; } if (p->is_cruft) @@ -565,14 +581,13 @@ static void midx_snapshot_refs(struct tempfile *f) } static void midx_included_packs(struct string_list *include, - struct string_list *existing_nonkept_packs, - struct string_list *existing_kept_packs, + struct existing_packs *existing, struct string_list *names, struct pack_geometry *geometry) { struct string_list_item *item; - for_each_string_list_item(item, existing_kept_packs) + for_each_string_list_item(item, &existing->kept_packs) string_list_insert(include, xstrfmt("%s.idx", item->string)); for_each_string_list_item(item, names) string_list_insert(include, xstrfmt("pack-%s.idx", item->string)); @@ -600,7 +615,7 @@ static void midx_included_packs(struct string_list *include, string_list_insert(include, strbuf_detach(&buf, NULL)); } - for_each_string_list_item(item, existing_nonkept_packs) { + for_each_string_list_item(item, &existing->non_kept_packs) { if (!((uintptr_t)item->util & CRUFT_PACK)) { /* * no need to check DELETE_PACK, since we're not @@ -611,7 +626,7 @@ static void midx_included_packs(struct string_list *include, string_list_insert(include, xstrfmt("%s.idx", item->string)); } } else { - for_each_string_list_item(item, existing_nonkept_packs) { + for_each_string_list_item(item, &existing->non_kept_packs) { if ((uintptr_t)item->util & DELETE_PACK) continue; string_list_insert(include, xstrfmt("%s.idx", item->string)); @@ -700,8 +715,7 @@ static int write_cruft_pack(const struct pack_objects_args *args, const char *pack_prefix, const char *cruft_expiration, struct string_list *names, - struct string_list *existing_packs, - struct string_list *existing_kept_packs) + struct existing_packs *existing) { struct child_process cmd = CHILD_PROCESS_INIT; struct strbuf line = STRBUF_INIT; @@ -744,9 +758,9 @@ static int write_cruft_pack(const struct pack_objects_args *args, in = xfdopen(cmd.in, "w"); for_each_string_list_item(item, names) fprintf(in, "%s-%s.pack\n", pack_prefix, item->string); - for_each_string_list_item(item, existing_packs) + for_each_string_list_item(item, &existing->non_kept_packs) fprintf(in, "-%s.pack\n", item->string); - for_each_string_list_item(item, existing_kept_packs) + for_each_string_list_item(item, &existing->kept_packs) fprintf(in, "%s.pack\n", item->string); fclose(in); @@ -778,8 +792,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) struct child_process cmd = CHILD_PROCESS_INIT; struct string_list_item *item; struct string_list names = STRING_LIST_INIT_DUP; - struct string_list existing_nonkept_packs = STRING_LIST_INIT_DUP; - struct string_list existing_kept_packs = STRING_LIST_INIT_DUP; + struct existing_packs existing = EXISTING_PACKS_INIT; struct pack_geometry geometry = { 0 }; struct strbuf line = STRBUF_INIT; struct tempfile *refs_snapshot = NULL; @@ -915,13 +928,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix) packtmp_name = xstrfmt(".tmp-%d-pack", (int)getpid()); packtmp = mkpathdup("%s/%s", packdir, packtmp_name); - collect_pack_filenames(&existing_nonkept_packs, &existing_kept_packs, - &keep_pack_list); + collect_pack_filenames(&existing, &keep_pack_list); if (geometry.split_factor) { if (pack_everything) die(_("options '%s' and '%s' cannot be used together"), "--geometric", "-A/-a"); - init_pack_geometry(&geometry, &existing_kept_packs, &po_args); + init_pack_geometry(&geometry, &existing, &po_args); split_pack_geometry(&geometry); } @@ -965,7 +977,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (pack_everything & ALL_INTO_ONE) { repack_promisor_objects(&po_args, &names); - if (existing_nonkept_packs.nr && delete_redundant && + if (existing.non_kept_packs.nr && delete_redundant && !(pack_everything & PACK_CRUFT)) { for_each_string_list_item(item, &names) { strvec_pushf(&cmd.args, "--keep-pack=%s-%s.pack", @@ -1054,8 +1066,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix, cruft_expiration, &names, - &existing_nonkept_packs, - &existing_kept_packs); + &existing); if (ret) goto cleanup; @@ -1086,8 +1097,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) pack_prefix, NULL, &names, - &existing_nonkept_packs, - &existing_kept_packs); + &existing); if (ret) goto cleanup; } @@ -1133,7 +1143,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (delete_redundant && pack_everything & ALL_INTO_ONE) { const int hexsz = the_hash_algo->hexsz; - for_each_string_list_item(item, &existing_nonkept_packs) { + for_each_string_list_item(item, &existing.non_kept_packs) { char *sha1; size_t len = strlen(item->string); if (len < hexsz) @@ -1152,8 +1162,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (write_midx) { struct string_list include = STRING_LIST_INIT_NODUP; - midx_included_packs(&include, &existing_nonkept_packs, - &existing_kept_packs, &names, &geometry); + midx_included_packs(&include, &existing, &names, &geometry); ret = write_midx_included_packs(&include, &geometry, refs_snapshot ? get_tempfile_path(refs_snapshot) : NULL, @@ -1172,7 +1181,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (delete_redundant) { int opts = 0; - for_each_string_list_item(item, &existing_nonkept_packs) { + for_each_string_list_item(item, &existing.non_kept_packs) { if (!((uintptr_t)item->util & DELETE_PACK)) continue; remove_redundant_pack(packdir, item->string); @@ -1193,7 +1202,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) strbuf_strip_suffix(&buf, ".pack"); if ((p->pack_keep) || - (string_list_has_string(&existing_kept_packs, + (string_list_has_string(&existing.kept_packs, buf.buf))) continue; @@ -1224,8 +1233,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) cleanup: string_list_clear(&names, 1); - string_list_clear(&existing_nonkept_packs, 0); - string_list_clear(&existing_kept_packs, 0); + existing_packs_release(&existing); free_pack_geometry(&geometry); return ret;
-- 2.42.0.119.gca7d13e7bf