[PATCH] history: close COMMIT_EDITMSG before launching the editor

["Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>]

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

The `git history reword` and `git history fixup` subcommands prepare the
commit message by writing it to COMMIT_EDITMSG and then opening that same
file a second time, in append mode, through `wt_status`'s `fp` field to
append the status information. That second handle is never closed before
`launch_editor()` runs, so the editor is started while git still holds
the file open.

Everywhere this leaks a file descriptor, but on Windows it is outright
broken: a process cannot replace a file that another process keeps open,
so an editor that rewrites COMMIT_EDITMSG by creating a fresh file in its
place fails. This surfaced while running Git for Windows' test suite with
BusyBox' `ash` as the POSIX shell: the fake editor's `cp message "$1"`
aborts with "cp: can't create '.../COMMIT_EDITMSG': File exists" (MSYS2's
coreutils `cp` hides the problem via its POSIX unlink emulation, BusyBox'
native `cp` does not), making t3451-history-reword and t3453-history-fixup
fail wholesale.

Close the handle once the status has been written, before handing the
file off to the editor.

Assisted-by: Opus 4.8
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    history: close COMMIT_EDITMSG before launching the editor
    
    I noticed this problem while trying to whip MinGit-BusyBox into a better
    shape during the -rc phase. Technically, this is not a fix for a
    regression during the v2.55.0 period, but I figured it'd be better to
    send it now anyway than to forget about sending it after v2.55.0 is
    released.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2158%2Fdscho%2Ffix-fd-leak-in-history-reword-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2158/dscho/fix-fd-leak-in-history-reword-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2158

 builtin/history.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/builtin/history.c b/builtin/history.c
index 9526938085..4a5d9192f3 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -74,6 +74,14 @@ static int fill_commit_message(struct repository *repo,
 	wt_status_collect_free_buffers(&s);
 	string_list_clear_func(&s.change, change_data_free);
 
+	/*
+	 * Close the handle before launching the editor: on Windows an open
+	 * handle would prevent the editor from replacing the file (e.g.
+	 * BusyBox' `ash` cannot overwrite a file that another process keeps
+	 * open), and leaving it open leaks the descriptor everywhere else.
+	 */
+	fclose(s.fp);
+
 	strbuf_reset(out);
 	if (launch_editor(path, out, NULL)) {
 		fprintf(stderr, _("Aborting commit as launching the editor failed.\n"));

base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
-- 
gitgitgadget