From 56d26ade97135614ccc60cb215d6c9cb22babfb1 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 14 Nov 2023 19:53:49 +0000 Subject: ref-filter.c: really don't sort when using --no-sort When '--no-sort' is passed to 'for-each-ref', 'tag', and 'branch', the printed refs are still sorted by ascending refname. Change the handling of sort options in these commands so that '--no-sort' to truly disables sorting. '--no-sort' does not disable sorting in these commands is because their option parsing does not distinguish between "the absence of '--sort'" (and/or values for tag.sort & branch.sort) and '--no-sort'. Both result in an empty 'sorting_options' string list, which is parsed by 'ref_sorting_options()' to create the 'struct ref_sorting *' for the command. If the string list is empty, 'ref_sorting_options()' interprets that as "the absence of '--sort'" and returns the default ref sorting structure (equivalent to "refname" sort). To handle '--no-sort' properly while preserving the "refname" sort in the "absence of --sort'" case, first explicitly add "refname" to the string list *before* parsing options. This alone doesn't actually change any behavior, since 'compare_refs()' already falls back on comparing refnames if two refs are equal w.r.t all other sort keys. Now that the string list is populated by default, '--no-sort' is the only way to empty the 'sorting_options' string list. Update 'ref_sorting_options()' to return a NULL 'struct ref_sorting *' if the string list is empty, and add a condition to 'ref_array_sort()' to skip the sort altogether if the sort structure is NULL. Note that other functions using 'struct ref_sorting *' do not need any changes because they already ignore NULL values. Finally, remove the condition around sorting in 'ls-remote', since it's no longer necessary. Unlike 'for-each-ref' et. al., it does *not* do any sorting by default. This default is preserved by simply leaving its sort key string list empty before parsing options; if no additional sort keys are set, 'struct ref_sorting *' is NULL and sorting is skipped. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- ref-filter.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) (limited to 'ref-filter.c') diff --git a/ref-filter.c b/ref-filter.c index 9dbc4f71bd..f65551dc68 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -3058,7 +3058,8 @@ void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting, void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array) { - QSORT_S(array->items, array->nr, compare_refs, sorting); + if (sorting) + QSORT_S(array->items, array->nr, compare_refs, sorting); } static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state) @@ -3164,18 +3165,6 @@ static int parse_sorting_atom(const char *atom) return res; } -/* If no sorting option is given, use refname to sort as default */ -static struct ref_sorting *ref_default_sorting(void) -{ - static const char cstr_name[] = "refname"; - - struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting)); - - sorting->next = NULL; - sorting->atom = parse_sorting_atom(cstr_name); - return sorting; -} - static void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg) { struct ref_sorting *s; @@ -3199,9 +3188,7 @@ struct ref_sorting *ref_sorting_options(struct string_list *options) struct string_list_item *item; struct ref_sorting *sorting = NULL, **tail = &sorting; - if (!options->nr) { - sorting = ref_default_sorting(); - } else { + if (options->nr) { for_each_string_list_item(item, options) parse_ref_sorting(tail, item->string); } -- cgit v1.3-5-g9baa From 6d6e5c53b0c3d5566680ee8786d40405db917917 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 14 Nov 2023 19:53:51 +0000 Subject: ref-filter.h: move contains caches into filter Move the 'contains_cache' and 'no_contains_cache' used in filter_refs into an 'internal' struct of the 'struct ref_filter'. In later patches, the 'struct ref_filter *' will be a common data structure across multiple filtering functions. These caches are part of the common functionality the filter struct will support, so they are updated to be internally accessible wherever the filter is used. The design used here mirrors what was introduced in 576de3d956 (unpack_trees: start splitting internal fields from public API, 2023-02-27) for 'unpack_trees_options'. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- ref-filter.c | 14 ++++++-------- ref-filter.h | 6 ++++++ 2 files changed, 12 insertions(+), 8 deletions(-) (limited to 'ref-filter.c') diff --git a/ref-filter.c b/ref-filter.c index f65551dc68..3de4cefcf5 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2680,8 +2680,6 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname) struct ref_filter_cbdata { struct ref_array *array; struct ref_filter *filter; - struct contains_cache contains_cache; - struct contains_cache no_contains_cache; }; /* @@ -2732,11 +2730,11 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, return 0; /* We perform the filtering for the '--contains' option... */ if (filter->with_commit && - !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache)) + !commit_contains(filter, commit, filter->with_commit, &filter->internal.contains_cache)) return 0; /* ...or for the `--no-contains' option */ if (filter->no_commit && - commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache)) + commit_contains(filter, commit, filter->no_commit, &filter->internal.no_contains_cache)) return 0; } @@ -2905,8 +2903,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int save_commit_buffer_orig = save_commit_buffer; save_commit_buffer = 0; - init_contains_cache(&ref_cbdata.contains_cache); - init_contains_cache(&ref_cbdata.no_contains_cache); + init_contains_cache(&filter->internal.contains_cache); + init_contains_cache(&filter->internal.no_contains_cache); /* Simple per-ref filtering */ if (!filter->kind) @@ -2930,8 +2928,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int head_ref(ref_filter_handler, &ref_cbdata); } - clear_contains_cache(&ref_cbdata.contains_cache); - clear_contains_cache(&ref_cbdata.no_contains_cache); + clear_contains_cache(&filter->internal.contains_cache); + clear_contains_cache(&filter->internal.no_contains_cache); /* Filters that need revision walking */ reach_filter(array, &filter->reachable_from, INCLUDE_REACHED); diff --git a/ref-filter.h b/ref-filter.h index d87d61238b..0db3ff5288 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -7,6 +7,7 @@ #include "commit.h" #include "string-list.h" #include "strvec.h" +#include "commit-reach.h" /* Quoting styles */ #define QUOTE_NONE 0 @@ -75,6 +76,11 @@ struct ref_filter { lines; int abbrev, verbose; + + struct { + struct contains_cache contains_cache; + struct contains_cache no_contains_cache; + } internal; }; struct ref_format { -- cgit v1.3-5-g9baa From e7574b0c6b1e0cf2de9e000766a13d23fd9516e6 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 14 Nov 2023 19:53:52 +0000 Subject: ref-filter.h: add functions for filter/format & format-only Add two new public methods to 'ref-filter.h': * 'print_formatted_ref_array()' which, given a format specification & array of ref items, formats and prints the items to stdout. * 'filter_and_format_refs()' which combines 'filter_refs()', 'ref_array_sort()', and 'print_formatted_ref_array()' into a single function. This consolidates much of the code used to filter and format refs in 'builtin/for-each-ref.c', 'builtin/tag.c', and 'builtin/branch.c', reducing duplication and simplifying the future changes needed to optimize the filter & format process. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- builtin/branch.c | 33 +++++++++++++++++---------------- builtin/for-each-ref.c | 27 +-------------------------- builtin/tag.c | 23 +---------------------- ref-filter.c | 35 +++++++++++++++++++++++++++++++++++ ref-filter.h | 14 ++++++++++++++ 5 files changed, 68 insertions(+), 64 deletions(-) (limited to 'ref-filter.c') diff --git a/builtin/branch.c b/builtin/branch.c index 49198b548e..40da31e640 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -437,8 +437,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin { int i; struct ref_array array; - struct strbuf out = STRBUF_INIT; - struct strbuf err = STRBUF_INIT; int maxwidth = 0; const char *remote_prefix = ""; char *to_free = NULL; @@ -468,24 +466,27 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin filter_ahead_behind(the_repository, format, &array); ref_array_sort(sorting, &array); - for (i = 0; i < array.nr; i++) { - strbuf_reset(&err); - strbuf_reset(&out); - if (format_ref_array_item(array.items[i], format, &out, &err)) - die("%s", err.buf); - if (column_active(colopts)) { - assert(!filter->verbose && "--column and --verbose are incompatible"); - /* format to a string_list to let print_columns() do its job */ + if (column_active(colopts)) { + struct strbuf out = STRBUF_INIT, err = STRBUF_INIT; + + assert(!filter->verbose && "--column and --verbose are incompatible"); + + for (i = 0; i < array.nr; i++) { + strbuf_reset(&err); + strbuf_reset(&out); + if (format_ref_array_item(array.items[i], format, &out, &err)) + die("%s", err.buf); + + /* format to a string_list to let print_columns() do its job */ string_list_append(output, out.buf); - } else { - fwrite(out.buf, 1, out.len, stdout); - if (out.len || !format->array_opts.omit_empty) - putchar('\n'); } + + strbuf_release(&err); + strbuf_release(&out); + } else { + print_formatted_ref_array(&array, format); } - strbuf_release(&err); - strbuf_release(&out); ref_array_clear(&array); free(to_free); } diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 881c3ee055..1c19cd5bd3 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -19,15 +19,11 @@ static char const * const for_each_ref_usage[] = { int cmd_for_each_ref(int argc, const char **argv, const char *prefix) { - int i, total; struct ref_sorting *sorting; struct string_list sorting_options = STRING_LIST_INIT_DUP; int icase = 0; - struct ref_array array; struct ref_filter filter = REF_FILTER_INIT; struct ref_format format = REF_FORMAT_INIT; - struct strbuf output = STRBUF_INIT; - struct strbuf err = STRBUF_INIT; int from_stdin = 0; struct strvec vec = STRVEC_INIT; @@ -61,8 +57,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_END(), }; - memset(&array, 0, sizeof(array)); - format.format = "%(objectname) %(objecttype)\t%(refname)"; git_config(git_default_config, NULL); @@ -104,27 +98,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) } filter.match_as_path = 1; - filter_refs(&array, &filter, FILTER_REFS_ALL); - filter_ahead_behind(the_repository, &format, &array); - - ref_array_sort(sorting, &array); - - total = format.array_opts.max_count; - if (!total || array.nr < total) - total = array.nr; - for (i = 0; i < total; i++) { - strbuf_reset(&err); - strbuf_reset(&output); - if (format_ref_array_item(array.items[i], &format, &output, &err)) - die("%s", err.buf); - fwrite(output.buf, 1, output.len, stdout); - if (output.len || !format.array_opts.omit_empty) - putchar('\n'); - } + filter_and_format_refs(&filter, FILTER_REFS_ALL, sorting, &format); - strbuf_release(&err); - strbuf_release(&output); - ref_array_clear(&array); ref_filter_clear(&filter); ref_sorting_release(sorting); strvec_clear(&vec); diff --git a/builtin/tag.c b/builtin/tag.c index 2d599245d4..2528d499dd 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -48,13 +48,7 @@ static int config_sign_tag = -1; /* unspecified */ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, struct ref_format *format) { - struct ref_array array; - struct strbuf output = STRBUF_INIT; - struct strbuf err = STRBUF_INIT; char *to_free = NULL; - int i; - - memset(&array, 0, sizeof(array)); if (filter->lines == -1) filter->lines = 0; @@ -72,23 +66,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, if (verify_ref_format(format)) die(_("unable to parse format string")); filter->with_commit_tag_algo = 1; - filter_refs(&array, filter, FILTER_REFS_TAGS); - filter_ahead_behind(the_repository, format, &array); - ref_array_sort(sorting, &array); - - for (i = 0; i < array.nr; i++) { - strbuf_reset(&output); - strbuf_reset(&err); - if (format_ref_array_item(array.items[i], format, &output, &err)) - die("%s", err.buf); - fwrite(output.buf, 1, output.len, stdout); - if (output.len || !format->array_opts.omit_empty) - putchar('\n'); - } + filter_and_format_refs(filter, FILTER_REFS_TAGS, sorting, format); - strbuf_release(&err); - strbuf_release(&output); - ref_array_clear(&array); free(to_free); return 0; diff --git a/ref-filter.c b/ref-filter.c index 3de4cefcf5..f062bb4938 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2939,6 +2939,18 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int return ret; } +void filter_and_format_refs(struct ref_filter *filter, unsigned int type, + struct ref_sorting *sorting, + struct ref_format *format) +{ + struct ref_array array = { 0 }; + filter_refs(&array, filter, type); + filter_ahead_behind(the_repository, format, &array); + ref_array_sort(sorting, &array); + print_formatted_ref_array(&array, format); + ref_array_clear(&array); +} + static int compare_detached_head(struct ref_array_item *a, struct ref_array_item *b) { if (!(a->kind ^ b->kind)) @@ -3128,6 +3140,29 @@ int format_ref_array_item(struct ref_array_item *info, return 0; } +void print_formatted_ref_array(struct ref_array *array, struct ref_format *format) +{ + int total; + struct strbuf output = STRBUF_INIT, err = STRBUF_INIT; + + total = format->array_opts.max_count; + if (!total || array->nr < total) + total = array->nr; + for (int i = 0; i < total; i++) { + strbuf_reset(&err); + strbuf_reset(&output); + if (format_ref_array_item(array->items[i], format, &output, &err)) + die("%s", err.buf); + if (output.len || !format->array_opts.omit_empty) { + fwrite(output.buf, 1, output.len, stdout); + putchar('\n'); + } + } + + strbuf_release(&err); + strbuf_release(&output); +} + void pretty_print_ref(const char *name, const struct object_id *oid, struct ref_format *format) { diff --git a/ref-filter.h b/ref-filter.h index 0db3ff5288..0ce5af58ab 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -137,6 +137,14 @@ struct ref_format { * filtered refs in the ref_array structure. */ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type); +/* + * Filter refs using the given ref_filter and type, sort the contents + * according to the given ref_sorting, format the filtered refs with the + * given ref_format, and print them to stdout. + */ +void filter_and_format_refs(struct ref_filter *filter, unsigned int type, + struct ref_sorting *sorting, + struct ref_format *format); /* Clear all memory allocated to ref_array */ void ref_array_clear(struct ref_array *array); /* Used to verify if the given format is correct and to parse out the used atoms */ @@ -161,6 +169,12 @@ char *get_head_description(void); /* Set up translated strings in the output. */ void setup_ref_filter_porcelain_msg(void); +/* + * Print up to maxcount ref_array elements to stdout using the given + * ref_format. + */ +void print_formatted_ref_array(struct ref_array *array, struct ref_format *format); + /* * Print a single ref, outside of any ref-filter. Note that the * name must be a fully qualified refname. -- cgit v1.3-5-g9baa From 9c575b020232c5af0d2068aa82ce390923b81646 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 14 Nov 2023 19:53:53 +0000 Subject: ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()' Rename 'ref_filter_handler()' to 'filter_one()' to more clearly distinguish it from other ref filtering callbacks that will be added in later patches. The "*_one()" naming convention is common throughout the codebase for iteration callbacks. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- ref-filter.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'ref-filter.c') diff --git a/ref-filter.c b/ref-filter.c index f062bb4938..02a852bb34 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2686,7 +2686,7 @@ struct ref_filter_cbdata { * A call-back given to for_each_ref(). Filter refs and keep them for * later object processing. */ -static int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data) +static int filter_one(const char *refname, const struct object_id *oid, int flag, void *cb_data) { struct ref_filter_cbdata *ref_cbdata = cb_data; struct ref_filter *filter = ref_cbdata->filter; @@ -2917,15 +2917,15 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int * of filter_ref_kind(). */ if (filter->kind == FILTER_REFS_BRANCHES) - ret = for_each_fullref_in("refs/heads/", ref_filter_handler, &ref_cbdata); + ret = for_each_fullref_in("refs/heads/", filter_one, &ref_cbdata); else if (filter->kind == FILTER_REFS_REMOTES) - ret = for_each_fullref_in("refs/remotes/", ref_filter_handler, &ref_cbdata); + ret = for_each_fullref_in("refs/remotes/", filter_one, &ref_cbdata); else if (filter->kind == FILTER_REFS_TAGS) - ret = for_each_fullref_in("refs/tags/", ref_filter_handler, &ref_cbdata); + ret = for_each_fullref_in("refs/tags/", filter_one, &ref_cbdata); else if (filter->kind & FILTER_REFS_ALL) - ret = for_each_fullref_in_pattern(filter, ref_filter_handler, &ref_cbdata); + ret = for_each_fullref_in_pattern(filter, filter_one, &ref_cbdata); if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD)) - head_ref(ref_filter_handler, &ref_cbdata); + head_ref(filter_one, &ref_cbdata); } clear_contains_cache(&filter->internal.contains_cache); -- cgit v1.3-5-g9baa From 613d9915499ed9d1fd21d743002d17ef972e9e57 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 14 Nov 2023 19:53:54 +0000 Subject: ref-filter.c: refactor to create common helper functions Factor out parts of 'ref_array_push()', 'ref_filter_handler()', and 'filter_refs()' into new helper functions: * Extract the code to grow a 'struct ref_array' and append a given 'struct ref_array_item *' to it from 'ref_array_push()' into 'ref_array_append()'. * Extract the code to filter a given ref by refname & object ID then create a new 'struct ref_array_item *' from 'filter_one()' into 'apply_ref_filter()'. * Extract the code for filter pre-processing, contains cache creation, and ref iteration from 'filter_refs()' into 'do_filter_refs()'. In later patches, these helpers will be used by new ref-filter API functions. This patch does not result in any user-facing behavior changes or changes to callers outside of 'ref-filter.c'. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- ref-filter.c | 115 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 69 insertions(+), 46 deletions(-) (limited to 'ref-filter.c') diff --git a/ref-filter.c b/ref-filter.c index 02a852bb34..8bfdc66e44 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2632,15 +2632,18 @@ static struct ref_array_item *new_ref_array_item(const char *refname, return ref; } +static void ref_array_append(struct ref_array *array, struct ref_array_item *ref) +{ + ALLOC_GROW(array->items, array->nr + 1, array->alloc); + array->items[array->nr++] = ref; +} + struct ref_array_item *ref_array_push(struct ref_array *array, const char *refname, const struct object_id *oid) { struct ref_array_item *ref = new_ref_array_item(refname, oid); - - ALLOC_GROW(array->items, array->nr + 1, array->alloc); - array->items[array->nr++] = ref; - + ref_array_append(array, ref); return ref; } @@ -2677,46 +2680,36 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname) return ref_kind_from_refname(refname); } -struct ref_filter_cbdata { - struct ref_array *array; - struct ref_filter *filter; -}; - -/* - * A call-back given to for_each_ref(). Filter refs and keep them for - * later object processing. - */ -static int filter_one(const char *refname, const struct object_id *oid, int flag, void *cb_data) +static struct ref_array_item *apply_ref_filter(const char *refname, const struct object_id *oid, + int flag, struct ref_filter *filter) { - struct ref_filter_cbdata *ref_cbdata = cb_data; - struct ref_filter *filter = ref_cbdata->filter; struct ref_array_item *ref; struct commit *commit = NULL; unsigned int kind; if (flag & REF_BAD_NAME) { warning(_("ignoring ref with broken name %s"), refname); - return 0; + return NULL; } if (flag & REF_ISBROKEN) { warning(_("ignoring broken ref %s"), refname); - return 0; + return NULL; } /* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */ kind = filter_ref_kind(filter, refname); if (!(kind & filter->kind)) - return 0; + return NULL; if (!filter_pattern_match(filter, refname)) - return 0; + return NULL; if (filter_exclude_match(filter, refname)) - return 0; + return NULL; if (filter->points_at.nr && !match_points_at(&filter->points_at, oid, refname)) - return 0; + return NULL; /* * A merge filter is applied on refs pointing to commits. Hence @@ -2727,15 +2720,15 @@ static int filter_one(const char *refname, const struct object_id *oid, int flag filter->with_commit || filter->no_commit || filter->verbose) { commit = lookup_commit_reference_gently(the_repository, oid, 1); if (!commit) - return 0; + return NULL; /* We perform the filtering for the '--contains' option... */ if (filter->with_commit && !commit_contains(filter, commit, filter->with_commit, &filter->internal.contains_cache)) - return 0; + return NULL; /* ...or for the `--no-contains' option */ if (filter->no_commit && commit_contains(filter, commit, filter->no_commit, &filter->internal.no_contains_cache)) - return 0; + return NULL; } /* @@ -2743,11 +2736,32 @@ static int filter_one(const char *refname, const struct object_id *oid, int flag * to do its job and the resulting list may yet to be pruned * by maxcount logic. */ - ref = ref_array_push(ref_cbdata->array, refname, oid); + ref = new_ref_array_item(refname, oid); ref->commit = commit; ref->flag = flag; ref->kind = kind; + return ref; +} + +struct ref_filter_cbdata { + struct ref_array *array; + struct ref_filter *filter; +}; + +/* + * A call-back given to for_each_ref(). Filter refs and keep them for + * later object processing. + */ +static int filter_one(const char *refname, const struct object_id *oid, int flag, void *cb_data) +{ + struct ref_filter_cbdata *ref_cbdata = cb_data; + struct ref_array_item *ref; + + ref = apply_ref_filter(refname, oid, flag, ref_cbdata->filter); + if (ref) + ref_array_append(ref_cbdata->array, ref); + return 0; } @@ -2883,26 +2897,12 @@ void filter_ahead_behind(struct repository *r, free(commits); } -/* - * API for filtering a set of refs. Based on the type of refs the user - * has requested, we iterate through those refs and apply filters - * as per the given ref_filter structure and finally store the - * filtered refs in the ref_array structure. - */ -int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type) +static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref_fn fn, void *cb_data) { - struct ref_filter_cbdata ref_cbdata; - int save_commit_buffer_orig; int ret = 0; - ref_cbdata.array = array; - ref_cbdata.filter = filter; - filter->kind = type & FILTER_REFS_KIND_MASK; - save_commit_buffer_orig = save_commit_buffer; - save_commit_buffer = 0; - init_contains_cache(&filter->internal.contains_cache); init_contains_cache(&filter->internal.no_contains_cache); @@ -2917,20 +2917,43 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int * of filter_ref_kind(). */ if (filter->kind == FILTER_REFS_BRANCHES) - ret = for_each_fullref_in("refs/heads/", filter_one, &ref_cbdata); + ret = for_each_fullref_in("refs/heads/", fn, cb_data); else if (filter->kind == FILTER_REFS_REMOTES) - ret = for_each_fullref_in("refs/remotes/", filter_one, &ref_cbdata); + ret = for_each_fullref_in("refs/remotes/", fn, cb_data); else if (filter->kind == FILTER_REFS_TAGS) - ret = for_each_fullref_in("refs/tags/", filter_one, &ref_cbdata); + ret = for_each_fullref_in("refs/tags/", fn, cb_data); else if (filter->kind & FILTER_REFS_ALL) - ret = for_each_fullref_in_pattern(filter, filter_one, &ref_cbdata); + ret = for_each_fullref_in_pattern(filter, fn, cb_data); if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD)) - head_ref(filter_one, &ref_cbdata); + head_ref(fn, cb_data); } clear_contains_cache(&filter->internal.contains_cache); clear_contains_cache(&filter->internal.no_contains_cache); + return ret; +} + +/* + * API for filtering a set of refs. Based on the type of refs the user + * has requested, we iterate through those refs and apply filters + * as per the given ref_filter structure and finally store the + * filtered refs in the ref_array structure. + */ +int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type) +{ + struct ref_filter_cbdata ref_cbdata; + int save_commit_buffer_orig; + int ret = 0; + + ref_cbdata.array = array; + ref_cbdata.filter = filter; + + save_commit_buffer_orig = save_commit_buffer; + save_commit_buffer = 0; + + ret = do_filter_refs(filter, type, filter_one, &ref_cbdata); + /* Filters that need revision walking */ reach_filter(array, &filter->reachable_from, INCLUDE_REACHED); reach_filter(array, &filter->unreachable_from, EXCLUDE_REACHED); -- cgit v1.3-5-g9baa From bd98f9774e10808276c13f8438aa3c71093bc6a4 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 14 Nov 2023 19:53:55 +0000 Subject: ref-filter.c: filter & format refs in the same callback Update 'filter_and_format_refs()' to try to perform ref filtering & formatting in a single ref iteration, without an intermediate 'struct ref_array'. This can only be done if no operations need to be performed on a pre-filtered array; specifically, if the refs are - filtered on reachability, - sorted, or - formatted with ahead-behind information they cannot be filtered & formatted in the same iteration. In that case, fall back on the current filter-then-sort-then-format flow. This optimization substantially improves memory usage due to no longer storing a ref array in memory. In some cases, it also dramatically reduces runtime (e.g. 'git for-each-ref --no-sort --count=1', which no longer loads all refs into a 'struct ref_array' to printing only the first ref). Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- ref-filter.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 6 deletions(-) (limited to 'ref-filter.c') diff --git a/ref-filter.c b/ref-filter.c index 8bfdc66e44..ca4ebed4eb 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2779,6 +2779,49 @@ static void free_array_item(struct ref_array_item *item) free(item); } +struct ref_filter_and_format_cbdata { + struct ref_filter *filter; + struct ref_format *format; + + struct ref_filter_and_format_internal { + int count; + } internal; +}; + +static int filter_and_format_one(const char *refname, const struct object_id *oid, int flag, void *cb_data) +{ + struct ref_filter_and_format_cbdata *ref_cbdata = cb_data; + struct ref_array_item *ref; + struct strbuf output = STRBUF_INIT, err = STRBUF_INIT; + + ref = apply_ref_filter(refname, oid, flag, ref_cbdata->filter); + if (!ref) + return 0; + + if (format_ref_array_item(ref, ref_cbdata->format, &output, &err)) + die("%s", err.buf); + + if (output.len || !ref_cbdata->format->array_opts.omit_empty) { + fwrite(output.buf, 1, output.len, stdout); + putchar('\n'); + } + + strbuf_release(&output); + strbuf_release(&err); + free_array_item(ref); + + /* + * Increment the running count of refs that match the filter. If + * max_count is set and we've reached the max, stop the ref + * iteration by returning a nonzero value. + */ + if (ref_cbdata->format->array_opts.max_count && + ++ref_cbdata->internal.count >= ref_cbdata->format->array_opts.max_count) + return 1; + + return 0; +} + /* Free all memory allocated for ref_array */ void ref_array_clear(struct ref_array *array) { @@ -2962,16 +3005,49 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int return ret; } +static inline int can_do_iterative_format(struct ref_filter *filter, + struct ref_sorting *sorting, + struct ref_format *format) +{ + /* + * Filtering & formatting results within a single ref iteration + * callback is not compatible with options that require + * post-processing a filtered ref_array. These include: + * - filtering on reachability + * - sorting the filtered results + * - including ahead-behind information in the formatted output + */ + return !(filter->reachable_from || + filter->unreachable_from || + sorting || + format->bases.nr); +} + void filter_and_format_refs(struct ref_filter *filter, unsigned int type, struct ref_sorting *sorting, struct ref_format *format) { - struct ref_array array = { 0 }; - filter_refs(&array, filter, type); - filter_ahead_behind(the_repository, format, &array); - ref_array_sort(sorting, &array); - print_formatted_ref_array(&array, format); - ref_array_clear(&array); + if (can_do_iterative_format(filter, sorting, format)) { + int save_commit_buffer_orig; + struct ref_filter_and_format_cbdata ref_cbdata = { + .filter = filter, + .format = format, + }; + + save_commit_buffer_orig = save_commit_buffer; + save_commit_buffer = 0; + + do_filter_refs(filter, type, filter_and_format_one, &ref_cbdata); + + save_commit_buffer = save_commit_buffer_orig; + } else { + struct ref_array array = { 0 }; + filter_refs(&array, filter, type); + filter_ahead_behind(the_repository, format, &array); + ref_array_sort(sorting, &array); + print_formatted_ref_array(&array, format); + ref_array_clear(&array); + } } static int compare_detached_head(struct ref_array_item *a, struct ref_array_item *b) -- cgit v1.3-5-g9baa From 188782ecb1d326ff724e41a77ea8ccb72f21ad44 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 14 Nov 2023 19:53:57 +0000 Subject: ref-filter.c: use peeled tag for '*' format fields In most builtins ('rev-parse ^{}', 'show-ref --dereference'), "dereferencing" a tag refers to a recursive peel of the tag object. Unlike these cases, the dereferencing prefix ('*') in 'for-each-ref' format specifiers triggers only a single, non-recursive dereference of a given tag object. For most annotated tags, a single dereference is all that is needed to access the tag's associated commit or tree; "recursive" and "non-recursive" dereferencing are functionally equivalent in these cases. However, nested tags (annotated tags whose target is another annotated tag) dereferenced once return another tag, where a recursive dereference would return the commit or tree. Currently, if a user wants to filter & format refs and include information about a recursively-dereferenced tag, they can do so with something like 'cat-file --batch-check': git for-each-ref --format="%(objectname)^{} %(refname)" | git cat-file --batch-check="%(objectname) %(rest)" But the combination of commands is inefficient. So, to improve the performance of this use case and align the defererencing behavior of 'for-each-ref' with that of other commands, update the ref formatting code to use the peeled tag (from 'peel_iterated_oid()') to populate '*' fields rather than the tag's immediate target object (from 'get_tagged_oid()'). Additionally, add a test to 't6300-for-each-ref' to verify new nested tag behavior and update 't6302-for-each-ref-filter.sh' to print the correct value for nested dereferenced fields. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- Documentation/git-for-each-ref.txt | 4 ++-- ref-filter.c | 13 ++++--------- t/t6300-for-each-ref.sh | 22 ++++++++++++++++++++++ t/t6302-for-each-ref-filter.sh | 4 ++-- 4 files changed, 30 insertions(+), 13 deletions(-) (limited to 'ref-filter.c') diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index f64fa8b406..9fa98e5872 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -296,8 +296,8 @@ from the `committer` or `tagger` fields depending on the object type. These are intended for working on a mix of annotated and lightweight tags. For tag objects, a `fieldname` prefixed with an asterisk (`*`) expands to -the `fieldname` value of object the tag points at, rather than that of the -tag object itself. +the `fieldname` value of the peeled object, rather than that of the tag +object itself. Fields that have name-email-date tuple as its value (`author`, `committer`, and `tagger`) can be suffixed with `name`, `email`, diff --git a/ref-filter.c b/ref-filter.c index ca4ebed4eb..b1093aebb8 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2424,17 +2424,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) return 0; /* - * If it is a tag object, see if we use a value that derefs - * the object, and if we do grab the object it refers to. + * If it is a tag object, see if we use the peeled value. If we do, + * grab the peeled OID. */ - oi_deref.oid = *get_tagged_oid((struct tag *)obj); + if (need_tagged && peel_iterated_oid(&obj->oid, &oi_deref.oid)) + die("bad tag"); - /* - * NEEDSWORK: This derefs tag only once, which - * is good to deal with chains of trust, but - * is not consistent with what deref_tag() does - * which peels the onion to the core. - */ return get_object(ref, 1, &obj, &oi_deref, err); } diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 119720495e..d62ba985da 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -1728,6 +1728,28 @@ test_expect_success 'git for-each-ref with non-existing refs' ' test_must_be_empty actual ' +test_expect_success 'git for-each-ref with nested tags' ' + git tag -am "Normal tag" nested/base HEAD && + git tag -am "Nested tag" nested/nest1 refs/tags/nested/base && + git tag -am "Double nested tag" nested/nest2 refs/tags/nested/nest1 && + + head_oid="$(git rev-parse HEAD)" && + base_tag_oid="$(git rev-parse refs/tags/nested/base)" && + nest1_tag_oid="$(git rev-parse refs/tags/nested/nest1)" && + nest2_tag_oid="$(git rev-parse refs/tags/nested/nest2)" && + + cat >expect <<-EOF && + refs/tags/nested/base $base_tag_oid tag $head_oid commit + refs/tags/nested/nest1 $nest1_tag_oid tag $head_oid commit + refs/tags/nested/nest2 $nest2_tag_oid tag $head_oid commit + EOF + + git for-each-ref \ + --format="%(refname) %(objectname) %(objecttype) %(*objectname) %(*objecttype)" \ + refs/tags/nested/ >actual && + test_cmp expect actual +' + GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)" TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)" diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh index af223e44d6..82f3d1ea0f 100755 --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -45,8 +45,8 @@ test_expect_success 'check signed tags with --points-at' ' sed -e "s/Z$//" >expect <<-\EOF && refs/heads/side Z refs/tags/annotated-tag four - refs/tags/doubly-annotated-tag An annotated tag - refs/tags/doubly-signed-tag A signed tag + refs/tags/doubly-annotated-tag four + refs/tags/doubly-signed-tag four refs/tags/four Z refs/tags/signed-tag four EOF -- cgit v1.3-5-g9baa