[PATCH] git-gui: drain the cat-file pipe before closing it

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(+)