{"thread":{"id":"52687","subject":"Misrendering of git 2.24 log --graph","startedAt":"2020-01-23T23:18:17Z","lastAt":"2020-01-24T21:26:11Z","messageCount":7,"participants":["Jan Engelhardt","Derrick Stolee","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"390317","messageId":"nycvar.YFH.7.76.2001232353220.26179@n3.vanv.qr","threadId":"52687","inReplyTo":null,"subject":"Misrendering of git 2.24 log --graph","fromName":"Jan Engelhardt","fromEmail":"jengelh@inai.de","sentAt":"2020-01-23T23:12:54Z","receivedAt":"2020-01-23T23:18:17Z","isPatch":false,"sender":{"key":"jengelh@inai.de","avatar":"https://avatars.githubusercontent.com/u/8861948?v=4"},"body":"Greetings.\n\nI have observed git 2.24 outputting a garbage graph element for a \nparticular history. The issue does not appear in 2.25, but the \nunderlying bug may still be in there; it is just that _this particular \nhistory_ does not expose it anymore due to the new, more compact tree \nrendering that 2.25 seems to be shipping.\n\nReproducer:\n\n$ git clone git://github.com/jengelh/git-issue-20200123 gi\n$ cd gi; git log --oneline --graph --all --topo-order | head -n 74\n[...]\n| | * |   ba85ad93c Merge branch 'kc-8.7.x'\n| | |\\ \\  \n| |/ / /  \n| | | _   \n| | * 79106b731 doc: update 8.7.x news\n\nSee that underscore there in line 73. The connection between commit \n610d621dd and its parent 79106b731 is not properly connected (visually). \nI think this should have been [diff notation follows]\n\n-| | | _   \n+| | | /\n+| | |/\n | | * 79106b731 doc: update 8.7.x news\n\nor something along those lines (pun intended).\n"},{"id":"390359","messageId":"b64e8fb7-1dd3-439f-6ab1-1b88c52879d1@gmail.com","threadId":"52687","inReplyTo":"nycvar.YFH.7.76.2001232353220.26179@n3.vanv.qr","subject":"Re: Misrendering of git 2.24 log --graph","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-24T14:05:48Z","receivedAt":"2020-01-24T14:05:52Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/23/2020 6:12 PM, Jan Engelhardt wrote:\n> Greetings.\n\nHello, Jan. Thanks for sending this report.\n\n> I have observed git 2.24 outputting a garbage graph element for a \n> particular history. The issue does not appear in 2.25, but the \n> underlying bug may still be in there; it is just that _this particular \n> history_ does not expose it anymore due to the new, more compact tree \n> rendering that 2.25 seems to be shipping.\n\nI initially thought you were right, as 2.25 did include some new\nrendering, and we've already found and fixed a regression [1] and\na style issue [2].\n\n[1] https://lore.kernel.org/git/pull.517.git.1578408947.gitgitgadget@gmail.com/\n[2] https://lore.kernel.org/git/pull.518.git.1578457675.gitgitgadget@gmail.com/\n\n> Reproducer:\n> \n> $ git clone git://github.com/jengelh/git-issue-20200123 gi\n\nI needed to use \"--mirror\" in my clone to create local copies\nof all the remote refs.\n\n> $ cd gi; git log --oneline --graph --all --topo-order | head -n 74\n> [...]\n> | | * |   ba85ad93c Merge branch 'kc-8.7.x'\n> | | |\\ \\  \n> | |/ / /  \n\nInteresting, since the compact graph output in 2.25 intentionally\nwanted this merge to have the first edge go immediately to the left\n(see my output below).\n\n> | | | _   \n> | | * 79106b731 doc: update 8.7.x news\n> \n> See that underscore there in line 73. The connection between commit \n> 610d621dd and its parent 79106b731 is not properly connected (visually). \n> I think this should have been [diff notation follows]\n> \n> -| | | _   \n> +| | | /\n> +| | |/\n>  | | * 79106b731 doc: update 8.7.x news\n> \n> or something along those lines (pun intended).\n\nYou are correct about the expected output here.\n\nHere is something I noticed: this does not reproduce without a\ncommit-graph! That's likely because it changes the order of the\ninitial refs. Keep that in mind for anyone trying to repro this.\n\nHowever, I see this output with 2.24.1:\n\n| | | | * d95d49694 freebusy: add missing \"else\" in HrGetHumanReadableString\n| | | |/  \n| | |/|   \n| | * |   ba85ad93c Merge branch 'kc-8.7.x'\n| | |\\ \\  \n| |/ / /  \n| | | _   \n| | * 79106b731 doc: update 8.7.x news\n\nand this output in 2.25.0:\n\n| | | | * d95d49694 freebusy: add missing \"else\" in HrGetHumanReadableString\n| | | |/  \n| | |/|   \n| | * | ba85ad93c Merge branch 'kc-8.7.x'\n| |/| | \n| | |/  \n| | * 79106b731 doc: update 8.7.x news\n\nAnd comparing v2.25.0 versus ds/graph-horizontal-edges has\nno difference in output.\n\nIs it possible that you are not running the version you think you are?\n\nThanks,\n-Stolee\n\n"},{"id":"390360","messageId":"75f5c537-bb46-7f04-54fe-0bc62762a682@gmail.com","threadId":"52687","inReplyTo":"b64e8fb7-1dd3-439f-6ab1-1b88c52879d1@gmail.com","subject":"Re: Misrendering of git 2.24 log --graph","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-24T14:06:56Z","receivedAt":"2020-01-24T14:07:00Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/24/2020 9:05 AM, Derrick Stolee wrote:\n> On 1/23/2020 6:12 PM, Jan Engelhardt wrote:\n>> Greetings.\n> \n> Hello, Jan. Thanks for sending this report.\n> \n>> I have observed git 2.24 outputting a garbage graph element for a \n>> particular history. The issue does not appear in 2.25, but the \n>> underlying bug may still be in there; it is just that _this particular \n>> history_ does not expose it anymore due to the new, more compact tree \n>> rendering that 2.25 seems to be shipping.\n\nI completely misread your ordering here. You are clear that this\nis an issue in 2.24 and NOT 2.25. Sorry.\n\nI'm not sure that there is anything to do since the graph rendering\nhas changed so much, and we intend to keep the new version instead.\n\nThanks,\n-Stolee\n\n"},{"id":"390366","messageId":"nycvar.YFH.7.76.2001241652250.32144@n3.vanv.qr","threadId":"52687","inReplyTo":"b64e8fb7-1dd3-439f-6ab1-1b88c52879d1@gmail.com","subject":"Re: Misrendering of git 2.24 log --graph","fromName":"Jan Engelhardt","fromEmail":"jengelh@inai.de","sentAt":"2020-01-24T16:08:33Z","receivedAt":"2020-01-24T16:08:36Z","isPatch":false,"sender":{"key":"jengelh@inai.de","avatar":"https://avatars.githubusercontent.com/u/8861948?v=4"},"body":"\nOn Friday 2020-01-24 15:05, Derrick Stolee wrote:\n>> I have observed git 2.24 outputting a garbage graph element for a \n>> particular history.\n>\n>I initially thought you were right, as 2.25 did include some new\n>rendering, and we've already found and fixed a regression [1] and\n>a style issue [2].\n>[...]\n>Here is something I noticed: this does not reproduce without a\n>commit-graph! That's likely because it changes the order of the\n>initial refs. Keep that in mind for anyone trying to repro this.\n\nCuriously, I need no --mirror for `git clone`, the default clone\noperation is just fine. It has, as far as I can remember back,\nalways downloaded all origin:refs/heads/* and refs/tags/*.\n\n>However, I see this output with 2.24.1:\n\n>| | | _   \n>| | * 79106b731 doc: update 8.7.x news\n>\n>and this output in 2.25.0 [...]\n>\n>Is it possible that you are not running the version you think you are?\n\nI rerun my checks as I write this mail. The observations:\n\n * Built v2.23.0 from source, ran it via `../tmpdir/git/git log --andsoon`,\n   shows said underscore.\n * Built v2.24.0 from source, shows underscore\n * Built v2.24.1 from source, shows underscore\n * Used v2.24.1 from distro (/usr/bin/git), renders underscore\n * Built ead10bacc466a409490fb743d7805893976f8f53 (most recent \n   branch join not having v2.25 as an ancestor) from src, \n   shows underscore\n * Built v2.25.0 from source, new rendering\n * Built 631a7dec11, same as v2.25.0\n * Built 631a7dec11^, same as v2.25.0\n"},{"id":"390367","messageId":"nycvar.YFH.7.76.2001241730030.32144@n3.vanv.qr","threadId":"52687","inReplyTo":"75f5c537-bb46-7f04-54fe-0bc62762a682@gmail.com","subject":"Re: Misrendering of git 2.25 log --graph","fromName":"Jan Engelhardt","fromEmail":"jengelh@inai.de","sentAt":"2020-01-24T16:34:47Z","receivedAt":"2020-01-24T16:34:50Z","isPatch":false,"sender":{"key":"jengelh@inai.de","avatar":"https://avatars.githubusercontent.com/u/8861948?v=4"},"body":"On Friday 2020-01-24 15:06, Derrick Stolee wrote:\n>\n>I completely misread your ordering here. You are clear that this\n>is an issue in 2.24 and NOT 2.25. Sorry.\n>\n>I'm not sure that there is anything to do since the graph rendering\n>has changed so much, and we intend to keep the new version instead.\n\nI now have a minimized reproducer, for *2.25*:\n\n» ../git/git log --oneline --graph --all --decorate --topo-order\n*   e1ee7b8 (HEAD -> master) Merge branch 'k9' into k10\n|\\  \n| *   7b48214 Merge branch 'k8' into k9\n| |\\  \n| | *   ed02a51 Merge branch 'blah' into k8\n| | |\\  \n| | | * 44279cf blah\n| | * |   4053c4d Merge branch 'k7' into k8\n| | |\\ \\  \n| |/ / /  \n| | | _   \n| | * 5b449d6 update 8.7 news\n| |/  \n| * 43a324f foo\n|/  \n* 5932a51 root\n\n» ../git/git --version\ngit version 2.25.0.24.gbc7a3d4dc0\n\nThe repository https://github.com/jengelh/git-issue-20200123 has been\nre-populated with the new contents.\n"},{"id":"390377","messageId":"3e9089c4-8ba6-b62c-cda3-607152790566@gmail.com","threadId":"52687","inReplyTo":"nycvar.YFH.7.76.2001241730030.32144@n3.vanv.qr","subject":"Re: Misrendering of git 2.25 log --graph","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-24T18:45:34Z","receivedAt":"2020-01-24T18:45:37Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/24/2020 11:34 AM, Jan Engelhardt wrote:\n> On Friday 2020-01-24 15:06, Derrick Stolee wrote:\n>>\n>> I completely misread your ordering here. You are clear that this\n>> is an issue in 2.24 and NOT 2.25. Sorry.\n>>\n>> I'm not sure that there is anything to do since the graph rendering\n>> has changed so much, and we intend to keep the new version instead.\n> \n> I now have a minimized reproducer, for *2.25*:\n> \n> » ../git/git log --oneline --graph --all --decorate --topo-order\n> *   e1ee7b8 (HEAD -> master) Merge branch 'k9' into k10\n> |\\  \n> | *   7b48214 Merge branch 'k8' into k9\n> | |\\  \n> | | *   ed02a51 Merge branch 'blah' into k8\n> | | |\\  \n> | | | * 44279cf blah\n> | | * |   4053c4d Merge branch 'k7' into k8\n> | | |\\ \\  \n> | |/ / /  \n> | | | _   \n> | | * 5b449d6 update 8.7 news\n> | |/  \n> | * 43a324f foo\n> |/  \n> * 5932a51 root\n\nHm. I get the above output for v2.24.1. Since you are using a\nspecific path for your execution, then perhaps that version\ndisagrees with the one on your PATH that reported \"git version\"?\n\nFor v2.25.0 I get this:\n\n*   e1ee7b8 (HEAD -> master, origin/master, origin/HEAD) Merge branch 'k9' into k10\n|\\  \n| *   7b48214 Merge branch 'k8' into k9\n| |\\  \n| | *   ed02a51 Merge branch 'blah' into k8\n| | |\\  \n| | | * 44279cf blah\n| | * | 4053c4d Merge branch 'k7' into k8\n| |/| | \n| | |/  \n| | * 5b449d6 update 8.7 news\n| |/  \n| * 43a324f foo\n|/  \n* 5932a51 root\n\nAgain, the key is at 4053c4d where the first edge out\nof the merge commit moves to the left.\n\nThanks,\n-Stolee\n"},{"id":"390413","messageId":"20200124212603.GH6837@szeder.dev","threadId":"52687","inReplyTo":"3e9089c4-8ba6-b62c-cda3-607152790566@gmail.com","subject":"Re: Misrendering of git 2.25 log --graph","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-01-24T21:26:03Z","receivedAt":"2020-01-24T21:26:11Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Jan 24, 2020 at 01:45:34PM -0500, Derrick Stolee wrote:\n> On 1/24/2020 11:34 AM, Jan Engelhardt wrote:\n> > On Friday 2020-01-24 15:06, Derrick Stolee wrote:\n> >>\n> >> I completely misread your ordering here. You are clear that this\n> >> is an issue in 2.24 and NOT 2.25. Sorry.\n> >>\n> >> I'm not sure that there is anything to do since the graph rendering\n> >> has changed so much, and we intend to keep the new version instead.\n> > \n> > I now have a minimized reproducer, for *2.25*:\n> > \n> > » ../git/git log --oneline --graph --all --decorate --topo-order\n> > *   e1ee7b8 (HEAD -> master) Merge branch 'k9' into k10\n> > |\\  \n> > | *   7b48214 Merge branch 'k8' into k9\n> > | |\\  \n> > | | *   ed02a51 Merge branch 'blah' into k8\n> > | | |\\  \n> > | | | * 44279cf blah\n> > | | * |   4053c4d Merge branch 'k7' into k8\n> > | | |\\ \\  \n> > | |/ / /  \n> > | | | _   \n> > | | * 5b449d6 update 8.7 news\n> > | |/  \n> > | * 43a324f foo\n> > |/  \n> > * 5932a51 root\n> \n> Hm. I get the above output for v2.24.1. Since you are using a\n> specific path for your execution, then perhaps that version\n> disagrees with the one on your PATH that reported \"git version\"?\n> \n> For v2.25.0 I get this:\n> \n> *   e1ee7b8 (HEAD -> master, origin/master, origin/HEAD) Merge branch 'k9' into k10\n> |\\  \n> | *   7b48214 Merge branch 'k8' into k9\n> | |\\  \n> | | *   ed02a51 Merge branch 'blah' into k8\n> | | |\\  \n> | | | * 44279cf blah\n> | | * | 4053c4d Merge branch 'k7' into k8\n> | |/| | \n> | | |/  \n> | | * 5b449d6 update 8.7 news\n> | |/  \n> | * 43a324f foo\n> |/  \n> * 5932a51 root\n\nI get this good-looking graph with v2.25.0, too.\n\nA few notes to add:\n\n  - We don't need the topmost commit e1ee7b8 to reproduce that strange\n    underscore below the kink with v2.24.0:\n\n      $ git --no-pager log --oneline --graph --topo-order 7b48214\n      *   7b48214 Merge branch 'k8' into k9\n      |\\  \n      | *   ed02a51 Merge branch 'blah' into k8\n      | |\\  \n      | | * 44279cf blah\n      | * |   4053c4d Merge branch 'k7' into k8\n      | |\\ \\  \n      |/ / /  \n      | | _   \n      | * 5b449d6 update 8.7 news\n      |/  \n      * 43a324f foo\n      * 5932a51 root\n\n  - Git behaved like that up until about the middle of the patch\n    series simplifying the graph output, namely until commit\n    458152cce1 (graph: example of graph output that can be simplified,\n    2019-10-15).  The next commit 0f0f389f12 (graph: tidy up display\n    of left-skewed merges, 2019-10-15) changed the bahavior for the\n    worse:\n\n      *   7b48214 Merge branch 'k8' into k9\n      |\\  \n      | *   ed02a51 Merge branch 'blah' into k8\n      | |\\  \n      | | * 44279cf blah\n      | * | 4053c4d Merge branch 'k7' into k8\n      |/|\\  \n      | |/  \n      | * 5b449d6 update 8.7 news\n      |/  \n      * 43a324f foo\n      * 5932a51 root\n\n    Notice how 4053c4d looks like an octopus merge, and 44279cf\n    seemingly comes out of nowhere.\n\n    And then the next commit d62893ecc1 (graph: commit and post-merge\n    lines for left-skewed merges, 2019-10-15) seems to have fixed this\n    issue, and we see the same good-looking graph that we get with\n    v2.25.0 as well:\n\n      *   7b48214 Merge branch 'k8' into k9\n      |\\  \n      | *   ed02a51 Merge branch 'blah' into k8\n      | |\\  \n      | | * 44279cf blah\n      | * | 4053c4d Merge branch 'k7' into k8\n      |/| | \n      | |/  \n      | * 5b449d6 update 8.7 news\n      |/  \n      * 43a324f foo\n      * 5932a51 root\n\n  - Interestingly, the issue of an underscore below the kink is\n    mentioned in the later commit 92beecc136 (graph: flatten edges\n    that fuse with their right neighbor, 2019-10-15), quoting the last\n    part of its log message:\n\n      One of the test cases here cannot be correctly rendered in Git v2.23.0;\n      it produces this output following commit E:\n      \n              | | *-. \\   5_E\n              | | |\\ \\ \\\n              | |/ / / /\n              | | | / _\n              | |_|/\n              |/| |\n      \n      The new implementation makes sure that the rightmost edge in this\n      history is not left dangling as above.\n\n    This is part of the new test case 'log --graph with octopus merge\n    with column joining its penultimate parent' in t4215 that was\n    added in 92beecc136.  Usually when a commit both changes the code\n    and add new tests, then that means that that new test would have\n    failed on previous versions.  However, that is not the case here:\n    I tried this new test case on earlier commits in that series, and\n    it already succeed with commit d62893ecc1, i.e. the same commit\n    that fixes Jan's cases. \n\n"}]}