From: Junio C Hamano Date: Tue, 29 Sep 2026 16:51:53 GMT Subject: Re: [PATCH v3 2/2] merge-ll: use tempfile API for external driver files Message-ID: In-Reply-To: <20260929051312.GB1100669@coredump.intra.peff.net> Jeff King writes: > 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"? > > [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.