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>
This commit is contained in:
René Scharfe 2022-11-29 13:25:05 +01:00 committed by Junio C Hamano
parent 825babe5d5
commit 0d5448a554

View file

@ -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;