From 5e58db65750a20ae6d0c620f8a2010e038d408f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Jan 2025 18:11:34 +0100 Subject: [PATCH 1/3] ref-filter: move ahead-behind bases into used_atom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verify_ref_format() parses a ref-filter format string and stores recognized items in the static array "used_atom". For "ahead-behind:" 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 Helped-by: Jeff King Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/branch.c | 2 +- ref-filter.c | 50 ++++++++++++++++++++++++---------------- ref-filter.h | 5 ---- t/t3203-branch-output.sh | 28 ++++++++++++++++++++++ 4 files changed, 59 insertions(+), 26 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 6e7b0cfddb..fbb9536282 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -473,7 +473,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin if (verify_ref_format(format)) die(_("unable to parse format string")); - filter_ahead_behind(the_repository, format, &array); + filter_ahead_behind(the_repository, &array); ref_array_sort(sorting, &array); if (column_active(colopts)) { diff --git a/ref-filter.c b/ref-filter.c index 23054694c2..250e412c47 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -235,6 +235,9 @@ static struct used_atom { enum { S_BARE, S_GRADE, S_SIGNER, S_KEY, S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option; } signature; + struct { + struct commit *commit; + } base; struct strvec describe_args; struct refname_atom refname; char *head; @@ -891,18 +894,15 @@ static int rest_atom_parser(struct ref_format *format UNUSED, return 0; } -static int ahead_behind_atom_parser(struct ref_format *format, - struct used_atom *atom UNUSED, +static int ahead_behind_atom_parser(struct ref_format *format UNUSED, + struct used_atom *atom, const char *arg, struct strbuf *err) { - struct string_list_item *item; - if (!arg) return strbuf_addf_ret(err, -1, _("expected format: %%(ahead-behind:)")); - item = string_list_append(&format->bases, arg); - item->util = lookup_commit_reference_by_name(arg); - if (!item->util) + atom->u.base.commit = lookup_commit_reference_by_name(arg); + if (!atom->u.base.commit) die("failed to find '%s'", arg); return 0; @@ -3084,22 +3084,30 @@ static void reach_filter(struct ref_array *array, } void filter_ahead_behind(struct repository *r, - struct ref_format *format, struct ref_array *array) { struct commit **commits; - size_t commits_nr = format->bases.nr + array->nr; + size_t bases_nr, commits_nr; - if (!format->bases.nr || !array->nr) + if (!array->nr) return; - ALLOC_ARRAY(commits, commits_nr); - for (size_t i = 0; i < format->bases.nr; i++) - commits[i] = format->bases.items[i].util; + for (size_t i = bases_nr = 0; i < used_atom_cnt; i++) { + if (used_atom[i].atom_type == ATOM_AHEADBEHIND) + bases_nr++; + } + if (!bases_nr) + return; - ALLOC_ARRAY(array->counts, st_mult(format->bases.nr, array->nr)); + ALLOC_ARRAY(commits, st_add(bases_nr, array->nr)); + for (size_t i = 0, j = 0; i < used_atom_cnt; i++) { + if (used_atom[i].atom_type == ATOM_AHEADBEHIND) + commits[j++] = used_atom[i].u.base.commit; + } - commits_nr = format->bases.nr; + ALLOC_ARRAY(array->counts, st_mult(bases_nr, array->nr)); + + commits_nr = bases_nr; array->counts_nr = 0; for (size_t i = 0; i < array->nr; i++) { const char *name = array->items[i]->refname; @@ -3108,8 +3116,8 @@ void filter_ahead_behind(struct repository *r, if (!commits[commits_nr]) continue; - CALLOC_ARRAY(array->items[i]->counts, format->bases.nr); - for (size_t j = 0; j < format->bases.nr; j++) { + CALLOC_ARRAY(array->items[i]->counts, bases_nr); + for (size_t j = 0; j < bases_nr; j++) { struct ahead_behind_count *count; count = &array->counts[array->counts_nr++]; count->tip_index = commits_nr; @@ -3277,9 +3285,12 @@ static inline int can_do_iterative_format(struct ref_filter *filter, * - filtering on reachability * - including ahead-behind information in the formatted output */ + for (size_t i = 0; i < used_atom_cnt; i++) { + if (used_atom[i].atom_type == ATOM_AHEADBEHIND) + return 0; + } return !(filter->reachable_from || filter->unreachable_from || - format->bases.nr || format->is_base_tips.nr); } @@ -3303,7 +3314,7 @@ void filter_and_format_refs(struct ref_filter *filter, unsigned int type, } else { struct ref_array array = { 0 }; filter_refs(&array, filter, type); - filter_ahead_behind(the_repository, format, &array); + filter_ahead_behind(the_repository, &array); filter_is_base(the_repository, format, &array); ref_array_sort(sorting, &array); print_formatted_ref_array(&array, format); @@ -3647,7 +3658,6 @@ void ref_format_init(struct ref_format *format) void ref_format_clear(struct ref_format *format) { - string_list_clear(&format->bases, 0); string_list_clear(&format->is_base_tips, 0); ref_format_init(format); } diff --git a/ref-filter.h b/ref-filter.h index 754038ab07..5f3dd6c931 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -99,9 +99,6 @@ struct ref_format { /* Internal state to ref-filter */ int need_color_reset_at_eol; - /* List of bases for ahead-behind counts. */ - struct string_list bases; - /* List of bases for is-base indicators. */ struct string_list is_base_tips; @@ -117,7 +114,6 @@ struct ref_format { } #define REF_FORMAT_INIT { \ .use_color = -1, \ - .bases = STRING_LIST_INIT_DUP, \ .is_base_tips = STRING_LIST_INIT_DUP, \ } @@ -205,7 +201,6 @@ struct ref_array_item *ref_array_push(struct ref_array *array, * If this is not called, then any ahead-behind atoms will be blank. */ void filter_ahead_behind(struct repository *r, - struct ref_format *format, struct ref_array *array); /* diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh index 500c9d0e72..a6bd88a58d 100755 --- a/t/t3203-branch-output.sh +++ b/t/t3203-branch-output.sh @@ -368,6 +368,34 @@ test_expect_success 'git branch --format with ahead-behind' ' test_cmp expect actual ' +test_expect_success 'git branch `--sort=[-]ahead-behind` option' ' + cat >expect <<-\EOF && + (HEAD detached from fromtag) 0 0 + refs/heads/ambiguous 0 0 + refs/heads/branch-two 0 0 + refs/heads/branch-one 1 0 + refs/heads/main 1 0 + refs/heads/ref-to-branch 1 0 + refs/heads/ref-to-remote 1 0 + EOF + git branch --format="%(refname) %(ahead-behind:HEAD)" \ + --sort=refname --sort=ahead-behind:HEAD >actual && + test_cmp expect actual && + + cat >expect <<-\EOF && + (HEAD detached from fromtag) 0 0 + refs/heads/branch-one 1 0 + refs/heads/main 1 0 + refs/heads/ref-to-branch 1 0 + refs/heads/ref-to-remote 1 0 + refs/heads/ambiguous 0 0 + refs/heads/branch-two 0 0 + EOF + git branch --format="%(refname) %(ahead-behind:HEAD)" \ + --sort=refname --sort=-ahead-behind:HEAD >actual && + test_cmp expect actual +' + test_expect_success 'git branch with --format=%(rest) must fail' ' test_must_fail git branch --format="%(rest)" >actual ' From 7ee4fd18ace71d187ee3ea5ba745a6a3493e0e0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Jan 2025 18:11:51 +0100 Subject: [PATCH 2/3] ref-filter: move is-base tip to used_atom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The string_list "is_base_tips" in struct ref_format stores the committish part of "is-base:". It has the same problems that its sibling string_list "bases" had. Fix them the same way as the previous commit did for the latter, by replacing the string_list with fields in "used_atom". Helped-by: Jeff King Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- ref-filter.c | 56 +++++++++++++++++++++++++------------------ ref-filter.h | 5 ---- t/t6600-test-reach.sh | 29 ++++++++++++++++++++++ 3 files changed, 62 insertions(+), 28 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 250e412c47..1c474224a0 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -236,6 +236,7 @@ static struct used_atom { S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option; } signature; struct { + char *name; struct commit *commit; } base; struct strvec describe_args; @@ -908,18 +909,16 @@ static int ahead_behind_atom_parser(struct ref_format *format UNUSED, return 0; } -static int is_base_atom_parser(struct ref_format *format, - struct used_atom *atom UNUSED, +static int is_base_atom_parser(struct ref_format *format UNUSED, + struct used_atom *atom, const char *arg, struct strbuf *err) { - struct string_list_item *item; - if (!arg) return strbuf_addf_ret(err, -1, _("expected format: %%(is-base:)")); - item = string_list_append(&format->is_base_tips, arg); - item->util = lookup_commit_reference_by_name(arg); - if (!item->util) + atom->u.base.name = xstrdup(arg); + atom->u.base.commit = lookup_commit_reference_by_name(arg); + if (!atom->u.base.commit) die("failed to find '%s'", arg); return 0; @@ -3009,6 +3008,8 @@ void ref_array_clear(struct ref_array *array) free(atom->u.head); else if (atom->atom_type == ATOM_DESCRIBE) strvec_clear(&atom->u.describe_args); + else if (atom->atom_type == ATOM_ISBASE) + free(atom->u.base.name); else if (atom->atom_type == ATOM_TRAILERS || (atom->atom_type == ATOM_CONTENTS && atom->u.contents.option == C_TRAILERS)) { @@ -3133,14 +3134,20 @@ void filter_ahead_behind(struct repository *r, } void filter_is_base(struct repository *r, - struct ref_format *format, struct ref_array *array) { struct commit **bases; - size_t bases_nr = 0; + size_t bases_nr = 0, is_base_nr; struct ref_array_item **back_index; - if (!format->is_base_tips.nr || !array->nr) + if (!array->nr) + return; + + for (size_t i = is_base_nr = 0; i < used_atom_cnt; i++) { + if (used_atom[i].atom_type == ATOM_ISBASE) + is_base_nr++; + } + if (!is_base_nr) return; CALLOC_ARRAY(back_index, array->nr); @@ -3150,7 +3157,7 @@ void filter_is_base(struct repository *r, const char *name = array->items[i]->refname; struct commit *c = lookup_commit_reference_by_name_gently(name, 1); - CALLOC_ARRAY(array->items[i]->is_base, format->is_base_tips.nr); + CALLOC_ARRAY(array->items[i]->is_base, is_base_nr); if (!c) continue; @@ -3160,15 +3167,20 @@ void filter_is_base(struct repository *r, bases_nr++; } - for (size_t i = 0; i < format->is_base_tips.nr; i++) { - struct commit *tip = format->is_base_tips.items[i].util; - int base_index = get_branch_base_for_tip(r, tip, bases, bases_nr); + for (size_t i = 0, j = 0; i < used_atom_cnt; i++) { + struct commit *tip; + int base_index; + if (used_atom[i].atom_type != ATOM_ISBASE) + continue; + + tip = used_atom[i].u.base.commit; + base_index = get_branch_base_for_tip(r, tip, bases, bases_nr); if (base_index < 0) continue; /* Store the string for use in output later. */ - back_index[base_index]->is_base[i] = xstrdup(format->is_base_tips.items[i].string); + back_index[base_index]->is_base[j++] = xstrdup(used_atom[i].u.base.name); } free(back_index); @@ -3260,8 +3272,7 @@ struct ref_sorting { }; static inline int can_do_iterative_format(struct ref_filter *filter, - struct ref_sorting *sorting, - struct ref_format *format) + struct ref_sorting *sorting) { /* * Reference backends sort patterns lexicographically by refname, so if @@ -3288,17 +3299,17 @@ static inline int can_do_iterative_format(struct ref_filter *filter, for (size_t i = 0; i < used_atom_cnt; i++) { if (used_atom[i].atom_type == ATOM_AHEADBEHIND) return 0; + if (used_atom[i].atom_type == ATOM_ISBASE) + return 0; } - return !(filter->reachable_from || - filter->unreachable_from || - format->is_base_tips.nr); + return !(filter->reachable_from || filter->unreachable_from); } void filter_and_format_refs(struct ref_filter *filter, unsigned int type, struct ref_sorting *sorting, struct ref_format *format) { - if (can_do_iterative_format(filter, sorting, format)) { + if (can_do_iterative_format(filter, sorting)) { int save_commit_buffer_orig; struct ref_filter_and_format_cbdata ref_cbdata = { .filter = filter, @@ -3315,7 +3326,7 @@ void filter_and_format_refs(struct ref_filter *filter, unsigned int type, struct ref_array array = { 0 }; filter_refs(&array, filter, type); filter_ahead_behind(the_repository, &array); - filter_is_base(the_repository, format, &array); + filter_is_base(the_repository, &array); ref_array_sort(sorting, &array); print_formatted_ref_array(&array, format); ref_array_clear(&array); @@ -3658,6 +3669,5 @@ void ref_format_init(struct ref_format *format) void ref_format_clear(struct ref_format *format) { - string_list_clear(&format->is_base_tips, 0); ref_format_init(format); } diff --git a/ref-filter.h b/ref-filter.h index 5f3dd6c931..0ba94df651 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -99,9 +99,6 @@ struct ref_format { /* Internal state to ref-filter */ int need_color_reset_at_eol; - /* List of bases for is-base indicators. */ - struct string_list is_base_tips; - struct { int max_count; int omit_empty; @@ -114,7 +111,6 @@ struct ref_format { } #define REF_FORMAT_INIT { \ .use_color = -1, \ - .is_base_tips = STRING_LIST_INIT_DUP, \ } /* Macros for checking --merged and --no-merged options */ @@ -210,7 +206,6 @@ void filter_ahead_behind(struct repository *r, * If this is not called, then any is-base atoms will be blank. */ void filter_is_base(struct repository *r, - struct ref_format *format, struct ref_array *array); void ref_filter_init(struct ref_filter *filter); diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh index 2591f8b8b3..6638d1aa1d 100755 --- a/t/t6600-test-reach.sh +++ b/t/t6600-test-reach.sh @@ -733,4 +733,33 @@ test_expect_success 'for-each-ref is-base:multiple' ' --format="%(refname)[%(is-base:commit-2-3)-%(is-base:commit-6-5)]" --stdin ' +test_expect_success 'for-each-ref is-base: --sort' ' + cat >input <<-\EOF && + refs/heads/commit-1-1 + refs/heads/commit-4-2 + refs/heads/commit-4-4 + refs/heads/commit-8-4 + EOF + + cat >expect <<-\EOF && + refs/heads/commit-1-1 + refs/heads/commit-4-4 + refs/heads/commit-8-4 + refs/heads/commit-4-2 + EOF + run_all_modes git for-each-ref \ + --format="%(refname)" --stdin \ + --sort=refname --sort=is-base:commit-2-3 && + + cat >expect <<-\EOF && + refs/heads/commit-4-2 + refs/heads/commit-1-1 + refs/heads/commit-4-4 + refs/heads/commit-8-4 + EOF + run_all_modes git for-each-ref \ + --format="%(refname)" --stdin \ + --sort=refname --sort=-is-base:commit-2-3 +' + test_done From c5490ce9d1b625516b17253d1d2d0352730b7b84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Jan 2025 18:11:59 +0100 Subject: [PATCH 3/3] ref-filter: remove ref_format_clear() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that ref_format_clear() no longer releases any memory we don't need it anymore. Remove it and its counterpart, ref_format_init(). Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/branch.c | 1 - builtin/for-each-ref.c | 1 - builtin/tag.c | 1 - builtin/verify-tag.c | 1 - ref-filter.c | 11 ----------- ref-filter.h | 3 --- 6 files changed, 18 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index fbb9536282..9a29de5bf1 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -884,7 +884,6 @@ int cmd_branch(int argc, string_list_clear(&output, 0); ref_sorting_release(sorting); ref_filter_clear(&filter); - ref_format_clear(&format); ret = 0; goto out; diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 715745a262..8085ebd8fe 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -108,7 +108,6 @@ int cmd_for_each_ref(int argc, filter_and_format_refs(&filter, flags, sorting, &format); ref_filter_clear(&filter); - ref_format_clear(&format); ref_sorting_release(sorting); strvec_clear(&vec); return 0; diff --git a/builtin/tag.c b/builtin/tag.c index c4bd145831..e8a344b926 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -698,7 +698,6 @@ int cmd_tag(int argc, cleanup: ref_sorting_release(sorting); ref_filter_clear(&filter); - ref_format_clear(&format); strbuf_release(&buf); strbuf_release(&ref); strbuf_release(&reflog_msg); diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index a7f20618ff..f6b97048a5 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -69,6 +69,5 @@ int cmd_verify_tag(int argc, if (format.format) pretty_print_ref(name, &oid, &format); } - ref_format_clear(&format); return had_error; } diff --git a/ref-filter.c b/ref-filter.c index 1c474224a0..2ec72d66bb 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -3660,14 +3660,3 @@ void ref_filter_clear(struct ref_filter *filter) free_commit_list(filter->unreachable_from); ref_filter_init(filter); } - -void ref_format_init(struct ref_format *format) -{ - struct ref_format blank = REF_FORMAT_INIT; - memcpy(format, &blank, sizeof(blank)); -} - -void ref_format_clear(struct ref_format *format) -{ - ref_format_init(format); -} diff --git a/ref-filter.h b/ref-filter.h index 0ba94df651..013d4cfa64 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -211,7 +211,4 @@ void filter_is_base(struct repository *r, void ref_filter_init(struct ref_filter *filter); void ref_filter_clear(struct ref_filter *filter); -void ref_format_init(struct ref_format *format); -void ref_format_clear(struct ref_format *format); - #endif /* REF_FILTER_H */