[PATCH v3 0/2] merge-ll: Cleanup merge driver temporaries after signal
flat view
COOLING10d
From: Jeff King <hidden>
Date: 2026-09-29 05:12:08
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ý [off-list ref]
Reported-by: Jean Delvare [off-list ref]
+ Reported-by: Michal Koutný [off-list ref]
Signed-off-by: Jeff King [off-list ref]
## 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);