Re: Comments on recursive merge..

From: Linus Torvalds <torvalds@osdl.org>
Date: Nov 9, 2005, 01:22 UTC
Message-ID: <Pine.LNX.4.64.0511081716450.3247@g5.osdl.org>
In-Reply-To: <7vlkzyd4aq.fsf@assigned-by-dhcp.cox.net>

On Tue, 8 Nov 2005, Junio C Hamano wrote:

Show 6 quoted lines

> 
> I did show-branch soon after we worked on those pathlogical
> merge-base fix, so I would be a bit surprised if I did it
> without using all the knowledge from that exercise, but I do not
> remember offhand.  The core logic should be simple
> generalization of two-head merge-base to N heads.

Hmm.

Look at the "join_revs()" logic, and tell me I'm crazy.

It does:

	struct commit *commit = pop_one_commit(list_p);
	int still_interesting = !!interesting(*list_p);

in that order: it looks whether there are any interesting commits left _after_ it has popped the top-of-stack.

Which means that "still_interesting" can go down to zero if we just popped the last interesting thing off the stack.

Which seems wrong, because the thing we just popped off the stack could easily itself be interesting (in fact, it should be so, 99% of the time), and can cause other interesting commits to be populated back onto the list. So the "still_interesting" flag seems to be wrongly computed: the way it is computed now, it's meaningless.

In contrast, the "merge_base()" thing does

	while (interesting(list)) {
		..
	}

which means that we really will walk the list until there is nothing interesting left. Which is admittedly expensive, but it was how we got rid of the pathological case.

But maybe I'm just missing something really subtle. Maybe git-show-branch does some really clever optimization that is valid.

		Linus