Re: Hmm.. Try harder to find the common commits in git protocol?
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 28, 2008, 23:20 UTC
- Message-ID
- <7vabjdbm8n.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <alpine.LFD.1.10.0804281310320.3119@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 22 quoted lines
> Hmm. Trying to debug this, I find the behavior hard to reproduce. But I
> *also* find that there seems to be something wrong in builtin-fetch-pack.
>
> Look at commit f3ec549481827b10609a43bf504517a0e8063a12 ("fetch-pack:
> check parse_commit/object results"), and tell me that the "parents"
> handling isn't totally broken. In particular, get_rev() does:
>
> struct commit_list *parents = NULL;
> ..
> commit = rev_list->item;
> if (!(commit->object.parsed))
> if (!parse_commit(commit))
> parents = commit->parents;
> ..
>
> which means that "parents" will never even get set if the commit was
> already parsed!
>
> And whether it got parsed or not depends on how we got there etc, so this
> may explain the occasionally odd behaviour I saw.
>
> Basically, I don't think that code can be right,...> This patch should fix that problem, but I wonder why it got rewritten that > way in the first place?
Sorry, my fault while reviewing the series. The intent of the update was too focused on not touching possibly bad data (i.e. when parse_commit() does not return zero to indicate success) and failed to make sure that it acts identically on good data --- very bad.