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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 12, 2020, 19:47 UTC
Message-ID: <xmqqy2kbns5l.fsf@gitster.c.googlers.com>
In-Reply-To: <20201011224804.722607-1-samuel@cavoj.net>

Samuel Čavoj <samuel@cavoj.net> writes:

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

Puzzling.  Why do you single out octopus merge like this?

sequencer.c::do_merge() is called from pick_commits() whenever we see a "merge" insn, and not limited to an octopus merge.

Can we have a test to demonstrate the existing failure, so that we can notice if this fix is broken in the future?

Show 28 quoted lines

> When performing octopus merges with interactive rebase with gpgsign
> enabled (either using rebase -S or config commit.gpgsign), the operation
> would fail on the merge. Instead of "-S%s" with the key id substituted,
> only the bare key id would get passed to the underlying merge command,
> which tried to interpret it as a ref.
>
> Signed-off-by: Samuel Čavoj <samuel@cavoj.net>
> ---
> It is unclear to me whether I should have based this off of maint or
> master, master made more sense to me. I apologize if maint was the
> correct one, please tell and I will resubmit.
> ---
>  sequencer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 00acb12496..88ccff4838 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3677,7 +3677,7 @@ static int do_merge(struct repository *r,
>  		strvec_push(&cmd.args, "-F");
>  		strvec_push(&cmd.args, git_path_merge_msg(r));
>  		if (opts->gpg_sign)
> -			strvec_push(&cmd.args, opts->gpg_sign);
> +			strvec_pushf(&cmd.args, "-S%s", opts->gpg_sign);
>  
>  		/* Add the tips to be merged */
>  		for (j = to_merge; j; j = j->next)