Thread (1 message) 1 message, 1 author, 2020-01-07

Re: [PATCH v2 1/2] graph: fix case that hit assert()

From: Junio C Hamano <hidden>
Date: 2020-01-07 21:50:16

"Derrick Stolee via GitGitGadget" [off-list ref] writes:
From: Derrick Stolee <redacted>
Subject: Re: [PATCH v2 1/2] graph: fix case that hit assert()

A failure was reported in "git log --graph --all" with the new
graph-rendering logic. The code fails an assert() statement when
collapsing multiple edges from a merge.

The assert was introduced by eaf158f8 (graph API: Use horizontal
lines for more compact graphs, 2009-04-21), which is quite old.
This assert is trying to say that when we complete a horizontal
line with a single slash, it is because we have reached our target.

This assertion is hit when we have two collapsing lines from the
same merge commit, as follows:

    | | | | *
    | |_|_|/|
    |/| | |/
    | | |/|
    | |/| |
    | * | |
    * | | |
I was sort-of expecting to see

    graph: drop assert() for merge with two collapsing parents

    When "git log --graph" shows a merge commit that has two collapsing
    lines, like:

        | | | | *
        | |_|_|/|
        |/| | |/
        | | |/|
        | |/| |
        | * | |
        * | | |

    we trigger an assert():

            graph.c:1228: graph_output_collapsing_line: Assertion
                          `graph->mapping[i - 3] == target' failed.

    The assert was introduced by eaf158f8 ("graph API: Use horizontal
    lines for more compact graphs", 2009-04-21), which is quite old.
    ...

near the beginning of this commit, though.
In a larger example, the current code will output a collapse
as follows:

	| | | | | | *
	| |_|_|_|_|/|\
	|/| | | | |/ /
	| | | | |/| /
	| | | |/| |/
	| | |/| |/|
	| |/| |/| |
	| | |/| | |
	| | * | | |

However, the intended collapse should allow multiple horizontal lines
as follows:

	| | | | | | *
	| |_|_|_|_|/|\
	|/| | | | |/ /
	| | |_|_|/| /
	| |/| | | |/
	| | | |_|/|
	| | |/| | |
	| | * | | |

This behavior is not corrected by this change, but is noted for a later
update.
This part is new and looks like a good thing to mention.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help