git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 2/2] merge-ll: use tempfile API for external driver files

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 29, 2026, 16:51 UTC
Message-ID
<xmqqcxtwhufq.fsf@gitster.g>
In-Reply-To
<20260929051312.GB1100669@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 22 quoted lines
> 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
> 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.
>
> We can fix this by using the tempfile API, which auto-cleans files on
> signal or other error. That covers the Ctrl+C case above, as well as any
> other incidental death (e.g., allocation error due to a gigantic
> output).
>
> Note that there is one gotcha here. The current code uses short,
> relative filenames for the tempfiles (like ".merge_file_abc123"). But
> the tempfile API stores and returns absolute paths. Because we run the
> merge driver as a shell command, this can result in problems if the
> leading directories contain shell metacharacters (like our tests, which
> put a space in the trash directory name for exactly this purpose).
>
> If we were starting from scratch, I'd say the correct solution here is
> to shell-quote the filenames we put in the command. But doing so isn't
> strictly backwards compatible, because users might have their own shell
> characters. For example, if I configure a driver like this:
"own shell characters" -> "own shell quoting"?
Show 11 quoted lines
>
>   [merge "foo"]
>   driver = "my-driver '%O' '%A' '%B'"
>
> then adding extra quoting will screw things up! Strictly speaking, this
> kind of quoting is wrong (it would fail if %A expanded to something with
> a single-quote in it), but it is entirely harmless with the current
> vanilla relative paths. It doesn't seem worth breaking it.
>
> So let's take the most conservative route, and just continue reporting
> the relative paths.

Very well reasoned, and the implementation exactly matches the designed behaviour.

Will replace. Let's mark it for 'next' (unless somebody notices what I overlooked, which is not a very high bar to cross).

Thanks.
Previous: Jeff KingNext: Jeff King
Message 21 of 22 in “merge-ll: Cleanup merge driver temporaries after interrupt”
  1. merge-ll: Cleanup merge driver temporaries after interruptMichal Koutný, Sep 10, 2026
  2. Jeff KingSep 10, 2026
  3. Michal KoutnýSep 11, 2026
  4. 0/3 merge-ll: Cleanup merge driver temporaries afterJeff King, Sep 11, 2026
  5. 1/3 merge-ll: use strbuf to read back external merge resultJeff King, Sep 11, 2026
  6. Elijah NewrenSep 11, 2026
  7. Junio C HamanoSep 11, 2026
  8. Jeff KingSep 14, 2026
  9. 2/3 merge-ll: catch close() errors when writing external tempfilesJeff King, Sep 11, 2026
  10. Elijah NewrenSep 11, 2026
  11. Jeff KingSep 14, 2026
  12. 3/3 merge-ll: use tempfile API for external driver filesJeff King, Sep 11, 2026
  13. Elijah NewrenSep 11, 2026
  14. Michal KoutnýSep 14, 2026
  15. Jeff KingSep 14, 2026
  16. Michal KoutnýSep 14, 2026
  17. Jeff KingSep 14, 2026
  18. 0/2 merge-ll: Cleanup merge driver temporaries after signalJeff King, Sep 29, 2026
  19. 1/2 merge-ll: catch close() errors when writing external tempfilesJeff King, Sep 29, 2026
  20. 2/2 merge-ll: use tempfile API for external driver filesJeff King, Sep 29, 2026
  21. Junio C HamanoSep 29, 2026
  22. Jeff KingSep 29, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.