Re: [PATCH] sequencer: fix gpg option passed to octopus merge

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

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>>> In run_git_commit() we do
>>>
>>> 	if (opts->gpg_sign)
>>> 		strvec_pushf(&cmd.args, "-S%s", opts->gpg_sign);
>>> 	else
>>> 		strvec_push(&cmd.args, "--no-gpg-sign");
>>>
>>> I'm not immediately clear why we pass --no-gpg-sign when
>>> opts->gpg_sign isn't set ...
>>
>> Isn't it because there is a configuration that the &cmd may honor
>> that forces gpg signing all the time?
>>
>>> but it makes me wonder if we should be doing
>>> that here as well
>
> I was reacting only based on what I saw in these message, but it
> turns out that cmd above is an internal invocation of "git merge";
> as the command does honor the "commit.gpgsign" option, if "rebase"
> or whatever command that invoked the sequencer turned off the
> signing by setting opts->gpg_sign to false, I agree that the part
> touched by the patch should have the else clause to explicitly
> override the configuration option.

Forgot to say that it is perfectly fine to leave it outside Samuel's
patch.  It "fixes" the "intended" behaviour of the current design
where it somehow was deemed a good idea to disallow overriding
commit.gpgsign variable.  Fixing that design so that we can override
configured default from the command line can be left for a follow-up
patch.

Thanks.