Re: [PATCH v6 06/13] merge-index: don't fork if the requested program is `git-merge-one-file'

Alban Gruin [off-list ref] 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.

Hi Junio,

Le 06/01/2021 à 03:04, Junio C Hamano a écrit :

Alban Gruin [off-list ref] writes:

quoted

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