pack-objects: simplify --filter handling

pack-objects uses OPT_PARSE_LIST_OBJECTS_FILTER_INIT() to initialize the
a rev_info struct lazily before populating its filter member using the
--filter option values.  It tracks whether the initialization is needed
using the .have_revs member of the callback data.

There is a better way: Use a stand-alone list_objects_filter_options
struct and build a rev_info struct with its .filter member after option
parsing.  This allows using the simpler OPT_PARSE_LIST_OBJECTS_FILTER()
and getting rid of the extra callback mechanism.

Even simpler would be using a struct rev_info as before 5cb28270a1
(pack-objects: lazily set up "struct rev_info", don't leak, 2022-03-28),
but that would expose a memory leak caused by repo_init_revisions()
followed by release_revisions() without a setup_revisions() call in
between.

Using list_objects_filter_options also allows pushing the rev_info
struct into get_object_list(), where it arguably belongs. Either way,
this is all left for later.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c702c09dd45e7fc84e6867c5449012a7e10c1e51..2193f80b897270deda441885fcfe82af4997ad91 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4149,22 +4149,6 @@ static int option_parse_cruft_expiration(const struct option *opt,
        return 0;
 }
 
-struct po_filter_data {
-       unsigned have_revs:1;
-       struct rev_info revs;
-};
-
-static struct list_objects_filter_options *po_filter_revs_init(void *value)
-{
-       struct po_filter_data *data = value;
-
-       if (!data->have_revs)
-               repo_init_revisions(the_repository, &data->revs, NULL);
-       data->have_revs = 1;
-
-       return &data->revs.filter;
-}
-
 int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 {
        int use_internal_rev_list = 0;
@@ -4175,7 +4159,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
        int rev_list_index = 0;
        int stdin_packs = 0;
        struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
-       struct po_filter_data pfd = { .have_revs = 0 };
+       struct list_objects_filter_options filter_options =
+               LIST_OBJECTS_FILTER_INIT;
 
        struct option pack_objects_options[] = {
                OPT_SET_INT('q', "quiet", &progress,
@@ -4266,7 +4251,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
                              &write_bitmap_index,
                              N_("write a bitmap index if possible"),
                              WRITE_BITMAP_QUIET, PARSE_OPT_HIDDEN),
-               OPT_PARSE_LIST_OBJECTS_FILTER_INIT(&pfd, po_filter_revs_init),
+               OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
                OPT_CALLBACK_F(0, "missing", NULL, N_("action"),
                  N_("handling for missing objects"), PARSE_OPT_NONEG,
                  option_parse_missing_action),
@@ -4386,7 +4371,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
        if (!rev_list_all || !rev_list_reflog || !rev_list_index)
                unpack_unreachable_expiration = 0;
 
-       if (pfd.have_revs && pfd.revs.filter.choice) {
+       if (filter_options.choice) {
                if (!pack_to_stdout)
                        die(_("cannot use --filter without --stdout"));
                if (stdin_packs)
@@ -4473,13 +4458,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
                read_cruft_objects();
        } else if (!use_internal_rev_list) {
                read_object_list_from_stdin();
-       } else if (pfd.have_revs) {
-               get_object_list(&pfd.revs, rp.nr, rp.v);
-               release_revisions(&pfd.revs);
        } else {
                struct rev_info revs;
 
                repo_init_revisions(the_repository, &revs, NULL);
+               list_objects_filter_copy(&revs.filter, &filter_options);
                get_object_list(&revs, rp.nr, rp.v);
                release_revisions(&revs);
        }
@@ -4514,6 +4497,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
                           reuse_packfile_objects);
 
 cleanup:
+       list_objects_filter_release(&filter_options);
        strvec_clear(&rp);
 
        return 0;