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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to