On Sat, Sep 12, 2015 at 01:29:43AM -0700, Junio C Hamano wrote: > >> - for (p = commit->parents; p; p = p->next) > >> + for (p = commit->parents; > >> + p && !revs->first_parent_only; > >> + p = p->next) > >> add_child(revs, p->item, commit); > >> } > >> } > > ... this is a total crap and shows that I am doubly an idiot. > > The loop is a no-op when first-parent-only (the intent is to call > add-child for just the first parent), so the code is stupid and > wrong in the first place, but more importantly, the logic is utterly > confused. Whoops, yeah. I think you need "if (revs->first_parent_only) break;". > The thing is, traversing first-parent chain in reverse fundamentally > is undefined. You can fork multiple topics at the tip of 'master' > and each of the topics may be single strand of pearls, but which one > of the topics is the first-child-chain---there is no answer to that > question. In general I think I agree, but in the case of blame, we know that we are starting from a single tip (and we know because we are using first-parent that we remain in a single strand of pearls, because we follow only one parent and there are no cycles). That is assuming that we create the set of children by traversing the first-parent history, though, which I am not at all sure about. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html