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.