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

Re: [PATCH] fixup! graph: output padding for merge subsequent parents

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 10, 2013, 22:38 UTC
Message-ID
<7vwqufrdzd.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20130210210229.GB2270@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
Show 20 quoted lines
> On Sun, Feb 10, 2013 at 11:30:39AM -0800, Junio C Hamano wrote:
> ...
>> Is it correct to say that this essentially re-does 656197ad3805
>> (graph.c: infinite loop in git whatchanged --graph -m, 2009-07-25)
>> in a slightly different way, in that Michał's original fix also
>> protected against the case where graph->state is flipped to
>> GRAPH_PADDING by graph_next_line() that returns false, but with your
>> fixup, the code knows it never happens (i.e. when graph_next_line()
>> returns false, graph->state is always in the GRAPH_PADDING state),
>> and the only thing we need to be careful about is when graph->state
>> is already in the PADDING state upon entry to this function?
>
> Yes, although I wonder if we can end up in POST_MERGE or COLLAPSING
> state here as well.  The check in the loop guards against that because
> those will eventually end up as PADDING.
>
> As far as I can see, this is okay because we have called
> graph_show_remainder() at the end of outputting a commit, even when we
> end up outputting the same (merge) commit more than once.  But someone
> more familiar with the graph code might want to comment here.

More importantly, that kind of thought process needs to be documented in the log message; that will help people to diagnose the cause of the problem if they later find that this patch made an incorrect assumption while simplifying the code.

Previous: John KeepingNext: John Keeping
Message 5 of 11 in “What's cooking in git.git (Feb 2013, #04; Sat, 9)”
  1. Junio C HamanoFeb 9, 2013
  2. fixup! graph: output padding for merge subsequent parentsJohn Keeping, Feb 10, 2013
  3. Junio C HamanoFeb 10, 2013
  4. John KeepingFeb 10, 2013
  5. Junio C HamanoFeb 10, 2013
  6. John KeepingFeb 11, 2013
  7. Junio C HamanoFeb 11, 2013
  8. John KeepingFeb 11, 2013
  9. Junio C HamanoFeb 11, 2013
  10. Matthieu MoyFeb 11, 2013
  11. Junio C HamanoFeb 11, 2013

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.