[PATCH v3 0/2] merge-ll: Cleanup merge driver temporaries after signal
- From
Jeff King <peff@peff.net>
- Date
- Sep 29, 2026, 05:12 UTC
- Message-ID
- <20260929051200.GA1100000@coredump.intra.peff.net>
- In-Reply-To
- <20260911171044.GA1609692@coredump.intra.peff.net>
Here's a revised version of the series to switch merge-ll to use tempfile structs. Sorry, I got derailed a bit by travel.
I dropped the v2 cleanup patch to use strbuf_read() for now. It was not strictly related and I think there's a bit of a rabbit hole that extends even beyond this function. That might become its own series later.
Beyond that, this is mostly the same as v2. I tweaked the error-checking for close() in the first patch so that it's more obviously correct (and can produce a slightly more informative message).
The range diff is below, though it's IMHO not very informative. The drop of the cleanup patch a lot of uninteresting textual ripples.
[1/2]: merge-ll: catch close() errors when writing external tempfiles [2/2]: merge-ll: use tempfile API for external driver files
merge-ll.c | 51 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 18 deletions(-)
1: 62b4ac5ae0 < -: ---------- merge-ll: use strbuf to read back external merge result
2: 020e3bfcbd < -: ---------- merge-ll: catch close() errors when writing external tempfiles
-: ---------- > 1: c6a4b3146d merge-ll: catch close() errors when writing external tempfiles
3: 914fafcd88 ! 2: b85e169cb3 merge-ll: use tempfile API for external driver files
@@ Commit message
When there's a long(er) running merge driver helper, the user may just
decide to terminate it with Ctrl+C. That sends a signal to the driver
- prog and to the whole process group as well, including the git merge
+ program and to the whole process group as well, including the git merge
command proper. Hence the cleanup code would not run and .merge_file_*
files are left behind.
@@ Commit message
So let's take the most conservative route, and just continue reporting
the relative paths.
- Commit-message-stolen-from: Michal Koutný <mkoutny@suse.com>
Reported-by: Jean Delvare <jdelvare@suse.de>
+ Reported-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Jeff King <peff@peff.net>
## merge-ll.c ##
@@ merge-ll.c: static struct ll_merge_driver ll_merge_drv[] = {
-
- xsnprintf(path, len, ".merge_file_XXXXXX");
- fd = xmkstemp(path);
-- if (write_in_full(fd, src->ptr, src->size) < 0 ||
-- close(fd) < 0)
+- if (write_in_full(fd, src->ptr, src->size) < 0)
+- die_errno(_("unable to write %s"), path);
+- if (close(fd) < 0)
+- die_errno(_("unable to close %s"), path);
+ struct tempfile *t = xmks_tempfile(".merge_file_XXXXXX");
-+ if (write_in_full(t->fd, src->ptr, src->size) < 0 ||
-+ close_tempfile_gently(t) < 0)
- die_errno("unable to write temp-file");
++ if (write_in_full(t->fd, src->ptr, src->size) < 0)
++ die_errno(_("unable to write %s"), get_tempfile_path(t));
++ if (close_tempfile_gently(t) < 0)
++ die_errno(_("unable to close %s"), get_tempfile_path(t));
+ return t;
+}
+
@@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_drive
struct strbuf cmd = STRBUF_INIT;
const char *format = fn->cmdline;
struct child_process child = CHILD_PROCESS_INIT;
-- int status, i;
-+ int status;
- struct strbuf result_buf = STRBUF_INIT;
+- int status, fd, i;
++ int status, fd;
+ struct stat st;
enum ll_merge_result ret;
assert(opts);
@@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
@@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_drive
strbuf_addf(&cmd, "%d", marker_size);
else if (skip_prefix(format, "P", &format))
@@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
+ child.use_shell = 1;
strvec_push(&child.args, cmd.buf);
status = run_command(&child);
-
-- if (strbuf_read_file(&result_buf, temp[1], 0) >= 0) {
-+ if (strbuf_read_file(&result_buf, get_tempfile_path(tmp_a), 0) >= 0) {
- result->size = result_buf.len;
- result->ptr = strbuf_detach(&result_buf, NULL);
- }
-
+- fd = open(temp[1], O_RDONLY);
++ fd = open(get_tempfile_path(tmp_a), O_RDONLY);
+ if (fd < 0)
+ goto bad;
+ if (fstat(fd, &st))
+@@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
+ close_bad:
+ close(fd);
+ bad:
- for (i = 0; i < 3; i++)
- unlink_or_warn(temp[i]);
+ delete_tempfile(&tmp_o);