git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [Feature Request] git blame showing only revisions from git rev-list --first-parent

From
Philip Oakley <philipoakley@iee.org>
Date
Sep 12, 2015, 22:09 UTC
Message-ID
<6EB617480AFE4F7898553BCE6A91D946@PhilipOakley>
In-Reply-To
<xmqqy4gcxcp4.fsf@gitster.mtv.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
Sent: Saturday, September 12, 2015 9:29 AM
Show 40 quoted lines
> Jeff King <peff@peff.net> 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?

Show 12 quoted lines
>
> 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

Previous: Junio C HamanoNext: Jeff King
Message 8 of 16 in “[Feature Request] git blame showing only revisions from git rev-list --first-parent”
  1. Stephen ConnollySep 11, 2015
  2. Jeff KingSep 11, 2015
  3. Stephen ConnollySep 11, 2015
  4. Junio C HamanoSep 11, 2015
  5. Junio C HamanoSep 11, 2015
  6. Jeff KingSep 12, 2015
  7. Junio C HamanoSep 12, 2015
  8. Philip OakleySep 12, 2015
  9. Jeff KingSep 13, 2015
  10. Junio C HamanoSep 14, 2015
  11. Jeff KingSep 15, 2015
  12. Junio C HamanoSep 16, 2015
  13. Jeff KingSep 16, 2015
  14. Junio C HamanoSep 17, 2015
  15. Max KirillovOct 18, 2015
  16. Junio C HamanoOct 18, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.