Re: Regression in :/ commit selection between 2.43.0 and 2.47.1

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 6, 2024, 09:52 UTC
Message-ID: <Z1LJSADiStlFicTL@pks.im>
In-Reply-To: <20241205173342.GA2593033@coredump.intra.peff.net>

On Thu, Dec 05, 2024 at 12:33:42PM -0500, Jeff King wrote:

Show 16 quoted lines

> On Thu, Dec 05, 2024 at 02:22:13PM +0200, Aarni Koskela wrote:
> 
> > See below for a reproduction:
> > 
> > * Initialize a repository
> > * Create a commit with message "mystery commit 1"
> > * Tag the commit with `git tag -a a-tag -m a-tag`
> > * Create another commit with message "mystery commit 2" (which should
> > be found from HEAD with `:/mystery`)
> > * Show the commit with `git show :/mystery` – it shows the older commit!
> > * Delete the tag with `git tag -d a-tag`
> > * Show the commit with `git show :/mystery` – it now shows the newer commit.
> 
> This bisects to 57fb139b5e (object-name: fix leaking commit list items,
> 2024-08-01). Looks like an unintended side effect, maybe related to
> swapping "list" and "copy" in get_oid_oneline()?

Indeed, that's it. We have this hunk in that commit:

@@ -1411,14 +1415,14 @@ static int get_oid_oneline(struct repository *r,
        for (l = list; l; l = l->next) {
                l->item->object.flags |= ONELINE_SEEN;
-               commit_list_insert(l->item, &backup);
+               commit_list_insert(l->item, &copy);
        }

The problem with this code is that it's _prepending_ to the copy every
time, and thus we accidentally started to reverse the list. To fix this
we again have to _append_ to it:

diff --git a/object-name.c b/object-name.c
index 240a93e7cef..4c50559ee8c 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1393,7 +1393,7 @@ static int get_oid_oneline(struct repository *r,
 			   const char *prefix, struct object_id *oid,
 			   const struct commit_list *list)
 {
-	struct commit_list *copy = NULL;
+	struct commit_list *copy = NULL, **copy_tail = &copy;
 	const struct commit_list *l;
 	int found = 0;
 	int negative = 0;
@@ -1415,7 +1415,7 @@ static int get_oid_oneline(struct repository *r,
 
 	for (l = list; l; l = l->next) {
 		l->item->object.flags |= ONELINE_SEEN;
-		commit_list_insert(l->item, &copy);
+		copy_tail = &commit_list_insert(l->item, copy_tail)->next;
 	}
 	while (copy) {
 		const char *p, *buf;

I've sent a patch via [1].

[1]: <20241206-pks-rev-parse-fix-reversed-list-v1-1-95a96564a4d7@pks.im>

Patrick