git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/2] gitk: handle long command-lines
@ 2023-01-24 11:23 Johannes Schindelin via GitGitGadget
  2023-01-24 11:23 ` [PATCH 1/2] gitk: prevent overly long command lines Johannes Schindelin via GitGitGadget
  2023-01-24 11:23 ` [PATCH 2/2] gitk: escape file paths before piping to git log Nico Rieck via GitGitGadget
  0 siblings, 2 replies; 4+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2023-01-24 11:23 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Johannes Schindelin

These patches have been cooking for such a long time in Git for Windows that
you might think they turned into broth. Yummy broth, to be sure. But broth.
'Tis beyond time for the patches to make it upstream.

Johannes Schindelin (1):
  gitk: prevent overly long command lines

Nico Rieck (1):
  gitk: escape file paths before piping to git log

 gitk | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)


base-commit: 465f03869ae11acd04abfa1b83c67879c867410c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1469%2Fdscho%2Fgitk-long-cmdline-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1469/dscho/gitk-long-cmdline-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1469
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] gitk: prevent overly long command lines
  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
  2023-01-24 15:38   ` Junio C Hamano
  2023-01-24 11:23 ` [PATCH 2/2] gitk: escape file paths before piping to git log Nico Rieck via GitGitGadget
  1 sibling, 1 reply; 4+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2023-01-24 11:23 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Johannes Schindelin, Johannes Schindelin

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] gitk: escape file paths before piping to git log
  2023-01-24 11:23 [PATCH 0/2] gitk: handle long command-lines Johannes Schindelin via GitGitGadget
  2023-01-24 11:23 ` [PATCH 1/2] gitk: prevent overly long command lines Johannes Schindelin via GitGitGadget
@ 2023-01-24 11:23 ` Nico Rieck via GitGitGadget
  1 sibling, 0 replies; 4+ messages in thread
From: Nico Rieck via GitGitGadget @ 2023-01-24 11:23 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Johannes Schindelin, Nico Rieck

From: Nico Rieck <nico.rieck@gmail.com>

We just started piping the file paths via `stdin` instead of passing
them via the command-line, to avoid running into command-line
limitations.

However, since we now pipe the file paths, we need to take care of
special characters.

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

Signed-off-by: Nico Rieck <nico.rieck@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 gitk | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/gitk b/gitk
index 92375ca6a2a..df3ba2ea99b 100755
--- a/gitk
+++ b/gitk
@@ -353,6 +353,16 @@ proc parseviewrevs {view revs} {
     return $ret
 }
 
+# Escapes a list of filter paths to be passed to git log via stdin. Note that
+# paths must not be quoted.
+proc escape_filter_paths {paths} {
+	set escaped [list]
+	foreach path $paths {
+		lappend escaped [string map {\\ \\\\ "\ " "\\\ "} $path]
+	}
+	return $escaped
+}
+
 # Start off a git log process and arrange to read its output
 proc start_rev_list {view} {
     global startmsecs commitidx viewcomplete curview
@@ -414,7 +424,8 @@ proc start_rev_list {view} {
     if {[catch {
         set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
                         --parents --boundary $args --stdin \
-                        "<<[join [concat $revs "--" $files] "\\n"]"] r]
+                        "<<[join [concat $revs "--" \
+                                [escape_filter_paths $files]] "\\n"]"] r]
     } err]} {
         error_popup "[mc "Error executing git log:"] $err"
         return 0
@@ -568,7 +579,8 @@ proc updatecommits {} {
         set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
                         --parents --boundary $args --stdin \
                         "<<[join [concat $revs "--" \
-                                $vfilelimit($view)] "\\n"]"] r]
+                                [escape_filter_paths \
+                                        $vfilelimit($view)]] "\\n"]"] r]
     } err]} {
         error_popup "[mc "Error executing git log:"] $err"
         return
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] gitk: prevent overly long command lines
  2023-01-24 11:23 ` [PATCH 1/2] gitk: prevent overly long command lines Johannes Schindelin via GitGitGadget
@ 2023-01-24 15:38   ` Junio C Hamano
  0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2023-01-24 15:38 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget
  Cc: git, Paul Mackerras, Johannes Schindelin

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

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

Makes perfect sense.  I do not know the point of saying exactly
here, though.

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

Nice, so this is not about "we may have too many args to fit in
our memory", but about "we may have too many args for system to
spawn the subprocess with".

> 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`.

It sounds like a design mistake, which may want to be fixed, but of
course gitk cannot depend on Git it runs with having such a fix, and
use of "^" prefix is a good alternative (after all, "--not" was
invented to save us writing ^ in front of many revs).

Good.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-01-24 15:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-24 11:23 [PATCH 0/2] gitk: handle long command-lines Johannes Schindelin via GitGitGadget
2023-01-24 11:23 ` [PATCH 1/2] gitk: prevent overly long command lines Johannes Schindelin via GitGitGadget
2023-01-24 15:38   ` Junio C Hamano
2023-01-24 11:23 ` [PATCH 2/2] gitk: escape file paths before piping to git log Nico Rieck via GitGitGadget

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).