repack: move `find_pack_prefix()` out of the builtin

Both callers within the repack builtin which call functions that take a
'write_pack_opts' structure have the following pattern:

    struct write_pack_opts opts = {
        .packdir = packdir,
        .packtmp = packtmp,
        .pack_prefix = find_pack_prefix(packdir, packtmp),
        /* ... */
    };
    int ret = write_some_kind_of_pack(&opts, /* ... */);

, but both "packdir" and "packtmp" are fields within the write_pack_opts
struct itself!

Instead of also computing the pack_prefix ahead of time, let's have the
callees compute it themselves by moving `find_pack_prefix()` out of the
repack builtin, and have it take a write_pack_opts pointer instead of
the "packdir" and "packtmp" fields directly.

This avoids the callers having to do some prep work that is common
between the two of them, but also avoids the potential pitfall of
accidentally writing:

    .pack_prefix = find_pack_prefix(packtmp, packdir),

(which is well-typed) when the caller meant to instead write:

    .pack_prefix = find_pack_prefix(packdir, packtmp),

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 7295135ec214fb5122d97c4d8c6247f7c525a7d8..b21799c650e0e3301080ceb4a5a58b5d9469a25c 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -149,6 +149,7 @@ static int write_filtered_pack(const struct write_pack_opts *opts,
        const char *caret;
        const char *scratch;
        int local = skip_prefix(opts->destination, opts->packdir, &scratch);
+       const char *pack_prefix = write_pack_opts_pack_prefix(opts);
 
        prepare_pack_objects(&cmd, opts->po_args, opts->destination);
 
@@ -173,7 +174,7 @@ static int write_filtered_pack(const struct write_pack_opts *opts,
         */
        in = xfdopen(cmd.in, "w");
        for_each_string_list_item(item, names)
-               fprintf(in, "^%s-%s.pack\n", opts->pack_prefix, item->string);
+               fprintf(in, "^%s-%s.pack\n", pack_prefix, item->string);
        for_each_string_list_item(item, &existing->non_kept_packs)
                fprintf(in, "%s.pack\n", item->string);
        for_each_string_list_item(item, &existing->cruft_packs)
@@ -233,6 +234,7 @@ static int write_cruft_pack(const struct write_pack_opts *opts,
        int ret;
        const char *scratch;
        int local = skip_prefix(opts->destination, opts->packdir, &scratch);
+       const char *pack_prefix = write_pack_opts_pack_prefix(opts);
 
        prepare_pack_objects(&cmd, opts->po_args, opts->destination);
 
@@ -265,7 +267,7 @@ static int write_cruft_pack(const struct write_pack_opts *opts,
         */
        in = xfdopen(cmd.in, "w");
        for_each_string_list_item(item, names)
-               fprintf(in, "%s-%s.pack\n", opts->pack_prefix, item->string);
+               fprintf(in, "%s-%s.pack\n", pack_prefix, item->string);
        if (combine_cruft_below_size && !cruft_expiration) {
                combine_small_cruft_packs(in, combine_cruft_below_size,
                                          existing);
@@ -283,17 +285,6 @@ static int write_cruft_pack(const struct write_pack_opts *opts,
                                       local);
 }
 
-static const char *find_pack_prefix(const char *packdir, const char *packtmp)
-{
-       const char *pack_prefix;
-       if (!skip_prefix(packtmp, packdir, &pack_prefix))
-               die(_("pack prefix %s does not begin with objdir %s"),
-                   packtmp, packdir);
-       if (*pack_prefix == '/')
-               pack_prefix++;
-       return pack_prefix;
-}
-
 int cmd_repack(int argc,
               const char **argv,
               const char *prefix,
@@ -596,11 +587,9 @@ int cmd_repack(int argc,
        }
 
        if (pack_everything & PACK_CRUFT) {
-               const char *pack_prefix = find_pack_prefix(packdir, packtmp);
                struct write_pack_opts opts = {
                        .po_args = &cruft_po_args,
                        .destination = packtmp,
-                       .pack_prefix = pack_prefix,
                        .packtmp = packtmp,
                        .packdir = packdir,
                };
@@ -667,7 +656,6 @@ int cmd_repack(int argc,
                struct write_pack_opts opts = {
                        .po_args = &po_args,
                        .destination = filter_to,
-                       .pack_prefix = find_pack_prefix(packdir, packtmp),
                        .packdir = packdir,
                        .packtmp = packtmp,
                };

diff --git a/repack.c b/repack.c
index 1d485e01124e923d2a211691989c7e806039cedd..19fd1d6d5ba96a73d0071a245bded2888b77bb79 100644 (file)
--- a/repack.c
+++ b/repack.c
@@ -66,6 +66,17 @@ void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,
        strbuf_release(&buf);
 }
 
+const char *write_pack_opts_pack_prefix(const struct write_pack_opts *opts)
+{
+       const char *pack_prefix;
+       if (!skip_prefix(opts->packtmp, opts->packdir, &pack_prefix))
+               die(_("pack prefix %s does not begin with objdir %s"),
+                   opts->packtmp, opts->packdir);
+       if (*pack_prefix == '/')
+               pack_prefix++;
+       return pack_prefix;
+}
+
 #define DELETE_PACK 1
 #define RETAIN_PACK 2
 

diff --git a/repack.h b/repack.h
index 6ef503f62376c5eb413212b7f29708f0d36ef07b..5852e2407f8da5331cf2be3b05d3f12055f4e949 100644 (file)
--- a/repack.h
+++ b/repack.h
@@ -35,11 +35,12 @@ void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,
 struct write_pack_opts {
        struct pack_objects_args *po_args;
        const char *destination;
-       const char *pack_prefix;
        const char *packdir;
        const char *packtmp;
 };
 
+const char *write_pack_opts_pack_prefix(const struct write_pack_opts *opts);
+
 struct repository;
 struct packed_git;