Re: Misrendering of git 2.24 log --graph

From: Derrick Stolee <stolee@gmail.com>
Date: Jan 24, 2020, 14:05 UTC
Message-ID: <b64e8fb7-1dd3-439f-6ab1-1b88c52879d1@gmail.com>
In-Reply-To: <nycvar.YFH.7.76.2001232353220.26179@n3.vanv.qr>

On 1/23/2020 6:12 PM, Jan Engelhardt wrote:

> Greetings.

Hello, Jan. Thanks for sending this report.

Show 5 quoted lines

> I have observed git 2.24 outputting a garbage graph element for a 
> particular history. The issue does not appear in 2.25, but the 
> underlying bug may still be in there; it is just that _this particular 
> history_ does not expose it anymore due to the new, more compact tree 
> rendering that 2.25 seems to be shipping.

I initially thought you were right, as 2.25 did include some new rendering, and we've already found and fixed a regression [1] and a style issue [2].

[1] https://lore.kernel.org/git/pull.517.git.1578408947.gitgitgadget@gmail.com/ [2] https://lore.kernel.org/git/pull.518.git.1578457675.gitgitgadget@gmail.com/

> Reproducer:
> 
> $ git clone git://github.com/jengelh/git-issue-20200123 gi

I needed to use "--mirror" in my clone to create local copies of all the remote refs.

Show 5 quoted lines

> $ cd gi; git log --oneline --graph --all --topo-order | head -n 74
> [...]
> | | * |   ba85ad93c Merge branch 'kc-8.7.x'
> | | |\ \  
> | |/ / /

Interesting, since the compact graph output in 2.25 intentionally wanted this merge to have the first edge go immediately to the left (see my output below).

Show 13 quoted lines

> | | | _   
> | | * 79106b731 doc: update 8.7.x news
> 
> See that underscore there in line 73. The connection between commit 
> 610d621dd and its parent 79106b731 is not properly connected (visually). 
> I think this should have been [diff notation follows]
> 
> -| | | _   
> +| | | /
> +| | |/
>  | | * 79106b731 doc: update 8.7.x news
> 
> or something along those lines (pun intended).

You are correct about the expected output here.

Here is something I noticed: this does not reproduce without a commit-graph! That's likely because it changes the order of the initial refs. Keep that in mind for anyone trying to repro this.

However, I see this output with 2.24.1:

| | | | * d95d49694 freebusy: add missing "else" in HrGetHumanReadableString
| | | |/  
| | |/|   
| | * |   ba85ad93c Merge branch 'kc-8.7.x'
| | |\ \  
| |/ / /  
| | | _   
| | * 79106b731 doc: update 8.7.x news

and this output in 2.25.0:

| | | | * d95d49694 freebusy: add missing "else" in HrGetHumanReadableString
| | | |/  
| | |/|   
| | * | ba85ad93c Merge branch 'kc-8.7.x'
| |/| | 
| | |/  
| | * 79106b731 doc: update 8.7.x news

And comparing v2.25.0 versus ds/graph-horizontal-edges has no difference in output.

Is it possible that you are not running the version you think you are?

Thanks, -Stolee