git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] ref-filter: share bases and is_base_tips between formatting and sorting

From
Jeff King <peff@peff.net>
Date
Jan 16, 2025, 10:21 UTC
Message-ID
<20250116102154.GA724126@coredump.intra.peff.net>
In-Reply-To
<20250116100637.GB144555@coredump.intra.peff.net>
On Thu, Jan 16, 2025 at 05:06:37AM -0500, Jeff King wrote:
Show 13 quoted lines
> On Thu, Jan 16, 2025 at 04:51:28AM -0500, Jeff King wrote:
> 
> > Yet another option in the near term might be storing these ahead-behind
> > bits in the individual atoms. Since the point is to do a single
> > traversal, we'd have to marshal them into a unified data structure at
> > some point. But we already do that! In filter_ahead_behind() we convert
> > the string list into an array (and ironically do not even look at the
> > strings, only their "util" fields).
> > 
> > So something like this (only lightly tested) seems to work:
> 
> I compiled it without DEVELOPER=1, so I missed a few unused parameters.
> We'd want this on top:

And one final thought on this approach: if we do want to do it, perhaps it would make sense to build on top of the patch you sent. I think yours fixes the bug in a more direct and obvious way, and then my approach would merely be internal reorganization on top.

At any rate, here is the is_base conversion for posterity.
diff --git a/ref-filter.c b/ref-filter.c
index 0de51f13e6..28d48ec585 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:<committish>)"));
 
-	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;
@@ -3137,14 +3136,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;
 	struct ref_array_item **back_index;
+	size_t atoms_nr;
 
-	if (!format->is_base_tips.nr || !array->nr)
+	atoms_nr = 0;
+	for (size_t i = 0; i < used_atom_cnt; i++) {
+		if (used_atom[i].atom_type == ATOM_ISBASE)
+			atoms_nr++;
+	}
+
+	if (!atoms_nr || !array->nr)
 		return;
 
 	CALLOC_ARRAY(back_index, array->nr);
@@ -3154,7 +3159,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, atoms_nr);
 
 		if (!c)
 			continue;
@@ -3164,15 +3169,21 @@ 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);
@@ -3264,8 +3275,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
@@ -3292,17 +3302,18 @@ 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);
+		 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,
@@ -3319,7 +3330,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);
@@ -3662,6 +3673,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);
Previous: Jeff KingNext: Junio C Hamano
Message 9 of 19 in “Bug sorting git branch output by ahead-behind:HEAD”
  1. Ross GoldbergJan 5, 2025
  2. ref-filter: share bases and is_base_tips between formatting and sortingRené Scharfe, Jan 12, 2025
  3. Jeff KingJan 13, 2025
  4. Jeff KingJan 13, 2025
  5. Junio C HamanoJan 13, 2025
  6. René ScharfeJan 14, 2025
  7. Jeff KingJan 16, 2025
  8. Jeff KingJan 16, 2025
  9. Jeff KingJan 16, 2025
  10. Junio C HamanoJan 16, 2025
  11. René ScharfeJan 18, 2025
  12. René ScharfeJan 19, 2025
  13. René ScharfeJan 19, 2025
  14. René ScharfeJan 19, 2025
  15. Jeff KingJan 19, 2025
  16. René ScharfeJan 18, 2025
  17. 1/3 ref-filter: move ahead-behind bases into used_atomRené Scharfe, Jan 18, 2025
  18. 3/3 ref-filter: remove ref_format_clear()René Scharfe, Jan 18, 2025
  19. 2/3 ref-filter: move is-base tip to used_atomRené Scharfe, Jan 18, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.