From: chib <chib@foxmail.com>
commit_committree opens "git cat-file commit <parent>" to read the tree line for the empty-commit check, reads only the first line, and then closes the pipe while the rest of the commit object (often several kilobytes of commit message) is still unread.
On Linux this is harmless: the child process dies of SIGPIPE when it keeps writing, and that is not reported as an error when the pipe is closed. On Windows there is no SIGPIPE: the native git.exe gets a broken-pipe error when writing and exits with a non-zero status. Tcl's [close] then surfaces that as "child process exited abnormally", the commit is aborted, and the index lock is released with nothing committed. The failure only shows up once the parent commit's object is larger than the pipe buffer: in testing with Git for Windows 2.52, objects up to ~6.5 KiB always succeed while objects of ~9 KiB and up fail 10 out of 10 times (the threshold is around the 8 KiB pipe buffer). Amending a commit with a long message therefore triggers it reliably while short commits slip through. Reading the pipe to EOF before closing fixes it 10 out of 10 times, and is harmless on POSIX platforms where the same test succeeds either way.
Read the rest of the pipe before closing it, mirroring what the amend path already does when loading the parent commit's message.
Signed-off-by: chib <chib@foxmail.com>
---
git-gui: drain the cat-file pipe before closing it
commit_committree opens "git cat-file commit " to read the tree line for
the empty-commit check, reads only the first line, and then closes the
pipe while the rest of the commit object (often several kilobytes of
commit message) is still unread.
On Linux this is harmless: the child process dies of SIGPIPE when it
keeps writing, and that is not reported as an error when the pipe is
closed. On Windows there is no SIGPIPE: the native git.exe gets a
broken-pipe error when writing and exits with a non-zero status. Tcl's
[close] then surfaces that as "child process exited abnormally", the
commit is aborted, and the index lock is released with nothing
committed. The failure only shows up once the parent commit's object is
larger than the pipe buffer: in testing with Git for Windows 2.52,
objects up to ~6.5 KiB always succeed while objects of ~9 KiB and up
fail 10 out of 10 times (the threshold is around the 8 KiB pipe buffer).
Amending a commit with a long message therefore triggers it reliably
while short commits slip through. Reading the pipe to EOF before closing
fixes it 10 out of 10 times, and is harmless on POSIX platforms where
the same test succeeds either way.
Read the rest of the pipe before closing it, mirroring what the amend
path already does when loading the parent commit's message.Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2216%2F1dao%2Fgui-drain-catfile-pipe-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2216/1dao/gui-drain-catfile-pipe-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/2216
lib/commit.tcl | 4 ++++ 1 file changed, 4 insertions(+)
Show changes to lib/commit.tcl +4 −0
diff --git a/lib/commit.tcl b/lib/commit.tcl index 89eb8c7b73..5e5f879f0e 100644 --- a/lib/commit.tcl +++ b/lib/commit.tcl @@ -386,6 +386,10 @@ proc commit_committree {fd_wt curHEAD msg_p} { set fd_ot [git_read [list cat-file commit $PARENT]] fconfigure $fd_ot -encoding iso8859-1 set old_tree [gets $fd_ot] + # Drain the pipe before closing it: on Windows, closing it + # while git cat-file still has output to write makes the + # child process exit with a failure status. + read $fd_ot close $fd_ot if {[string equal -length 5 {tree } $old_tree] base-commit: 5dcb97869546d600a114ef422a135e2e909c923c
-- gitgitgadget