Re: [PATCH 2/2] commit: use a priority queue in merge base functions

[Jeff King <peff@peff.net>]

On Wed, Aug 29, 2012 at 05:00:32PM -0400, Jeff King wrote:

> > Hmm, this does seem to break t6024 for me, though.
> 
> Probably because:
> 
> >  	/* Clean up the result to remove stale ones */
> > -	free_commit_list(list);
> > -	list = result; result = NULL;
> > -	while (list) {
> > -		struct commit_list *next = list->next;
> > -		if (!(list->item->object.flags & STALE))
> > -			commit_list_insert_by_date(list->item, &result);
> > -		free(list);
> > -		list = next;
> > -	}
> > -	return result;
> > +	while (result.nr)
> > +		commit_list_append(queue_pop(&result), &tail);
> > +	queue_clear(&result);
> > +	queue_clear(&list);
> > +	return ret;
> 
> I forgot to to port the STALE flag handling here.

You would want this on top:

diff --git a/commit.c b/commit.c
index c64ef94..8259871 100644
--- a/commit.c
+++ b/commit.c
@@ -661,8 +661,11 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co
 	}
 
 	/* Clean up the result to remove stale ones */
-	while (result.nr)
-		commit_list_append(queue_pop(&result), &tail);
+	while (result.nr) {
+		struct commit *commit = queue_pop(&result);
+		if (!(commit->object.flags & STALE))
+			commit_list_append(commit, &tail);
+	}
 	queue_clear(&result);
 	queue_clear(&list);
 	return ret;

but t6024 still fails (it clearly is finding a different merge base than
the test expects).  I'll trace through it, but it will have to be later
tonight.

-Peff