From: Alban Gruin Date: Sun, 10 Jan 2021 17:15:52 GMT Subject: Re: [PATCH v6 06/13] merge-index: don't fork if the requested program is `git-merge-one-file' Message-ID: In-Reply-To: Hi Junio, Le 06/01/2021 à 03:04, Junio C Hamano a écrit : > Alban Gruin writes: > >> We had the same discussion with Phillip, who pointed out this previous >> discussion about this topic: >> https://lore.kernel.org/git/xmqqblv5kr9u.fsf@gitster-ct.c.googlers.com/ >> >> So, it's probably OK to do that. > > These days, there exists an optional installation option exists that > won't even install built-in commands in $GIT_EXEC_PATH, which > invalidates the assessment made in 2019 in the article you cited > above, so the code might still be OK, but the old justification no > longer would apply. > > In any case, if two people who reviewed a patch found the same thing > in it fishy, it is an indication that the reason why the apparently > fishy code is OK needs to be better explained so that future readers > of the code do not have to be puzzled about the same thing. > > Thanks. > Perhaps we could try to check if the provided command exists (with locate_in_PATH()), if it does, run it through merge_one_file_spawn(), else, use merge_one_file_func()? Alban