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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> 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.