Re: [PATCH 4/2] Fix parent rewriting in --early-output

On Mon, 12 Nov 2007, Junio C Hamano wrote:

It's subtle. And you're probably right, I need to fix it up some more.

It works, but it works for all the wrong reasons. When thinking about it, you need to keep three "generations" of commits in mind. Let's call them "c" (commit), "p" (parent of c) and "pp" (parent of p) respectively.

What is going on is:
 - we have calculated TREECHANGE for "c".
 - that, in turn, means that we have parsed "p" (it's done by 
   add_parents_to_list() - either as part of try_to_simplify_commit(), or 
   if that code doesn't trigger, by the later loop over the parents)
 - but we haven't parsed "pp" yet.

Now, when we decide to rewrite "c", we look at whether we can change the parent list of c to point from "p" to "pp". But with the added check, we now will trigger on the fact that "pp" hasn't even been parsed yet, so we won't even try, and we leave the parent list alone.

But I agree, I don't think it's really stable. We could have gotten to "pp" through some other chain.

I think the real problem is that "TREECHANGE" has the wrong polarity. It should default to always being set, and then we could actively clear it when we do the work to say "it's the same tree". Instead, we default it to being the same (which triggers parent rewriting), and only later may we notice that it wasn't the same.

			Linus