ref-filter: move ahead-behind bases into used_atom

verify_ref_format() parses a ref-filter format string and stores recognized items in the static array "used_atom". For "ahead-behind:<committish>" it stores the committish part in a string_list member "bases" of struct ref_format. ref_sorting_options() also parses bare ref-filter format items and stores stores recognized ones in "used_atom" as well. The committish parts go to a dummy struct ref_format in parse_sorting_atom(), though, and are leaked and forgotten. If verify_ref_format() is called before ref_sorting_options(), like in git for-each-ref, then all works well if the sort key is included in the format string. If it isn't then sorting cannot work as the committishes are missing. If ref_sorting_options() is called first, like in git branch, then we have the additional issue that if the sort key is included in the format string then filter_ahead_behind() can't see its committish, will not generate any results for it and thus it will be expanded to an empty string. Fix those issues by replacing the string_list with a field in used_atom for storing the committish. This way it can be shared for handling both ref-filter format strings and sorting options in the same command. Reported-by: Ross Goldberg <ross.goldberg@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed Jan 18, 2025 at 18:11 UTC 5e58db65750a20ae6d0c620f8a2010e038d408f5
4 files changed +59 -26
builtin/branch.c
+1 -1
@@ -473,7 +473,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
473 if (verify_ref_format(format))
474 die(_("unable to parse format string"));
475
476 - filter_ahead_behind(the_repository, format, &array);
476 + filter_ahead_behind(the_repository, &array);
477 ref_array_sort(sorting, &array);
478
479 if (column_active(colopts)) {
ref-filter.c
+30 -20
@@ -235,6 +235,9 @@ static struct used_atom {
235 enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
236 S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option;
237 } signature;
238 + struct {
239 + struct commit *commit;
240 + } base;
241 struct strvec describe_args;
242 struct refname_atom refname;
243 char *head;
@@ -891,18 +894,15 @@ static int rest_atom_parser(struct ref_format *format UNUSED,
894 return 0;
895 }
896
894 -static int ahead_behind_atom_parser(struct ref_format *format,
895 - struct used_atom *atom UNUSED,
897 +static int ahead_behind_atom_parser(struct ref_format *format UNUSED,
898 + struct used_atom *atom,
899 const char *arg, struct strbuf *err)
900 {
898 - struct string_list_item *item;
899 -
901 if (!arg)
902 return strbuf_addf_ret(err, -1, _("expected format: %%(ahead-behind:<committish>)"));
903
903 - item = string_list_append(&format->bases, arg);
904 - item->util = lookup_commit_reference_by_name(arg);
905 - if (!item->util)
904 + atom->u.base.commit = lookup_commit_reference_by_name(arg);
905 + if (!atom->u.base.commit)
906 die("failed to find '%s'", arg);
907
908 return 0;
@@ -3084,22 +3084,30 @@ static void reach_filter(struct ref_array *array,
3084 }
3085
3086 void filter_ahead_behind(struct repository *r,
3087 - struct ref_format *format,
3087 struct ref_array *array)
3088 {
3089 struct commit **commits;
3091 - size_t commits_nr = format->bases.nr + array->nr;
3090 + size_t bases_nr, commits_nr;
3091
3093 - if (!format->bases.nr || !array->nr)
3092 + if (!array->nr)
3093 return;
3094
3096 - ALLOC_ARRAY(commits, commits_nr);
3097 - for (size_t i = 0; i < format->bases.nr; i++)
3098 - commits[i] = format->bases.items[i].util;
3095 + for (size_t i = bases_nr = 0; i < used_atom_cnt; i++) {
3096 + if (used_atom[i].atom_type == ATOM_AHEADBEHIND)
3097 + bases_nr++;
3098 + }
3099 + if (!bases_nr)
3100 + return;
3101
3100 - ALLOC_ARRAY(array->counts, st_mult(format->bases.nr, array->nr));
3102 + ALLOC_ARRAY(commits, st_add(bases_nr, array->nr));
3103 + for (size_t i = 0, j = 0; i < used_atom_cnt; i++) {
3104 + if (used_atom[i].atom_type == ATOM_AHEADBEHIND)
3105 + commits[j++] = used_atom[i].u.base.commit;
3106 + }
3107
3102 - commits_nr = format->bases.nr;
3108 + ALLOC_ARRAY(array->counts, st_mult(bases_nr, array->nr));
3109 +
3110 + commits_nr = bases_nr;
3111 array->counts_nr = 0;
3112 for (size_t i = 0; i < array->nr; i++) {
3113 const char *name = array->items[i]->refname;
@@ -3108,8 +3116,8 @@ void filter_ahead_behind(struct repository *r,
3116 if (!commits[commits_nr])
3117 continue;
3118
3111 - CALLOC_ARRAY(array->items[i]->counts, format->bases.nr);
3112 - for (size_t j = 0; j < format->bases.nr; j++) {
3119 + CALLOC_ARRAY(array->items[i]->counts, bases_nr);
3120 + for (size_t j = 0; j < bases_nr; j++) {
3121 struct ahead_behind_count *count;
3122 count = &array->counts[array->counts_nr++];
3123 count->tip_index = commits_nr;
@@ -3277,9 +3285,12 @@ static inline int can_do_iterative_format(struct ref_filter *filter,
3285 * - filtering on reachability
3286 * - including ahead-behind information in the formatted output
3287 */
3288 + for (size_t i = 0; i < used_atom_cnt; i++) {
3289 + if (used_atom[i].atom_type == ATOM_AHEADBEHIND)
3290 + return 0;
3291 + }
3292 return !(filter->reachable_from ||
3293 filter->unreachable_from ||
3282 - format->bases.nr ||
3294 format->is_base_tips.nr);
3295 }
3296
@@ -3303,7 +3314,7 @@ void filter_and_format_refs(struct ref_filter *filter, unsigned int type,
3314 } else {
3315 struct ref_array array = { 0 };
3316 filter_refs(&array, filter, type);
3306 - filter_ahead_behind(the_repository, format, &array);
3317 + filter_ahead_behind(the_repository, &array);
3318 filter_is_base(the_repository, format, &array);
3319 ref_array_sort(sorting, &array);
3320 print_formatted_ref_array(&array, format);
@@ -3647,7 +3658,6 @@ void ref_format_init(struct ref_format *format)
3658
3659 void ref_format_clear(struct ref_format *format)
3660 {
3650 - string_list_clear(&format->bases, 0);
3661 string_list_clear(&format->is_base_tips, 0);
3662 ref_format_init(format);
3663 }
ref-filter.h
-5
@@ -99,9 +99,6 @@ struct ref_format {
99 /* Internal state to ref-filter */
100 int need_color_reset_at_eol;
101
102 - /* List of bases for ahead-behind counts. */
103 - struct string_list bases;
104 -
102 /* List of bases for is-base indicators. */
103 struct string_list is_base_tips;
104
@@ -117,7 +114,6 @@ struct ref_format {
114 }
115 #define REF_FORMAT_INIT { \
116 .use_color = -1, \
120 - .bases = STRING_LIST_INIT_DUP, \
117 .is_base_tips = STRING_LIST_INIT_DUP, \
118 }
119
@@ -205,7 +201,6 @@ struct ref_array_item *ref_array_push(struct ref_array *array,
201 * If this is not called, then any ahead-behind atoms will be blank.
202 */
203 void filter_ahead_behind(struct repository *r,
208 - struct ref_format *format,
204 struct ref_array *array);
205
206 /*
t/t3203-branch-output.sh
+28
@@ -368,6 +368,34 @@ test_expect_success 'git branch --format with ahead-behind' '
368 test_cmp expect actual
369 '
370
371 +test_expect_success 'git branch `--sort=[-]ahead-behind` option' '
372 + cat >expect <<-\EOF &&
373 + (HEAD detached from fromtag) 0 0
374 + refs/heads/ambiguous 0 0
375 + refs/heads/branch-two 0 0
376 + refs/heads/branch-one 1 0
377 + refs/heads/main 1 0
378 + refs/heads/ref-to-branch 1 0
379 + refs/heads/ref-to-remote 1 0
380 + EOF
381 + git branch --format="%(refname) %(ahead-behind:HEAD)" \
382 + --sort=refname --sort=ahead-behind:HEAD >actual &&
383 + test_cmp expect actual &&
384 +
385 + cat >expect <<-\EOF &&
386 + (HEAD detached from fromtag) 0 0
387 + refs/heads/branch-one 1 0
388 + refs/heads/main 1 0
389 + refs/heads/ref-to-branch 1 0
390 + refs/heads/ref-to-remote 1 0
391 + refs/heads/ambiguous 0 0
392 + refs/heads/branch-two 0 0
393 + EOF
394 + git branch --format="%(refname) %(ahead-behind:HEAD)" \
395 + --sort=refname --sort=-ahead-behind:HEAD >actual &&
396 + test_cmp expect actual
397 +'
398 +
399 test_expect_success 'git branch with --format=%(rest) must fail' '
400 test_must_fail git branch --format="%(rest)" >actual
401 '