git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: "Samuel Čavoj" <samuel@cavoj.net>,
	git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v2 1/2] sequencer: fix gpg option passed to merge subcommand
Date: Tue, 13 Oct 2020 11:02:38 +0100	[thread overview]
Message-ID: <d2491759-e761-647b-7e76-3515b8eeaf3f@gmail.com> (raw)
In-Reply-To: <20201012234901.1356948-1-samuel@cavoj.net>

Hi Samuel

On 13/10/2020 00:49, Samuel Čavoj wrote:
> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> When performing a rebase with --rebase-merges using either a custom
> strategy specified with -s or an octopus merge, and at the same time
> having gpgsign enabled (either rebase -S or config commit.gpgsign), the
> operation would fail on making the merge commit. 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.
> 
> Fix the issue and add a test case as suggested by Johannes Schindelin.
> 
> Signed-off-by: Samuel Čavoj <samuel@cavoj.net>
> ---
> changed v1 -> v2:
>      added test case
> ---
>   sequencer.c                | 2 +-
>   t/t3435-rebase-gpg-sign.sh | 6 ++++++
>   2 files changed, 7 insertions(+), 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)
> diff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh
> index b47c59c190..f70b280f5c 100755
> --- a/t/t3435-rebase-gpg-sign.sh
> +++ b/t/t3435-rebase-gpg-sign.sh
> @@ -68,4 +68,10 @@ test_expect_failure 'rebase -p --no-gpg-sign override commit.gpgsign' '
>   	test_must_fail git verify-commit HEAD
>   '
>   
> +test_expect_success 'rebase -r, GPG and merge strategies' '
> +	git reset --hard merged &&
> +	git rebase -fr --gpg-sign -s resolve --root &&
> +	git verify-commit HEAD
> +'

Unfortunately I've just noticed that the test above runs

	git config commit.gpgsign true

So this test would pass anyway if merge picks up that config setting. 
The previous test needs to be changed to

	test_config commit.gpgsign true

so that the config setting is cleared when that test finishes. That 
previous test would then be a good template for testing `rebase -r 
--no-gpg-sign` if you wanted to add a test for that to the next patch as 
Junio suggested.

Best Wishes

Phillip

>   test_done
> 


  parent reply	other threads:[~2020-10-13 10:02 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-12 23:49 [PATCH v2 1/2] sequencer: fix gpg option passed to merge subcommand Samuel Čavoj
2020-10-12 23:49 ` [PATCH v2 2/2] sequencer: pass explicit --no-gpg-sign to merge Samuel Čavoj
2020-10-13  4:46   ` Junio C Hamano
2020-10-13 10:03   ` Phillip Wood
2020-10-12 23:52 ` [PATCH v2 1/2] sequencer: fix gpg option passed to merge subcommand Samuel Čavoj
2020-10-13  9:55 ` Phillip Wood
2020-10-13 10:43   ` Samuel Čavoj
2020-10-13 13:15     ` Phillip Wood
2020-10-13 10:02 ` Phillip Wood [this message]
2020-10-13 10:51   ` Samuel Čavoj
2020-10-13 13:28     ` Phillip Wood
2020-10-13 22:06   ` Junio C Hamano
2020-10-13 23:45     ` Samuel Čavoj
2020-10-14 15:31       ` Junio C Hamano
2020-10-16 13:40     ` Phillip Wood
2020-10-16 16:37       ` Junio C Hamano
2020-10-16 17:25         ` Phillip Wood
2020-10-16 20:15       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d2491759-e761-647b-7e76-3515b8eeaf3f@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=samuel@cavoj.net \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).