From: "Junio C Hamano" <gitster@xxxxxxxxx>
Sent: Saturday, September 12, 2015 9:29 AM
Jeff King <peff@xxxxxxxx> writes:
Yeah, I think what is happening in this first hunk:
...
is doing the right thing. It did feel a little weird to me to be
munging
the global commit objects themselves, but I guess it is fairly normal
for git code.
Yeah, the above may be brilliant, but ...
diff --git a/revision.c b/revision.c
index af2a18e..a020a42 100644
--- a/revision.c
+++ b/revision.c
@@ -2765,7 +2765,9 @@ static void set_children(struct rev_info
*revs)
struct commit *commit = l->item;
struct commit_list *p;
- 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.
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.
I don't understand why this would be so. If we just have a base commit
from which to seek a --reversed chain then it would be true.
But if I understood the man page correctly: --reverse "This requires a
range of revision like START..END where the path to blame exists in
START." which when combined with --first-parent, would define a single
string of pearls which could then be reversed. Or is it that the code
doesn't do it in that order?
The add_child() call above is exactly that. It is saying "record
the fact that commit is child of p->item for these p on that parents
list". We may limit the call to record that commit C is a child of
its first parent P (and nobody else), but that does not make us
record that that P has only one child (which is C). This loop will
be entered with another commit C' whose first parent is the same P
and when we return from the set_children() function, we will find
that P has more than one children (C and C', at least); the only
thing we ensured is that all of these recorded children have P as
their first parent. It does not tell us which one of C's we would
want to pick when digging in reverse from P.
--
Philip
--
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