undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

Browse Source 
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.
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.

It is actually the _second_ collapsing line that hits this assert.
The reason we are in this code path is because we are collapsing
the first line, and in that case we are hitting our target now
that the horizontal line is complete. However, the second line
cannot be a horizontal line, so it will collapse without horizontal
lines. In this case, it is inappropriate to assert that we have
reached our target, as we need to continue for another column
before reaching the target. Dropping the assert is safe here.

The new behavior in 0f0f389f12 (graph: tidy up display of
left-skewed merges, 2019-10-15) caused the behavior change that
made this assertion failure possible. In addition to making the
assert possible, it also changed how multiple edges collapse.

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.

Helped-by: Jeff King <peff@peff.net>
Reported-by: Bradley Smith <brad@brad-smith.co.uk>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Derrick Stolee 2020-01-07 21:27:01 +00:00 committed by Junio C Hamano
parent 042ed3e048
commit 0d251c3291
2 changed files with 42 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
graph.c
View File

@ -1219,13 +1219,9 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
*
* The space just to the left of this
* branch should always be empty.
*
* The branch to the left of that space
* should be our eventual target.
*/
assert(graph->mapping[i - 1] > target);
assert(graph->mapping[i - 2] < 0);
assert(graph->mapping[i - 3] == target);
graph->mapping[i - 2] = target;
/*
* Mark this branch as the horizontal edge to

42
t/t4215-log-skewed-merges.sh
View File

@ -240,4 +240,46 @@ test_expect_success 'log --graph with octopus merge with column joining its penu
EOF
'
test_expect_success 'log --graph with multiple tips' '
git checkout --orphan 6_1 &&
test_commit 6_A &&
git branch 6_2 &&
git branch 6_4 &&
test_commit 6_B &&
git branch 6_3 &&
test_commit 6_C &&
git checkout 6_2 && test_commit 6_D &&
git checkout 6_3 && test_commit 6_E &&
git checkout -b 6_5 6_1 &&
git merge --no-ff 6_2 -m 6_F &&
git checkout 6_4 && test_commit 6_G &&
git checkout 6_3 &&
git merge --no-ff 6_4 -m 6_H &&
git checkout 6_1 &&
git merge --no-ff 6_2 -m 6_I &&
check_graph 6_1 6_3 6_5 <<-\EOF
* 6_I
|\
| | * 6_H
| | |\
| | | * 6_G
| | * | 6_E
| | | | * 6_F
| |_|_|/|
|/| | |/
| | |/|
| |/| |
| * | | 6_D
| | |/
| |/|
* | | 6_C
| |/
|/|
* | 6_B
|/
* 6_A
EOF
'
test_done