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>

diff --git a/builtin/repack.c b/builtin/repack.c
index 2b43a5be08b720ec2c109764767d87eb8b8b0e02..c3ab89912ea0baa9dd510a89237bba329ccbd4ee 100644 (file)
--- 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;