git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Paul Mackerras <paulus@ozlabs.org>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 1/2] gitk: prevent overly long command lines
Date: Tue, 24 Jan 2023 11:23:16 +0000	[thread overview]
Message-ID: <89fe0380cd3a373e7e23d663b506466fd6cb5fb6.1674559397.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1469.git.1674559397.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

To avoid running into command line limitations, some of Git's commands
support the `--stdin` option.

Let's use exactly this option in the three rev-list/log invocations in
gitk that would otherwise possibly run the danger of trying to invoke a
too-long command line.

While it is easy to redirect either stdin or stdout in Tcl/Tk scripts,
what we need here is both. We need to capture the output, yet we also
need to pipe in the revs/files arguments via stdin (because stdin does
not have any limit, unlike the command line). To help this, we use the
neat Tcl feature where you can capture stdout and at the same time feed
a fixed string as stdin to the spawned process.

One non-obvious aspect about this change is that the `--stdin` option
allows to specify revs, the double-dash, and files, but *no* other
options such as `--not`. This is addressed by prefixing the "negative"
revs with `^` explicitly rather than relying on the `--not` option
(thanks for coming up with that idea, Max!).

This fixes https://github.com/git-for-windows/git/issues/1987

Analysis-and-initial-patch-by: Max Kirillov <max@max630.net>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 gitk | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/gitk b/gitk
index 0ae7d685904..92375ca6a2a 100755
--- a/gitk
+++ b/gitk
@@ -405,14 +405,16 @@ proc start_rev_list {view} {
         if {$revs eq {}} {
             return 0
         }
-        set args [concat $vflags($view) $revs]
+        set args $vflags($view)
     } else {
+        set revs {}
         set args $vorigargs($view)
     }
 
     if {[catch {
         set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
-                        --parents --boundary $args "--" $files] r]
+                        --parents --boundary $args --stdin \
+                        "<<[join [concat $revs "--" $files] "\\n"]"] r]
     } err]} {
         error_popup "[mc "Error executing git log:"] $err"
         return 0
@@ -554,13 +556,19 @@ proc updatecommits {} {
             set revs $newrevs
             set vposids($view) [lsort -unique [concat $oldpos $vposids($view)]]
         }
-        set args [concat $vflags($view) $revs --not $oldpos]
+        set args $vflags($view)
+        foreach r $oldpos {
+                lappend revs "^$r"
+        }
     } else {
+        set revs {}
         set args $vorigargs($view)
     }
     if {[catch {
         set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
-                        --parents --boundary $args "--" $vfilelimit($view)] r]
+                        --parents --boundary $args --stdin \
+                        "<<[join [concat $revs "--" \
+                                $vfilelimit($view)] "\\n"]"] r]
     } err]} {
         error_popup "[mc "Error executing git log:"] $err"
         return
@@ -10231,10 +10239,16 @@ proc getallcommits {} {
             foreach id $seeds {
                 lappend ids "^$id"
             }
+            lappend ids "--"
         }
     }
     if {$ids ne {}} {
-        set fd [open [concat $cmd $ids] r]
+        if {$ids eq "--all"} {
+            set cmd [concat $cmd "--all"]
+        } else {
+            set cmd [concat $cmd --stdin "<<[join $ids "\\n"]"]
+        }
+        set fd [open $cmd r]
         fconfigure $fd -blocking 0
         incr allcommits
         nowbusy allcommits
-- 
gitgitgadget


  reply	other threads:[~2023-01-24 11:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-24 11:23 [PATCH 0/2] gitk: handle long command-lines Johannes Schindelin via GitGitGadget
2023-01-24 11:23 ` Johannes Schindelin via GitGitGadget [this message]
2023-01-24 15:38   ` [PATCH 1/2] gitk: prevent overly long command lines Junio C Hamano
2023-01-24 11:23 ` [PATCH 2/2] gitk: escape file paths before piping to git log Nico Rieck via GitGitGadget
2023-03-27  9:17 ` [PATCH 0/2] gitk: handle long command-lines Johannes Schindelin
2023-03-27 10:17   ` Felipe Contreras
2023-05-06 16:00   ` Philip Oakley
2023-05-07 21:35     ` New canonical gitk url Felipe Contreras
2023-05-08 17:53     ` [PATCH 0/2] gitk: handle long command-lines Junio C Hamano
2023-05-09  1:38       ` Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=89fe0380cd3a373e7e23d663b506466fd6cb5fb6.1674559397.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=paulus@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).