{"thread":{"id":"59145","subject":"[PATCH 0/2] gitk: handle long command-lines","startedAt":"2023-01-24T11:23:26Z","lastAt":"2023-05-09T01:38:17Z","messageCount":10,"participants":["Johannes Schindelin via GitGitGadget","Nico Rieck via GitGitGadget","Junio C Hamano","Johannes Schindelin","Felipe Contreras","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"470966","messageId":"pull.1469.git.1674559397.gitgitgadget@gmail.com","threadId":"59145","inReplyTo":null,"subject":"[PATCH 0/2] gitk: handle long command-lines","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-01-24T11:23:15Z","receivedAt":"2023-01-24T11:23:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"These patches have been cooking for such a long time in Git for Windows that\nyou might think they turned into broth. Yummy broth, to be sure. But broth.\n'Tis beyond time for the patches to make it upstream.\n\nJohannes Schindelin (1):\n gitk: prevent overly long command lines\n\nNico Rieck (1):\n gitk: escape file paths before piping to git log\n\n gitk | 36 +++++++++++++++++++++++++++++++-----\n 1 file changed, 31 insertions(+), 5 deletions(-)\n\n\nbase-commit: 465f03869ae11acd04abfa1b83c67879c867410c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1469%2Fdscho%2Fgitk-long-cmdline-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1469/dscho/gitk-long-cmdline-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1469\n-- \ngitgitgadget\n"},{"id":"470967","messageId":"89fe0380cd3a373e7e23d663b506466fd6cb5fb6.1674559397.git.gitgitgadget@gmail.com","threadId":"59145","inReplyTo":"pull.1469.git.1674559397.gitgitgadget@gmail.com","subject":"[PATCH 1/2] gitk: prevent overly long command lines","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-01-24T11:23:16Z","receivedAt":"2023-01-24T11:23:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nTo avoid running into command line limitations, some of Git's commands\nsupport the `--stdin` option.\n\nLet's use exactly this option in the three rev-list/log invocations in\ngitk that would otherwise possibly run the danger of trying to invoke a\ntoo-long command line.\n\nWhile it is easy to redirect either stdin or stdout in Tcl/Tk scripts,\nwhat we need here is both. We need to capture the output, yet we also\nneed to pipe in the revs/files arguments via stdin (because stdin does\nnot have any limit, unlike the command line). To help this, we use the\nneat Tcl feature where you can capture stdout and at the same time feed\na fixed string as stdin to the spawned process.\n\nOne non-obvious aspect about this change is that the `--stdin` option\nallows to specify revs, the double-dash, and files, but *no* other\noptions such as `--not`. This is addressed by prefixing the \"negative\"\nrevs with `^` explicitly rather than relying on the `--not` option\n(thanks for coming up with that idea, Max!).\n\nThis fixes https://github.com/git-for-windows/git/issues/1987\n\nAnalysis-and-initial-patch-by: Max Kirillov \nSigned-off-by: Johannes Schindelin \n---\n gitk | 24 +++++++++++++++++++-----\n 1 file changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 0ae7d685904..92375ca6a2a 100755\n--- a/gitk\n+++ b/gitk\n@@ -405,14 +405,16 @@ proc start_rev_list {view} {\n if {$revs eq {}} {\n return 0\n }\n- set args [concat $vflags($view) $revs]\n+ set args $vflags($view)\n } else {\n+ set revs {}\n set args $vorigargs($view)\n }\n \n if {[catch {\n set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \\\n- --parents --boundary $args \"--\" $files] r]\n+ --parents --boundary $args --stdin \\\n+ \"<<[join [concat $revs \"--\" $files] \"\\\\n\"]\"] r]\n } err]} {\n error_popup \"[mc \"Error executing git log:\"] $err\"\n return 0\n@@ -554,13 +556,19 @@ proc updatecommits {} {\n set revs $newrevs\n set vposids($view) [lsort -unique [concat $oldpos $vposids($view)]]\n }\n- set args [concat $vflags($view) $revs --not $oldpos]\n+ set args $vflags($view)\n+ foreach r $oldpos {\n+ lappend revs \"^$r\"\n+ }\n } else {\n+ set revs {}\n set args $vorigargs($view)\n }\n if {[catch {\n set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \\\n- --parents --boundary $args \"--\" $vfilelimit($view)] r]\n+ --parents --boundary $args --stdin \\\n+ \"<<[join [concat $revs \"--\" \\\n+ $vfilelimit($view)] \"\\\\n\"]\"] r]\n } err]} {\n error_popup \"[mc \"Error executing git log:\"] $err\"\n return\n@@ -10231,10 +10239,16 @@ proc getallcommits {} {\n foreach id $seeds {\n lappend ids \"^$id\"\n }\n+ lappend ids \"--\"\n }\n }\n if {$ids ne {}} {\n- set fd [open [concat $cmd $ids] r]\n+ if {$ids eq \"--all\"} {\n+ set cmd [concat $cmd \"--all\"]\n+ } else {\n+ set cmd [concat $cmd --stdin \"<<[join $ids \"\\\\n\"]\"]\n+ }\n+ set fd [open $cmd r]\n fconfigure $fd -blocking 0\n incr allcommits\n nowbusy allcommits\n-- \ngitgitgadget\n\n"},{"id":"470968","messageId":"ff22dd238e9ea6c540eb1083d3b56a7bd804580f.1674559397.git.gitgitgadget@gmail.com","threadId":"59145","inReplyTo":"pull.1469.git.1674559397.gitgitgadget@gmail.com","subject":"[PATCH 2/2] gitk: escape file paths before piping to git log","fromName":"Nico Rieck via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-01-24T11:23:17Z","receivedAt":"2023-01-24T11:23:29Z","isPatch":true,"sender":{"key":"name:Nico Rieck","avatar":null},"body":"From: Nico Rieck \n\nWe just started piping the file paths via `stdin` instead of passing\nthem via the command-line, to avoid running into command-line\nlimitations.\n\nHowever, since we now pipe the file paths, we need to take care of\nspecial characters.\n\nThis fixes https://github.com/git-for-windows/git/issues/2293\n\nSigned-off-by: Nico Rieck \nSigned-off-by: Johannes Schindelin \n---\n gitk | 16 ++++++++++++++--\n 1 file changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 92375ca6a2a..df3ba2ea99b 100755\n--- a/gitk\n+++ b/gitk\n@@ -353,6 +353,16 @@ proc parseviewrevs {view revs} {\n return $ret\n }\n \n+# Escapes a list of filter paths to be passed to git log via stdin. Note that\n+# paths must not be quoted.\n+proc escape_filter_paths {paths} {\n+\tset escaped [list]\n+\tforeach path $paths {\n+\t\tlappend escaped [string map {\\\\ \\\\\\\\ \"\\ \" \"\\\\\\ \"} $path]\n+\t}\n+\treturn $escaped\n+}\n+\n # Start off a git log process and arrange to read its output\n proc start_rev_list {view} {\n global startmsecs commitidx viewcomplete curview\n@@ -414,7 +424,8 @@ proc start_rev_list {view} {\n if {[catch {\n set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \\\n --parents --boundary $args --stdin \\\n- \"<<[join [concat $revs \"--\" $files] \"\\\\n\"]\"] r]\n+ \"<<[join [concat $revs \"--\" \\\n+ [escape_filter_paths $files]] \"\\\\n\"]\"] r]\n } err]} {\n error_popup \"[mc \"Error executing git log:\"] $err\"\n return 0\n@@ -568,7 +579,8 @@ proc updatecommits {} {\n set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \\\n --parents --boundary $args --stdin \\\n \"<<[join [concat $revs \"--\" \\\n- $vfilelimit($view)] \"\\\\n\"]\"] r]\n+ [escape_filter_paths \\\n+ $vfilelimit($view)]] \"\\\\n\"]\"] r]\n } err]} {\n error_popup \"[mc \"Error executing git log:\"] $err\"\n return\n-- \ngitgitgadget\n"},{"id":"470984","messageId":"xmqqv8kwhrja.fsf@gitster.g","threadId":"59145","inReplyTo":"89fe0380cd3a373e7e23d663b506466fd6cb5fb6.1674559397.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] gitk: prevent overly long command lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-24T15:38:01Z","receivedAt":"2023-01-24T15:38:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> To avoid running into command line limitations, some of Git's commands\n> support the `--stdin` option.\n>\n> Let's use exactly this option in the three rev-list/log invocations in\n> gitk that would otherwise possibly run the danger of trying to invoke a\n> too-long command line.\n\nMakes perfect sense. I do not know the point of saying exactly\nhere, though.\n\n> While it is easy to redirect either stdin or stdout in Tcl/Tk scripts,\n> what we need here is both. We need to capture the output, yet we also\n> need to pipe in the revs/files arguments via stdin (because stdin does\n> not have any limit, unlike the command line). To help this, we use the\n> neat Tcl feature where you can capture stdout and at the same time feed\n> a fixed string as stdin to the spawned process.\n\nNice, so this is not about \"we may have too many args to fit in\nour memory\", but about \"we may have too many args for system to\nspawn the subprocess with\".\n\n> One non-obvious aspect about this change is that the `--stdin` option\n> allows to specify revs, the double-dash, and files, but *no* other\n> options such as `--not`.\n\nIt sounds like a design mistake, which may want to be fixed, but of\ncourse gitk cannot depend on Git it runs with having such a fix, and\nuse of \"^\" prefix is a good alternative (after all, \"--not\" was\ninvented to save us writing ^ in front of many revs).\n\nGood.\n\n"},{"id":"474224","messageId":"af2111f8-2669-b952-6c4e-45ee0330ec14@gmx.de","threadId":"59145","inReplyTo":"pull.1469.git.1674559397.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] gitk: handle long command-lines","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-03-27T09:17:32Z","receivedAt":"2023-03-27T09:17:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Pau & Junio,\n\nthis patch series saw a positive review from Junio (thank you! I know that\nyou try to stay away from Tcl code, so I appreciate the effort very much),\nbut apart from that it simply languished on the mailing list for more than\ntwo months now.\n\nPaul, is there anything I can do to help you integrate this into `gitk`?\nOr is it time to pass over `gitk` maintenance to the Git project?\n\nCiao,\nJohannes\n\nOn Tue, 24 Jan 2023, Johannes Schindelin via GitGitGadget wrote:\n\n> These patches have been cooking for such a long time in Git for Windows that\n> you might think they turned into broth. Yummy broth, to be sure. But broth.\n> 'Tis beyond time for the patches to make it upstream.\n>\n> Johannes Schindelin (1):\n> gitk: prevent overly long command lines\n>\n> Nico Rieck (1):\n> gitk: escape file paths before piping to git log\n>\n> gitk | 36 +++++++++++++++++++++++++++++++-----\n> 1 file changed, 31 insertions(+), 5 deletions(-)\n>\n>\n> base-commit: 465f03869ae11acd04abfa1b83c67879c867410c\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1469%2Fdscho%2Fgitk-long-cmdline-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1469/dscho/gitk-long-cmdline-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1469\n> --\n> gitgitgadget\n>\n"},{"id":"474229","messageId":"CAMP44s0jG1CQMygMASjEe8Uzu7rOrNuxUbjS05FAA=zBwMV72Q@mail.gmail.com","threadId":"59145","inReplyTo":"af2111f8-2669-b952-6c4e-45ee0330ec14@gmx.de","subject":"Re: [PATCH 0/2] gitk: handle long command-lines","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-03-27T10:17:20Z","receivedAt":"2023-03-27T10:17:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Mar 27, 2023 at 3:41 AM Johannes Schindelin\n wrote:\n\n> this patch series saw a positive review from Junio (thank you! I know that\n> you try to stay away from Tcl code, so I appreciate the effort very much),\n> but apart from that it simply languished on the mailing list for more than\n> two months now.\n\nTwo months? My patch which contains an obvious fix [1] has been\nwaiting almost two years.\n\n> Paul, is there anything I can do to help you integrate this into `gitk`?\n> Or is it time to pass over `gitk` maintenance to the Git project?\n\nOr just remove it from the Git codebase and maintain it elsewhere.\n\n[1] https://lore.kernel.org/git/20210505211846.1842824-1-felipe.contreras@gmail.com/\n\n-- \nFelipe Contreras\n"},{"id":"476680","messageId":"fce603aa-ceb7-d601-6a19-657e36d9059c@iee.email","threadId":"59145","inReplyTo":"af2111f8-2669-b952-6c4e-45ee0330ec14@gmx.de","subject":"Re: [PATCH 0/2] gitk: handle long command-lines","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2023-05-06T16:00:23Z","receivedAt":"2023-05-06T16:00:35Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 27/03/2023 10:17, Johannes Schindelin wrote:\n> Hi Pau & Junio,\n>\n> this patch series saw a positive review from Junio (thank you! I know that\n> you try to stay away from Tcl code, so I appreciate the effort very much),\n> but apart from that it simply languished on the mailing list for more than\n> two months now.\n>\n> Paul, is there anything I can do to help you integrate this into `gitk`?\n> Or is it time to pass over `gitk` maintenance to the Git project?\n>\n> Ciao,\n> Johannes\n\nI just tripped over this problem while trying to de-stack my Git backlog\nhttps://github.com/git-for-windows/git/issues/4408 \"sdk gitk interaction\".\n\n>\n> On Tue, 24 Jan 2023, Johannes Schindelin via GitGitGadget wrote:\n>\n>> These patches have been cooking for such a long time in Git for Windows that\n>> you might think they turned into broth. Yummy broth, to be sure. But broth.\n>> 'Tis beyond time for the patches to make it upstream.\n>>\n>> Johannes Schindelin (1):\n>> gitk: prevent overly long command lines\n>>\n>> Nico Rieck (1):\n>> gitk: escape file paths before piping to git log\n>>\n>> gitk | 36 +++++++++++++++++++++++++++++++-----\n>> 1 file changed, 31 insertions(+), 5 deletions(-)\n>>\n>>\n>> base-commit: 465f03869ae11acd04abfa1b83c67879c867410c\n>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1469%2Fdscho%2Fgitk-long-cmdline-v1\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1469/dscho/gitk-long-cmdline-v1\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/1469\n>> --\n>> gitgitgadget\n>>\n\nIf there was a way to un-stick this [1] it would be great.\n\nPhilip\n[1]\nhttps://lore.kernel.org/git/pull.1469.git.1674559397.gitgitgadget@gmail.com/\n"},{"id":"476729","messageId":"64581989b26de_4e6129456@chronos.notmuch","threadId":"59145","inReplyTo":"fce603aa-ceb7-d601-6a19-657e36d9059c@iee.email","subject":"New canonical gitk url","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-07T21:35:05Z","receivedAt":"2023-05-07T21:35:22Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hello,\n\nPhilip Oakley wrote:\n> On 27/03/2023 10:17, Johannes Schindelin wrote:\n> > Hi Pau & Junio,\n> >\n> > this patch series saw a positive review from Junio (thank you! I know that\n> > you try to stay away from Tcl code, so I appreciate the effort very much),\n> > but apart from that it simply languished on the mailing list for more than\n> > two months now.\n> >\n> > Paul, is there anything I can do to help you integrate this into `gitk`?\n> > Or is it time to pass over `gitk` maintenance to the Git project?\n> \n> I just tripped over this problem while trying to de-stack my Git backlog\n> https://github.com/git-for-windows/git/issues/4408 \"sdk gitk interaction\".\n> \n> > On Tue, 24 Jan 2023, Johannes Schindelin via GitGitGadget wrote:\n> >\n> >> These patches have been cooking for such a long time in Git for Windows that\n> >> you might think they turned into broth. Yummy broth, to be sure. But broth.\n> >> 'Tis beyond time for the patches to make it upstream.\n> \n> If there was a way to un-stick this [1] it would be great.\n\nOne of the main selling points of git is that it was a *distributed* version\ncontrol system, which means there wasn't any centralized repository.\n\nWe are not forced to use Paul Mackerras' repository, so I created a fork that\nincludes the outstanding patches [1]. Junio can simply pull from there from now\non.\n\nI don't want to maintain this, but as I've often found out is the case in open\nsource: if I don't do it, nobody else will.\n\nIf anyone else wants to step up and maintain gitk, that would be great.\n\nCheers.\n\n[1] https://github.com/felipec/gitk\n\n-- \nFelipe Contreras\n"},{"id":"476783","messageId":"xmqqbkiupv4m.fsf@gitster.g","threadId":"59145","inReplyTo":"fce603aa-ceb7-d601-6a19-657e36d9059c@iee.email","subject":"Re: [PATCH 0/2] gitk: handle long command-lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-08T17:53:45Z","receivedAt":"2023-05-08T17:53:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> On 27/03/2023 10:17, Johannes Schindelin wrote:\n>> Hi Pau & Junio,\n>>\n>> this patch series saw a positive review from Junio (thank you! I know that\n>> you try to stay away from Tcl code, so I appreciate the effort very much),\n>> but apart from that it simply languished on the mailing list for more than\n>> two months now.\n>>\n>> Paul, is there anything I can do to help you integrate this into `gitk`?\n>> Or is it time to pass over `gitk` maintenance to the Git project?\n>>\n>> Ciao,\n>> Johannes\n>\n> I just tripped over this problem while trying to de-stack my Git backlog\n> https://github.com/git-for-windows/git/issues/4408 \"sdk gitk interaction\".\n\nI have done the same \"create temporary fork of gitk, queue the\npatches, merge the result down while asking Paul to pull from me\"\ndance I did every once in a while in the past (it seems the last I\ndid it was in Sep 2019 [*]).\n\n * The last merge from Paulus is ef9b086d (Merge branch 'master' of\n git://git.ozlabs.org/~paulus/gitk, 2022-05-11), whose second\n parent (i.e. Paulus's tip) was 465f0386 (gitk: include y coord in\n recorded sash position, 2022-02-20).\n\n * The two patches that originate from GfW have been applied on top\n of 465f0386 in the gitk history; the result of this is 7dd272ec\n (gitk: escape file paths before piping to git log, 2023-01-24).\n\n * The js/gitk-fixes-from-gfw topic holds a -Xsubtree=gitk-git merge\n of 7dd272ec into my tree. This was merged to 'next' and\n hopefully will finish the usual 'next' to 'master' journey\n soonish.\n\nPaul, can you fetch js/gitk-fixes-from-gfw branch of\nhttps://github.com/gitster/git/, which will give you 7dd272ec that\nyou can use to\n\n $ git merge 7dd272ec\n\nto obtain these two commits from Johannes and Nico?\n\nAlternatively, you can fetch 'seen' from any of the mirrors, as it\nhopefully should always contain that topic from now on.\n\nThanks.\n\n\n[References]\n\n* https://lore.kernel.org/git/xmqqlfum7epn.fsf@gitster-ct.c.googlers.com/\n"},{"id":"476863","messageId":"6459a40291a4a_7c682948d@chronos.notmuch","threadId":"59145","inReplyTo":"xmqqbkiupv4m.fsf@gitster.g","subject":"Re: [PATCH 0/2] gitk: handle long command-lines","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-09T01:38:10Z","receivedAt":"2023-05-09T01:38:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Philip Oakley writes:\n> > On 27/03/2023 10:17, Johannes Schindelin wrote:\n\n> >> Paul, is there anything I can do to help you integrate this into `gitk`?\n> >> Or is it time to pass over `gitk` maintenance to the Git project?\n> >\n> > I just tripped over this problem while trying to de-stack my Git backlog\n> > https://github.com/git-for-windows/git/issues/4408 \"sdk gitk interaction\".\n\n> * The two patches that originate from GfW have been applied on top\n> of 465f0386 in the gitk history; the result of this is 7dd272ec\n> (gitk: escape file paths before piping to git log, 2023-01-24).\n\nWhy only those patches?\n\nDoes patches signed off by GfW developers carry a special status\nregardless of their content?\n\n-- \nFelipe Contreras\n"}]}