Support for `--no-gpg-sign` in git rebase, cherry-pick, etc

Support for `--no-gpg-sign` in git rebase, cherry-pick, etc

[Dominic Chen]

Hello,

The subcommand `git commit` supports a `--no-gpg-sign` argument, which I
find useful for cases where e.g. a GPG key is specified in `.gitconfig`,
but is located on a hardware key that may not currently be attached to
the system. However, other commands like `git rebase`, `git
cherry-pick`, etc, which internally invoke `git commit`, don't support
this argument at all, making things more cumbersome. It'd be great if
these other commands could also support this argument, and propagate it
internally when calling `git commit`.

Thanks,

Dominic

[PATCH] rebase.c: teach --no-gpg-sign to git-rebase

[Danh Doan]

On 2020-03-30 16:03:55-0400, Dominic Chen <d.c.ddcc@gmail.com> wrote:
> The subcommand `git commit` supports a `--no-gpg-sign` argument, which I
> find useful for cases where e.g. a GPG key is specified in `.gitconfig`,
> but is located on a hardware key that may not currently be attached to
> the system. However, other commands like `git rebase`, `git
> cherry-pick`, etc, which internally invoke `git commit`, don't support

cherry-pick (in git 2.25.1) understands --no-gpg-sign

I've encountered this in the past, but I stopped signing my commit.

Anyways, here is the patch

-----------------8<-----------------
From: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Subject: [PATCH] rebase.c: teach --no-gpg-sign to git-rebase

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-rebase.txt |  5 +++
 builtin/rebase.c             | 10 +++--
 t/t3435-rebase-gpg-sign.sh   | 72 ++++++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 3 deletions(-)
 create mode 100755 t/t3435-rebase-gpg-sign.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f7a6033607..54023cf3bb 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -358,6 +358,11 @@ See also INCOMPATIBLE OPTIONS below.
 	defaults to the committer identity; if specified, it must be
 	stuck to the option without a space.
 
+--no-gpg-sign::
+	Countermand `commit.gpgSign` configuration variable that is
+	set to force each and every commit to be signed.
+
+
 -q::
 --quiet::
 	Be quiet. Implies --no-stat.
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 27a07d4e78..a8cc5cfe0c 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1593,6 +1593,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
 	options.allow_empty_message = 1;
 	git_config(rebase_config, &options);
+	// options.gpg_sign_opt will be either "-S" or NULL
+	// It'll be freed later, hence, no skip-prefix
+	gpg_sign = options.gpg_sign_opt ? "" : NULL;
 
 	if (options.use_legacy_rebase ||
 	    !git_env_bool("GIT_TEST_REBASE_USE_BUILTIN", -1))
@@ -1823,10 +1826,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (options.empty != EMPTY_UNSPECIFIED)
 		imply_merge(&options, "--empty");
 
-	if (gpg_sign) {
-		free(options.gpg_sign_opt);
+	free(options.gpg_sign_opt);
+	if (gpg_sign)
 		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
-	}
+	else
+		options.gpg_sign_opt = NULL;
 
 	if (exec.nr) {
 		int i;
diff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh
new file mode 100755
index 0000000000..d12b30b033
--- /dev/null
+++ b/t/t3435-rebase-gpg-sign.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Copyright (c) 2020 Doan Tran Cong Danh
+#
+
+test_description='test rebase --[no-]gpg-sign'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+if ! test_have_prereq GPG
+then
+	skip_all='skip all test rebase --[no-]gpg-sign, gpg not available'
+	test_done
+fi
+
+test_expect_success 'setup: not-signed commit' '
+	test_commit one &&
+	test_commit two &&
+	test_must_fail git verify-commit HEAD &&
+	test_must_fail git verify-commit HEAD^ &&
+	git tag unsigned
+'
+
+test_expect_success 'setup: rebase --gpg-sign to sign all commit' '
+	git rebase --gpg-sign --force-rebase --root &&
+	git verify-commit HEAD &&
+	git verify-commit HEAD^ &&
+	git tag signed
+'
+
+test_expect_success 'rebase without commit.gpgsign config' '
+	git reset --hard signed &&
+	test_might_fail git config --unset commit.gpgsign &&
+	git rebase --force-rebase --root &&
+	test_must_fail git verify-commit HEAD &&
+	test_must_fail git verify-commit HEAD^
+'
+
+test_expect_success 'rebase respects commit.gpgsign=true config' '
+	git reset --hard unsigned &&
+	git config commit.gpgsign true &&
+	git rebase --force-rebase --root &&
+	git verify-commit HEAD &&
+	git verify-commit HEAD^
+'
+
+test_expect_success 'rebase --no-gpg-sign overrides commit.gpgsign' '
+	git reset --hard unsigned &&
+	git config commit.gpgsign true &&
+	git rebase --no-gpg-sign --force-rebase --root &&
+	test_must_fail git verify-commit HEAD &&
+	test_must_fail git verify-commit HEAD^
+'
+
+test_expect_success 'rebase --no-gpg-sign clear signed commit' '
+	git reset --hard signed &&
+	git config commit.gpgsign true &&
+	git rebase --no-gpg-sign --force-rebase --root &&
+	test_must_fail git verify-commit HEAD &&
+	test_must_fail git verify-commit HEAD^
+'
+
+test_expect_success 'rebase -i --no-gpg-sign override commit.gpgsign' '
+	git reset --hard signed &&
+	git config commit.gpgsign true &&
+	GIT_EDITOR=true git rebase -i --no-gpg-sign --force-rebase --root &&
+	test_must_fail git verify-commit HEAD &&
+	test_must_fail git verify-commit HEAD^
+'
+
+test_done
-- 
2.26.0.334.g6536db25bb

Re: [PATCH] rebase.c: teach --no-gpg-sign to git-rebase

[Junio C Hamano]

Danh Doan <congdanhqx@gmail.com> writes:

> From: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> Subject: [PATCH] rebase.c: teach --no-gpg-sign to git-rebase
>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index f7a6033607..54023cf3bb 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -358,6 +358,11 @@ See also INCOMPATIBLE OPTIONS below.
>  	defaults to the committer identity; if specified, it must be
>  	stuck to the option without a space.
>  
> +--no-gpg-sign::
> +	Countermand `commit.gpgSign` configuration variable that is
> +	set to force each and every commit to be signed.
> +
> +

Two points.  

 - There must be already an entry for '--gpg-sign'.  It would make
   more sense to make this addtion a part of its description.

 - The --no-<option> form is not just to override a configured
   default, but also to coumtermand an option given earlier on the
   command line.  In other words "rebase -S --no-gpg-sign" without
   any commit.gpgSign should work just fine.

> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 27a07d4e78..a8cc5cfe0c 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1593,6 +1593,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  
>  	options.allow_empty_message = 1;
>  	git_config(rebase_config, &options);
> +	// options.gpg_sign_opt will be either "-S" or NULL
> +	// It'll be freed later, hence, no skip-prefix

Don't use //- comments.

> +	gpg_sign = options.gpg_sign_opt ? "" : NULL;

We've read configured commit.gpgSign in options.gpg_sign_opt; it is
either a freeable "-S" or NULL depending on its value.  We initialize
the local gpg_sign variable to either an unfreeable "" or NULL here.

Let's see how that local variable is later used here.  We know it is
given as the target variable to OPTION_STRING, which will overwrite
with the value given from the command line, so "" that is unfreeable
avoids an unnecessary leak.

 - If we did not have --gpg-sign, or --no-gpg-sign, then the local
   variable gpg_sign will stay to be either "" or NULL after
   parse_options() returns.

 - If we had --gpg-sign or --no-gpg-sign, we will have the value
   given from the last one of them on the command line in gpg_sign
   after parse_options() returns.



> @@ -1823,10 +1826,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	if (options.empty != EMPTY_UNSPECIFIED)
>  		imply_merge(&options, "--empty");
>  
> -	if (gpg_sign) {
> -		free(options.gpg_sign_opt);
> +	free(options.gpg_sign_opt);
> +	if (gpg_sign)
>  		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
> -	}
> +	else
> +		options.gpg_sign_opt = NULL;

Now we _always_ override options.gpg_sign_opt based on the value in
the local gpg_sign variable, so the *ONLY* time options.gpg_sign_opt
is used is immediately after git_config() returns to decide what
value to assign to gpg_sign we saw above.  I *think* it would be
much clearer to FREE_AND_NULL options.gpg_sign_out immediately after
we initialize gpg_sign above, instead of freeing it here.

Then you do not need the elese clause here, either.

This is a total tangent, but do we ever call cmd_rebase__interactive()
these days?  It does not seem to do the config thing, and assigns the
string taken from the command line to opts.gpg_sign_opt, which means
that it is an error to free the field in any codepath that can be
reached from there.

I suspect that after removing "rebase --preserve-merges", there is
nobody that calls "git rebase--interactive", and at that point the
function will be dead-code and can safely be removed.

Thanks.

Re: [PATCH] rebase.c: teach --no-gpg-sign to git-rebase

[Danh Doan]

On 2020-04-01 10:47:15-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Two points.  
> 
>  - There must be already an entry for '--gpg-sign'.  It would make
>    more sense to make this addtion a part of its description.
> 
>  - The --no-<option> form is not just to override a configured
>    default, but also to coumtermand an option given earlier on the
>    command line.  In other words "rebase -S --no-gpg-sign" without
>    any commit.gpgSign should work just fine.

That paragraph was copy-pasted from git-commit documentation.
I think it would need a clean up there, too.

And, mention of --no-gpg-sign in am, cherry-pick, revert,
merge-option.

While writing this, I've checked (again) all commands mentioned
--gpg-sign. To my surprise, "revert" (despite shares most of code with
"cherry-pick") doesn't honour --no-gpg-sign, either.

I'll teach "--no-gpg-sign" too revert and update all documentation for
this.

> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 27a07d4e78..a8cc5cfe0c 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -1593,6 +1593,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> >  
> >  	options.allow_empty_message = 1;
> >  	git_config(rebase_config, &options);
> > +	// options.gpg_sign_opt will be either "-S" or NULL
> > +	// It'll be freed later, hence, no skip-prefix
> 
> Don't use //- comments.
>
> > +	gpg_sign = options.gpg_sign_opt ? "" : NULL;
> 
> We've read configured commit.gpgSign in options.gpg_sign_opt; it is
> either a freeable "-S" or NULL depending on its value.  We initialize
> the local gpg_sign variable to either an unfreeable "" or NULL here.
> 
> Let's see how that local variable is later used here.  We know it is
> given as the target variable to OPTION_STRING, which will overwrite
> with the value given from the command line, so "" that is unfreeable
> avoids an unnecessary leak.
> 
>  - If we did not have --gpg-sign, or --no-gpg-sign, then the local
>    variable gpg_sign will stay to be either "" or NULL after
>    parse_options() returns.
> 
>  - If we had --gpg-sign or --no-gpg-sign, we will have the value
>    given from the last one of them on the command line in gpg_sign
>    after parse_options() returns.
> 
> 
> 
> > @@ -1823,10 +1826,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> >  	if (options.empty != EMPTY_UNSPECIFIED)
> >  		imply_merge(&options, "--empty");
> >  
> > -	if (gpg_sign) {
> > -		free(options.gpg_sign_opt);
> > +	free(options.gpg_sign_opt);
> > +	if (gpg_sign)
> >  		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
> > -	}
> > +	else
> > +		options.gpg_sign_opt = NULL;
> 
> Now we _always_ override options.gpg_sign_opt based on the value in
> the local gpg_sign variable, so the *ONLY* time options.gpg_sign_opt
> is used is immediately after git_config() returns to decide what
> value to assign to gpg_sign we saw above.  I *think* it would be
> much clearer to FREE_AND_NULL options.gpg_sign_out immediately after
> we initialize gpg_sign above, instead of freeing it here.

Make sense,

> Then you do not need the elese clause here, either.
> 
> This is a total tangent, but do we ever call cmd_rebase__interactive()
> these days?  It does not seem to do the config thing, and assigns the
> string taken from the command line to opts.gpg_sign_opt, which means
> that it is an error to free the field in any codepath that can be
> reached from there.

cmd_rebase__interactive go through different code path, and it doesn't
run into above line

> I suspect that after removing "rebase --preserve-merges", there is
> nobody that calls "git rebase--interactive", and at that point the
> function will be dead-code and can safely be removed.

I've grep-ed the code and it's look like only "rebase -p" call
cmd_rebase__interactive,

I've drafted a test, and "rebase -p" indeeds doesn't honour
"--no-gpg-sign",

Consider the deprecation of "--preserve-merges" is more than a year,
I think I'll mark that test as broken instead of trying to fix it.

-- 
Danh

[PATCH v2 0/5] Honour and Document --no-gpg-sign

[Đoàn Trần Công Danh]

The subcommand `git commit` supports a `--no-gpg-sign` argument, which is
useful incases where e.g. a GPG key is specified in `.gitconfig`,
but is located on a hardware key that may not currently be attached to
the system.

Multiple commands that understand --gpg-sign but haven't understand
--no-gpg-sign, yet.

Also, document more commands that honours --no-gpg-sign

Change since v1:
- FREE_AND_NULL as soon as possible
- update test code for rebase --no-gpg-sign
- revert (--edit which is default in tty) now honours --no-gpg-sign
- cherry-pick --edit now honours --no-gpg-sign
- add more documentation

Đoàn Trần Công Danh (5):
  rebase.c: honour --no-gpg-sign
  cherry-pick/revert: honour --no-gpg-sign in all case
  Documentation: document am --no-gpg-sign
  Documentation: reword commit --no-gpg-sign
  Documentation: document merge option --no-gpg-sign

 Documentation/git-am.txt          |  4 +-
 Documentation/git-cherry-pick.txt |  5 +-
 Documentation/git-commit.txt      |  8 ++-
 Documentation/git-rebase.txt      |  5 +-
 Documentation/git-revert.txt      |  5 +-
 Documentation/merge-options.txt   |  5 +-
 builtin/rebase.c                  |  7 +--
 sequencer.c                       |  2 +
 t/t3435-rebase-gpg-sign.sh        | 71 +++++++++++++++++++++++++
 t/t3514-cherry-pick-revert-gpg.sh | 86 +++++++++++++++++++++++++++++++
 10 files changed, 185 insertions(+), 13 deletions(-)
 create mode 100755 t/t3435-rebase-gpg-sign.sh
 create mode 100755 t/t3514-cherry-pick-revert-gpg.sh

-- 
2.26.0.334.g6536db25bb

[PATCH v2 1/5] rebase.c: honour --no-gpg-sign

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-rebase.txt |  5 ++-
 builtin/rebase.c             |  7 ++--
 t/t3435-rebase-gpg-sign.sh   | 71 ++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 4 deletions(-)
 create mode 100755 t/t3435-rebase-gpg-sign.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f7a6033607..3b94c134fe 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -354,9 +354,12 @@ See also INCOMPATIBLE OPTIONS below.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
+	stuck to the option without a space. "--no-gpg-sign" is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier "--gpg-sign".
 
 -q::
 --quiet::
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 27a07d4e78..7e2ad66e9e 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1593,6 +1593,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
 	options.allow_empty_message = 1;
 	git_config(rebase_config, &options);
+	/* options.gpg_sign_opt will be either "-S" or NULL */
+	gpg_sign = options.gpg_sign_opt ? "" : NULL;
+	FREE_AND_NULL(options.gpg_sign_opt);
 
 	if (options.use_legacy_rebase ||
 	    !git_env_bool("GIT_TEST_REBASE_USE_BUILTIN", -1))
@@ -1823,10 +1826,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (options.empty != EMPTY_UNSPECIFIED)
 		imply_merge(&options, "--empty");
 
-	if (gpg_sign) {
-		free(options.gpg_sign_opt);
+	if (gpg_sign)
 		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
-	}
 
 	if (exec.nr) {
 		int i;
diff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh
new file mode 100755
index 0000000000..b47c59c190
--- /dev/null
+++ b/t/t3435-rebase-gpg-sign.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+#
+# Copyright (c) 2020 Doan Tran Cong Danh
+#
+
+test_description='test rebase --[no-]gpg-sign'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-rebase.sh"
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+if ! test_have_prereq GPG
+then
+	skip_all='skip all test rebase --[no-]gpg-sign, gpg not available'
+	test_done
+fi
+
+test_rebase_gpg_sign () {
+	local must_fail= will=will fake_editor=
+	if test "x$1" = "x!"
+	then
+		must_fail=test_must_fail
+		will="won't"
+		shift
+	fi
+	conf=$1
+	shift
+	test_expect_success "rebase $* with commit.gpgsign=$conf $will sign commit" "
+		git reset two &&
+		git config commit.gpgsign $conf &&
+		set_fake_editor &&
+		FAKE_LINES='r 1 p 2' git rebase --force-rebase --root $* &&
+		$must_fail git verify-commit HEAD^ &&
+		$must_fail git verify-commit HEAD
+	"
+}
+
+test_expect_success 'setup' '
+	test_commit one &&
+	test_commit two &&
+	test_must_fail git verify-commit HEAD &&
+	test_must_fail git verify-commit HEAD^
+'
+
+test_expect_success 'setup: merge commit' '
+	test_commit fork-point &&
+	git switch -c side &&
+	test_commit three &&
+	git switch master &&
+	git merge --no-ff side &&
+	git tag merged
+'
+
+test_rebase_gpg_sign ! false
+test_rebase_gpg_sign   true
+test_rebase_gpg_sign ! true  --no-gpg-sign
+test_rebase_gpg_sign ! true  --gpg-sign --no-gpg-sign
+test_rebase_gpg_sign   false --no-gpg-sign --gpg-sign
+test_rebase_gpg_sign   true  -i
+test_rebase_gpg_sign ! true  -i --no-gpg-sign
+test_rebase_gpg_sign ! true  -i --gpg-sign --no-gpg-sign
+test_rebase_gpg_sign   false -i --no-gpg-sign --gpg-sign
+
+test_expect_failure 'rebase -p --no-gpg-sign override commit.gpgsign' '
+	git reset --hard merged &&
+	git config commit.gpgsign true &&
+	git rebase -p --no-gpg-sign --onto=one fork-point master &&
+	test_must_fail git verify-commit HEAD
+'
+
+test_done
-- 
2.26.0.334.g6536db25bb

Re: [PATCH v2 1/5] rebase.c: honour --no-gpg-sign

[Martin Ågren]

Hi Danh,

On Thu, 2 Apr 2020 at 12:17, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
>
>  -S[<keyid>]::
>  --gpg-sign[=<keyid>]::
> +--no-gpg-sign::
>         GPG-sign commits. The `keyid` argument is optional and
>         defaults to the committer identity; if specified, it must be
> -       stuck to the option without a space.
> +       stuck to the option without a space. "--no-gpg-sign" is useful to
> +       countermand both `commit.gpgSign` configuration variable, and
> +       earlier "--gpg-sign".

Please use `backticks`: `--no-gpg-sign` and `--gpg-sign` just like you
do with `commit.gpgSign`. It will then end up typeset in monospace. This
comment applies to all five patches.

Thanks for working on this.

Martin

[PATCH v2 2/5] cherry-pick/revert: honour --no-gpg-sign in all case

[Đoàn Trần Công Danh]

{cherry-pick,revert} --edit hasn't honoured --no-gpg-sign yet.

Pass this option down to git-commit to honour it.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-cherry-pick.txt |  5 +-
 Documentation/git-revert.txt      |  5 +-
 sequencer.c                       |  2 +
 t/t3514-cherry-pick-revert-gpg.sh | 86 +++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100755 t/t3514-cherry-pick-revert-gpg.sh

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 83ce51aedf..89d7a7920a 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -109,9 +109,12 @@ effect to your index in a row.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
+	stuck to the option without a space. "--no-gpg-sign" is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier "--gpg-sign".
 
 --ff::
 	If the current HEAD is the same as the parent of the
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 9d22270757..7e7f39517b 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -90,9 +90,12 @@ effect to your index in a row.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
+	stuck to the option without a space. "--no-gpg-sign" is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier "--gpg-sign".
 
 -s::
 --signoff::
diff --git a/sequencer.c b/sequencer.c
index 6fd2674632..9969355de7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -946,6 +946,8 @@ static int run_git_commit(struct repository *r,
 		argv_array_push(&cmd.args, "--amend");
 	if (opts->gpg_sign)
 		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
+	else
+		argv_array_push(&cmd.args, "--no-gpg-sign");
 	if (defmsg)
 		argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
 	else if (!(flags & EDIT_MSG))
diff --git a/t/t3514-cherry-pick-revert-gpg.sh b/t/t3514-cherry-pick-revert-gpg.sh
new file mode 100755
index 0000000000..5b2e250eaa
--- /dev/null
+++ b/t/t3514-cherry-pick-revert-gpg.sh
@@ -0,0 +1,86 @@
+#!/bin/sh
+#
+# Copyright (c) 2020 Doan Tran Cong Danh
+#
+
+test_description='test {cherry-pick,revert} --[no-]gpg-sign'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+if ! test_have_prereq GPG
+then
+	skip_all='skip all test {cherry-pick,revert} --[no-]gpg-sign, gpg not available'
+	test_done
+fi
+
+test_gpg_sign () {
+	local must_fail= will=will fake_editor=
+	if test "x$1" = "x!"
+	then
+		must_fail=test_must_fail
+		will="won't"
+		shift
+	fi
+	conf=$1
+	cmd=$2
+	cmit=$3
+	shift 3
+	test_expect_success "$cmd $* $cmit with commit.gpgsign=$conf $will sign commit" "
+		git reset --hard tip &&
+		git config commit.gpgsign $conf &&
+		git $cmd $* $cmit &&
+		git rev-list tip.. >rev-list &&
+		$must_fail git verify-commit \$(cat rev-list)
+	"
+}
+
+test_expect_success 'setup' '
+	test_commit one &&
+	git switch -c side &&
+	test_commit side1 &&
+	test_commit side2 &&
+	git switch - &&
+	test_commit two &&
+	test_commit three &&
+	test_commit tip
+'
+
+test_gpg_sign ! false cherry-pick   side
+test_gpg_sign ! false cherry-pick ..side
+test_gpg_sign   true  cherry-pick   side
+test_gpg_sign   true  cherry-pick ..side
+test_gpg_sign ! true  cherry-pick   side --no-gpg-sign
+test_gpg_sign ! true  cherry-pick ..side --no-gpg-sign
+test_gpg_sign ! true  cherry-pick   side --gpg-sign --no-gpg-sign
+test_gpg_sign ! true  cherry-pick ..side --gpg-sign --no-gpg-sign
+test_gpg_sign   false cherry-pick   side --no-gpg-sign --gpg-sign
+test_gpg_sign   false cherry-pick ..side --no-gpg-sign --gpg-sign
+test_gpg_sign   true  cherry-pick   side --edit
+test_gpg_sign   true  cherry-pick ..side --edit
+test_gpg_sign ! true  cherry-pick   side --edit --no-gpg-sign
+test_gpg_sign ! true  cherry-pick ..side --edit --no-gpg-sign
+test_gpg_sign ! true  cherry-pick   side --edit --gpg-sign --no-gpg-sign
+test_gpg_sign ! true  cherry-pick ..side --edit --gpg-sign --no-gpg-sign
+test_gpg_sign   false cherry-pick   side --edit --no-gpg-sign --gpg-sign
+test_gpg_sign   false cherry-pick ..side --edit --no-gpg-sign --gpg-sign
+
+test_gpg_sign ! false revert HEAD  --edit
+test_gpg_sign ! false revert two.. --edit
+test_gpg_sign   true  revert HEAD  --edit
+test_gpg_sign   true  revert two.. --edit
+test_gpg_sign ! true  revert HEAD  --edit --no-gpg-sign
+test_gpg_sign ! true  revert two.. --edit --no-gpg-sign
+test_gpg_sign ! true  revert HEAD  --edit --gpg-sign --no-gpg-sign
+test_gpg_sign ! true  revert two.. --edit --gpg-sign --no-gpg-sign
+test_gpg_sign   false revert HEAD  --edit --no-gpg-sign --gpg-sign
+test_gpg_sign   false revert two.. --edit --no-gpg-sign --gpg-sign
+test_gpg_sign   true  revert HEAD  --no-edit
+test_gpg_sign   true  revert two.. --no-edit
+test_gpg_sign ! true  revert HEAD  --no-edit --no-gpg-sign
+test_gpg_sign ! true  revert two.. --no-edit --no-gpg-sign
+test_gpg_sign ! true  revert HEAD  --no-edit --gpg-sign --no-gpg-sign
+test_gpg_sign ! true  revert two.. --no-edit --gpg-sign --no-gpg-sign
+test_gpg_sign   false revert HEAD  --no-edit --no-gpg-sign --gpg-sign
+
+test_done
-- 
2.26.0.334.g6536db25bb

[PATCH v2 3/5] Documentation: document am --no-gpg-sign

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-am.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index ab5754e05d..05a2c1b887 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -150,7 +150,9 @@ default.   You can use `--no-utf8` to override this.
 --gpg-sign[=<keyid>]::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
+	stuck to the option without a space. "--no-gpg-sign" is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier "--gpg-sign".
 
 --continue::
 -r::
-- 
2.26.0.334.g6536db25bb

[PATCH v2 4/5] Documentation: reword commit --no-gpg-sign

[Đoàn Trần Công Danh]

Merge with --gpg-sign option, and clarify that --no-gpg-sign also
override earlier --gpg-sign.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-commit.txt | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 13f653989f..6f6c06028b 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -350,11 +350,9 @@ changes to tracked files.
 --gpg-sign[=<keyid>]::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
-
---no-gpg-sign::
-	Countermand `commit.gpgSign` configuration variable that is
-	set to force each and every commit to be signed.
+	stuck to the option without a space. "--no-gpg-sign" is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier "--gpg-sign".
 
 \--::
 	Do not interpret any more arguments as options.
-- 
2.26.0.334.g6536db25bb

[PATCH v2 5/5] Documentation: document merge option --no-gpg-sign

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/merge-options.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 40dc4f5e8c..c46e4fe598 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -61,9 +61,12 @@ When not possible, refuse to merge and exit with a non-zero status.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign the resulting merge commit. The `keyid` argument is
 	optional and defaults to the committer identity; if specified,
-	it must be stuck to the option without a space.
+	it must be stuck to the option without a space. "--no-gpg-sign"
+	is useful to countermand both `commit.gpgSign` configuration variable,
+	and earlier "--gpg-sign".
 
 --log[=<n>]::
 --no-log::
-- 
2.26.0.334.g6536db25bb

Re: [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  Documentation/merge-options.txt | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index 40dc4f5e8c..c46e4fe598 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -61,9 +61,12 @@ When not possible, refuse to merge and exit with a non-zero status.
>  
>  -S[<keyid>]::
>  --gpg-sign[=<keyid>]::
> +--no-gpg-sign::
>  	GPG-sign the resulting merge commit. The `keyid` argument is
>  	optional and defaults to the committer identity; if specified,
> -	it must be stuck to the option without a space.
> +	it must be stuck to the option without a space. "--no-gpg-sign"
> +	is useful to countermand both `commit.gpgSign` configuration variable,
> +	and earlier "--gpg-sign".

Shouldn't [4/5] also add '--no-gpg-sign' to the header like the
above?

Other than that, all patches were pleasant read.
Thanks.

Re: [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign

[Danh Doan]

On 2020-04-02 11:07:19-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
> 
> > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> > ---
> >  Documentation/merge-options.txt | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> > index 40dc4f5e8c..c46e4fe598 100644
> > --- a/Documentation/merge-options.txt
> > +++ b/Documentation/merge-options.txt
> > @@ -61,9 +61,12 @@ When not possible, refuse to merge and exit with a non-zero status.
> >  
> >  -S[<keyid>]::
> >  --gpg-sign[=<keyid>]::
> > +--no-gpg-sign::
> >  	GPG-sign the resulting merge commit. The `keyid` argument is
> >  	optional and defaults to the committer identity; if specified,
> > -	it must be stuck to the option without a space.
> > +	it must be stuck to the option without a space. "--no-gpg-sign"
> > +	is useful to countermand both `commit.gpgSign` configuration variable,
> > +	and earlier "--gpg-sign".
> 
> Shouldn't [4/5] also add '--no-gpg-sign' to the header like the
> above?

Yes, my bad. Since it's very small change.
Could you please help me do it instead.

-- 
Danh

[PATCH v3 0/6] Honour and Document --no-gpg-sign

[Đoàn Trần Công Danh]

The subcommand `git commit` supports a `--no-gpg-sign` argument, which is
useful incases where e.g. a GPG key is specified in `.gitconfig`,
but is located on a hardware key that may not currently be attached to
the system.

Multiple commands that understand --gpg-sign but haven't understand
--no-gpg-sign, yet.

Also, document more commands that honours --no-gpg-sign

Change in v3 since v2:
- Use `backticks` instead of "dquote" for options
- Add missing "--no-gpg-sign::"
- Merge git-commit-tree --[no-]gpg-sign"

Change in v2 since v1:
- FREE_AND_NULL as soon as possible
- update test code for rebase --no-gpg-sign
- revert (--edit which is default in tty) now honours --no-gpg-sign
- cherry-pick --edit now honours --no-gpg-sign
- add more documentation

Đoàn Trần Công Danh (6):
  rebase.c: honour --no-gpg-sign
  cherry-pick/revert: honour --no-gpg-sign in all case
  Documentation: document am --no-gpg-sign
  Documentation: reword commit --no-gpg-sign
  Documentation: merge commit-tree --[no-]gpg-sign
  Documentation: document merge option --no-gpg-sign

 Documentation/git-am.txt          |  5 +-
 Documentation/git-cherry-pick.txt |  5 +-
 Documentation/git-commit-tree.txt |  8 ++-
 Documentation/git-commit.txt      |  9 ++--
 Documentation/git-rebase.txt      |  5 +-
 Documentation/git-revert.txt      |  5 +-
 Documentation/merge-options.txt   |  5 +-
 builtin/rebase.c                  |  7 +--
 sequencer.c                       |  2 +
 t/t3435-rebase-gpg-sign.sh        | 71 +++++++++++++++++++++++++
 t/t3514-cherry-pick-revert-gpg.sh | 86 +++++++++++++++++++++++++++++++
 11 files changed, 190 insertions(+), 18 deletions(-)
 create mode 100755 t/t3435-rebase-gpg-sign.sh
 create mode 100755 t/t3514-cherry-pick-revert-gpg.sh

Range-diff against v2:
1:  b886c69e92 ! 1:  b601c99f7b rebase.c: honour --no-gpg-sign
    @@ Metadata
      ## Commit message ##
         rebase.c: honour --no-gpg-sign
     
    -    Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
    -
      ## Documentation/git-rebase.txt ##
     @@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below.
      
    @@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below.
      	GPG-sign commits. The `keyid` argument is optional and
      	defaults to the committer identity; if specified, it must be
     -	stuck to the option without a space.
    -+	stuck to the option without a space. "--no-gpg-sign" is useful to
    ++	stuck to the option without a space. `--no-gpg-sign` is useful to
     +	countermand both `commit.gpgSign` configuration variable, and
    -+	earlier "--gpg-sign".
    ++	earlier `--gpg-sign`.
      
      -q::
      --quiet::
2:  e1b6d7866d ! 2:  28ebbfe72a cherry-pick/revert: honour --no-gpg-sign in all case
    @@ Documentation/git-cherry-pick.txt: effect to your index in a row.
      	GPG-sign commits. The `keyid` argument is optional and
      	defaults to the committer identity; if specified, it must be
     -	stuck to the option without a space.
    -+	stuck to the option without a space. "--no-gpg-sign" is useful to
    ++	stuck to the option without a space. `--no-gpg-sign` is useful to
     +	countermand both `commit.gpgSign` configuration variable, and
    -+	earlier "--gpg-sign".
    ++	earlier `--gpg-sign`.
      
      --ff::
      	If the current HEAD is the same as the parent of the
    @@ Documentation/git-revert.txt: effect to your index in a row.
      	GPG-sign commits. The `keyid` argument is optional and
      	defaults to the committer identity; if specified, it must be
     -	stuck to the option without a space.
    -+	stuck to the option without a space. "--no-gpg-sign" is useful to
    ++	stuck to the option without a space. `--no-gpg-sign` is useful to
     +	countermand both `commit.gpgSign` configuration variable, and
    -+	earlier "--gpg-sign".
    ++	earlier `--gpg-sign`.
      
      -s::
      --signoff::
3:  9cc14b52e4 < -:  ---------- Documentation: document am --no-gpg-sign
4:  801750016e < -:  ---------- Documentation: reword commit --no-gpg-sign
-:  ---------- > 3:  340374fb68 Documentation: document am --no-gpg-sign
-:  ---------- > 4:  1b952f7348 Documentation: reword commit --no-gpg-sign
-:  ---------- > 5:  685832b069 Documentation: merge commit-tree --[no-]gpg-sign
5:  857178defd ! 6:  b1a3d42355 Documentation: document merge option --no-gpg-sign
    @@ Documentation/merge-options.txt: When not possible, refuse to merge and exit wit
      	GPG-sign the resulting merge commit. The `keyid` argument is
      	optional and defaults to the committer identity; if specified,
     -	it must be stuck to the option without a space.
    -+	it must be stuck to the option without a space. "--no-gpg-sign"
    ++	it must be stuck to the option without a space. `--no-gpg-sign`
     +	is useful to countermand both `commit.gpgSign` configuration variable,
    -+	and earlier "--gpg-sign".
    ++	and earlier `--gpg-sign`.
      
      --log[=<n>]::
      --no-log::
-- 
2.26.0.334.g6536db25bb

[PATCH v3 1/6] rebase.c: honour --no-gpg-sign

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-rebase.txt |  5 ++-
 builtin/rebase.c             |  7 ++--
 t/t3435-rebase-gpg-sign.sh   | 71 ++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 4 deletions(-)
 create mode 100755 t/t3435-rebase-gpg-sign.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f7a6033607..19e280f93f 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -354,9 +354,12 @@ See also INCOMPATIBLE OPTIONS below.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
+	stuck to the option without a space. `--no-gpg-sign` is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier `--gpg-sign`.
 
 -q::
 --quiet::
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 27a07d4e78..7e2ad66e9e 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1593,6 +1593,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
 	options.allow_empty_message = 1;
 	git_config(rebase_config, &options);
+	/* options.gpg_sign_opt will be either "-S" or NULL */
+	gpg_sign = options.gpg_sign_opt ? "" : NULL;
+	FREE_AND_NULL(options.gpg_sign_opt);
 
 	if (options.use_legacy_rebase ||
 	    !git_env_bool("GIT_TEST_REBASE_USE_BUILTIN", -1))
@@ -1823,10 +1826,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (options.empty != EMPTY_UNSPECIFIED)
 		imply_merge(&options, "--empty");
 
-	if (gpg_sign) {
-		free(options.gpg_sign_opt);
+	if (gpg_sign)
 		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
-	}
 
 	if (exec.nr) {
 		int i;
diff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh
new file mode 100755
index 0000000000..b47c59c190
--- /dev/null
+++ b/t/t3435-rebase-gpg-sign.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+#
+# Copyright (c) 2020 Doan Tran Cong Danh
+#
+
+test_description='test rebase --[no-]gpg-sign'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-rebase.sh"
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+if ! test_have_prereq GPG
+then
+	skip_all='skip all test rebase --[no-]gpg-sign, gpg not available'
+	test_done
+fi
+
+test_rebase_gpg_sign () {
+	local must_fail= will=will fake_editor=
+	if test "x$1" = "x!"
+	then
+		must_fail=test_must_fail
+		will="won't"
+		shift
+	fi
+	conf=$1
+	shift
+	test_expect_success "rebase $* with commit.gpgsign=$conf $will sign commit" "
+		git reset two &&
+		git config commit.gpgsign $conf &&
+		set_fake_editor &&
+		FAKE_LINES='r 1 p 2' git rebase --force-rebase --root $* &&
+		$must_fail git verify-commit HEAD^ &&
+		$must_fail git verify-commit HEAD
+	"
+}
+
+test_expect_success 'setup' '
+	test_commit one &&
+	test_commit two &&
+	test_must_fail git verify-commit HEAD &&
+	test_must_fail git verify-commit HEAD^
+'
+
+test_expect_success 'setup: merge commit' '
+	test_commit fork-point &&
+	git switch -c side &&
+	test_commit three &&
+	git switch master &&
+	git merge --no-ff side &&
+	git tag merged
+'
+
+test_rebase_gpg_sign ! false
+test_rebase_gpg_sign   true
+test_rebase_gpg_sign ! true  --no-gpg-sign
+test_rebase_gpg_sign ! true  --gpg-sign --no-gpg-sign
+test_rebase_gpg_sign   false --no-gpg-sign --gpg-sign
+test_rebase_gpg_sign   true  -i
+test_rebase_gpg_sign ! true  -i --no-gpg-sign
+test_rebase_gpg_sign ! true  -i --gpg-sign --no-gpg-sign
+test_rebase_gpg_sign   false -i --no-gpg-sign --gpg-sign
+
+test_expect_failure 'rebase -p --no-gpg-sign override commit.gpgsign' '
+	git reset --hard merged &&
+	git config commit.gpgsign true &&
+	git rebase -p --no-gpg-sign --onto=one fork-point master &&
+	test_must_fail git verify-commit HEAD
+'
+
+test_done
-- 
2.26.0.334.g6536db25bb

[PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case

[Đoàn Trần Công Danh]

{cherry-pick,revert} --edit hasn't honoured --no-gpg-sign yet.

Pass this option down to git-commit to honour it.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-cherry-pick.txt |  5 +-
 Documentation/git-revert.txt      |  5 +-
 sequencer.c                       |  2 +
 t/t3514-cherry-pick-revert-gpg.sh | 86 +++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100755 t/t3514-cherry-pick-revert-gpg.sh

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 83ce51aedf..75feeef08a 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -109,9 +109,12 @@ effect to your index in a row.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
+	stuck to the option without a space. `--no-gpg-sign` is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier `--gpg-sign`.
 
 --ff::
 	If the current HEAD is the same as the parent of the
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 9d22270757..044276e9da 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -90,9 +90,12 @@ effect to your index in a row.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
+	stuck to the option without a space. `--no-gpg-sign` is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier `--gpg-sign`.
 
 -s::
 --signoff::
diff --git a/sequencer.c b/sequencer.c
index 6fd2674632..9969355de7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -946,6 +946,8 @@ static int run_git_commit(struct repository *r,
 		argv_array_push(&cmd.args, "--amend");
 	if (opts->gpg_sign)
 		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
+	else
+		argv_array_push(&cmd.args, "--no-gpg-sign");
 	if (defmsg)
 		argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
 	else if (!(flags & EDIT_MSG))
diff --git a/t/t3514-cherry-pick-revert-gpg.sh b/t/t3514-cherry-pick-revert-gpg.sh
new file mode 100755
index 0000000000..5b2e250eaa
--- /dev/null
+++ b/t/t3514-cherry-pick-revert-gpg.sh
@@ -0,0 +1,86 @@
+#!/bin/sh
+#
+# Copyright (c) 2020 Doan Tran Cong Danh
+#
+
+test_description='test {cherry-pick,revert} --[no-]gpg-sign'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+if ! test_have_prereq GPG
+then
+	skip_all='skip all test {cherry-pick,revert} --[no-]gpg-sign, gpg not available'
+	test_done
+fi
+
+test_gpg_sign () {
+	local must_fail= will=will fake_editor=
+	if test "x$1" = "x!"
+	then
+		must_fail=test_must_fail
+		will="won't"
+		shift
+	fi
+	conf=$1
+	cmd=$2
+	cmit=$3
+	shift 3
+	test_expect_success "$cmd $* $cmit with commit.gpgsign=$conf $will sign commit" "
+		git reset --hard tip &&
+		git config commit.gpgsign $conf &&
+		git $cmd $* $cmit &&
+		git rev-list tip.. >rev-list &&
+		$must_fail git verify-commit \$(cat rev-list)
+	"
+}
+
+test_expect_success 'setup' '
+	test_commit one &&
+	git switch -c side &&
+	test_commit side1 &&
+	test_commit side2 &&
+	git switch - &&
+	test_commit two &&
+	test_commit three &&
+	test_commit tip
+'
+
+test_gpg_sign ! false cherry-pick   side
+test_gpg_sign ! false cherry-pick ..side
+test_gpg_sign   true  cherry-pick   side
+test_gpg_sign   true  cherry-pick ..side
+test_gpg_sign ! true  cherry-pick   side --no-gpg-sign
+test_gpg_sign ! true  cherry-pick ..side --no-gpg-sign
+test_gpg_sign ! true  cherry-pick   side --gpg-sign --no-gpg-sign
+test_gpg_sign ! true  cherry-pick ..side --gpg-sign --no-gpg-sign
+test_gpg_sign   false cherry-pick   side --no-gpg-sign --gpg-sign
+test_gpg_sign   false cherry-pick ..side --no-gpg-sign --gpg-sign
+test_gpg_sign   true  cherry-pick   side --edit
+test_gpg_sign   true  cherry-pick ..side --edit
+test_gpg_sign ! true  cherry-pick   side --edit --no-gpg-sign
+test_gpg_sign ! true  cherry-pick ..side --edit --no-gpg-sign
+test_gpg_sign ! true  cherry-pick   side --edit --gpg-sign --no-gpg-sign
+test_gpg_sign ! true  cherry-pick ..side --edit --gpg-sign --no-gpg-sign
+test_gpg_sign   false cherry-pick   side --edit --no-gpg-sign --gpg-sign
+test_gpg_sign   false cherry-pick ..side --edit --no-gpg-sign --gpg-sign
+
+test_gpg_sign ! false revert HEAD  --edit
+test_gpg_sign ! false revert two.. --edit
+test_gpg_sign   true  revert HEAD  --edit
+test_gpg_sign   true  revert two.. --edit
+test_gpg_sign ! true  revert HEAD  --edit --no-gpg-sign
+test_gpg_sign ! true  revert two.. --edit --no-gpg-sign
+test_gpg_sign ! true  revert HEAD  --edit --gpg-sign --no-gpg-sign
+test_gpg_sign ! true  revert two.. --edit --gpg-sign --no-gpg-sign
+test_gpg_sign   false revert HEAD  --edit --no-gpg-sign --gpg-sign
+test_gpg_sign   false revert two.. --edit --no-gpg-sign --gpg-sign
+test_gpg_sign   true  revert HEAD  --no-edit
+test_gpg_sign   true  revert two.. --no-edit
+test_gpg_sign ! true  revert HEAD  --no-edit --no-gpg-sign
+test_gpg_sign ! true  revert two.. --no-edit --no-gpg-sign
+test_gpg_sign ! true  revert HEAD  --no-edit --gpg-sign --no-gpg-sign
+test_gpg_sign ! true  revert two.. --no-edit --gpg-sign --no-gpg-sign
+test_gpg_sign   false revert HEAD  --no-edit --no-gpg-sign --gpg-sign
+
+test_done
-- 
2.26.0.334.g6536db25bb

Re: [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case

[Phillip Wood]

Hi Đoàn

On 03/04/2020 11:28, Đoàn Trần Công Danh wrote:
> {cherry-pick,revert} --edit hasn't honoured --no-gpg-sign yet.
> 
> Pass this option down to git-commit to honour it.

I did wonder if try_to_commit() needed any changes as we do not fork 
'git commit' unless the message is being edited but the tests seem to 
cover that case. It might be worth checking the code just to be sure if 
you haven't done so already.

Best Wishes

Phillip

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>   Documentation/git-cherry-pick.txt |  5 +-
>   Documentation/git-revert.txt      |  5 +-
>   sequencer.c                       |  2 +
>   t/t3514-cherry-pick-revert-gpg.sh | 86 +++++++++++++++++++++++++++++++
>   4 files changed, 96 insertions(+), 2 deletions(-)
>   create mode 100755 t/t3514-cherry-pick-revert-gpg.sh
> 
> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
> index 83ce51aedf..75feeef08a 100644
> --- a/Documentation/git-cherry-pick.txt
> +++ b/Documentation/git-cherry-pick.txt
> @@ -109,9 +109,12 @@ effect to your index in a row.
>   
>   -S[<keyid>]::
>   --gpg-sign[=<keyid>]::
> +--no-gpg-sign::
>   	GPG-sign commits. The `keyid` argument is optional and
>   	defaults to the committer identity; if specified, it must be
> -	stuck to the option without a space.
> +	stuck to the option without a space. `--no-gpg-sign` is useful to
> +	countermand both `commit.gpgSign` configuration variable, and
> +	earlier `--gpg-sign`.
>   
>   --ff::
>   	If the current HEAD is the same as the parent of the
> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
> index 9d22270757..044276e9da 100644
> --- a/Documentation/git-revert.txt
> +++ b/Documentation/git-revert.txt
> @@ -90,9 +90,12 @@ effect to your index in a row.
>   
>   -S[<keyid>]::
>   --gpg-sign[=<keyid>]::
> +--no-gpg-sign::
>   	GPG-sign commits. The `keyid` argument is optional and
>   	defaults to the committer identity; if specified, it must be
> -	stuck to the option without a space.
> +	stuck to the option without a space. `--no-gpg-sign` is useful to
> +	countermand both `commit.gpgSign` configuration variable, and
> +	earlier `--gpg-sign`.
>   
>   -s::
>   --signoff::
> diff --git a/sequencer.c b/sequencer.c
> index 6fd2674632..9969355de7 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -946,6 +946,8 @@ static int run_git_commit(struct repository *r,
>   		argv_array_push(&cmd.args, "--amend");
>   	if (opts->gpg_sign)
>   		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
> +	else
> +		argv_array_push(&cmd.args, "--no-gpg-sign");
>   	if (defmsg)
>   		argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
>   	else if (!(flags & EDIT_MSG))
> diff --git a/t/t3514-cherry-pick-revert-gpg.sh b/t/t3514-cherry-pick-revert-gpg.sh
> new file mode 100755
> index 0000000000..5b2e250eaa
> --- /dev/null
> +++ b/t/t3514-cherry-pick-revert-gpg.sh
> @@ -0,0 +1,86 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2020 Doan Tran Cong Danh
> +#
> +
> +test_description='test {cherry-pick,revert} --[no-]gpg-sign'
> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-gpg.sh"
> +
> +if ! test_have_prereq GPG
> +then
> +	skip_all='skip all test {cherry-pick,revert} --[no-]gpg-sign, gpg not available'
> +	test_done
> +fi
> +
> +test_gpg_sign () {
> +	local must_fail= will=will fake_editor=
> +	if test "x$1" = "x!"
> +	then
> +		must_fail=test_must_fail
> +		will="won't"
> +		shift
> +	fi
> +	conf=$1
> +	cmd=$2
> +	cmit=$3
> +	shift 3
> +	test_expect_success "$cmd $* $cmit with commit.gpgsign=$conf $will sign commit" "
> +		git reset --hard tip &&
> +		git config commit.gpgsign $conf &&
> +		git $cmd $* $cmit &&
> +		git rev-list tip.. >rev-list &&
> +		$must_fail git verify-commit \$(cat rev-list)
> +	"
> +}
> +
> +test_expect_success 'setup' '
> +	test_commit one &&
> +	git switch -c side &&
> +	test_commit side1 &&
> +	test_commit side2 &&
> +	git switch - &&
> +	test_commit two &&
> +	test_commit three &&
> +	test_commit tip
> +'
> +
> +test_gpg_sign ! false cherry-pick   side
> +test_gpg_sign ! false cherry-pick ..side
> +test_gpg_sign   true  cherry-pick   side
> +test_gpg_sign   true  cherry-pick ..side
> +test_gpg_sign ! true  cherry-pick   side --no-gpg-sign
> +test_gpg_sign ! true  cherry-pick ..side --no-gpg-sign
> +test_gpg_sign ! true  cherry-pick   side --gpg-sign --no-gpg-sign
> +test_gpg_sign ! true  cherry-pick ..side --gpg-sign --no-gpg-sign
> +test_gpg_sign   false cherry-pick   side --no-gpg-sign --gpg-sign
> +test_gpg_sign   false cherry-pick ..side --no-gpg-sign --gpg-sign
> +test_gpg_sign   true  cherry-pick   side --edit
> +test_gpg_sign   true  cherry-pick ..side --edit
> +test_gpg_sign ! true  cherry-pick   side --edit --no-gpg-sign
> +test_gpg_sign ! true  cherry-pick ..side --edit --no-gpg-sign
> +test_gpg_sign ! true  cherry-pick   side --edit --gpg-sign --no-gpg-sign
> +test_gpg_sign ! true  cherry-pick ..side --edit --gpg-sign --no-gpg-sign
> +test_gpg_sign   false cherry-pick   side --edit --no-gpg-sign --gpg-sign
> +test_gpg_sign   false cherry-pick ..side --edit --no-gpg-sign --gpg-sign
> +
> +test_gpg_sign ! false revert HEAD  --edit
> +test_gpg_sign ! false revert two.. --edit
> +test_gpg_sign   true  revert HEAD  --edit
> +test_gpg_sign   true  revert two.. --edit
> +test_gpg_sign ! true  revert HEAD  --edit --no-gpg-sign
> +test_gpg_sign ! true  revert two.. --edit --no-gpg-sign
> +test_gpg_sign ! true  revert HEAD  --edit --gpg-sign --no-gpg-sign
> +test_gpg_sign ! true  revert two.. --edit --gpg-sign --no-gpg-sign
> +test_gpg_sign   false revert HEAD  --edit --no-gpg-sign --gpg-sign
> +test_gpg_sign   false revert two.. --edit --no-gpg-sign --gpg-sign
> +test_gpg_sign   true  revert HEAD  --no-edit
> +test_gpg_sign   true  revert two.. --no-edit
> +test_gpg_sign ! true  revert HEAD  --no-edit --no-gpg-sign
> +test_gpg_sign ! true  revert two.. --no-edit --no-gpg-sign
> +test_gpg_sign ! true  revert HEAD  --no-edit --gpg-sign --no-gpg-sign
> +test_gpg_sign ! true  revert two.. --no-edit --gpg-sign --no-gpg-sign
> +test_gpg_sign   false revert HEAD  --no-edit --no-gpg-sign --gpg-sign
> +
> +test_done
> 

Re: [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case

[Danh Doan]

On 2020-04-03 14:43:05+0100, Phillip Wood <phillip.wood123@gmail.com> wrote:
> Hi Đoàn
> 
> On 03/04/2020 11:28, Đoàn Trần Công Danh wrote:
> > {cherry-pick,revert} --edit hasn't honoured --no-gpg-sign yet.
> > 
> > Pass this option down to git-commit to honour it.
> 
> I did wonder if try_to_commit() needed any changes as we do not fork 'git
> commit' unless the message is being edited but the tests seem to cover that
> case. It might be worth checking the code just to be sure if you haven't
> done so already.

try_to_commit pass opts->gpg_sign to commit_tree_extended (sequencer.c:1413)

if "--edit" was passed into cherry-pick and/or revert (--edit is
default if revert run in tty), sequencer.c will run git-commit just
6 lines above it.

-- 
Danh

[PATCH v3 3/6] Documentation: document am --no-gpg-sign

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-am.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index ab5754e05d..38c0852139 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -148,9 +148,12 @@ default.   You can use `--no-utf8` to override this.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
+	stuck to the option without a space. `--no-gpg-sign` is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier `--gpg-sign`.
 
 --continue::
 -r::
-- 
2.26.0.334.g6536db25bb

[PATCH v3 4/6] Documentation: reword commit --no-gpg-sign

[Đoàn Trần Công Danh]

Merge with --gpg-sign option, and clarify that --no-gpg-sign also
override earlier --gpg-sign.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-commit.txt | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 13f653989f..a3baea32ae 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -348,13 +348,12 @@ changes to tracked files.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
-
---no-gpg-sign::
-	Countermand `commit.gpgSign` configuration variable that is
-	set to force each and every commit to be signed.
+	stuck to the option without a space. `--no-gpg-sign` is useful to
+	countermand both `commit.gpgSign` configuration variable, and
+	earlier `--gpg-sign`.
 
 \--::
 	Do not interpret any more arguments as options.
-- 
2.26.0.334.g6536db25bb

[PATCH v3 5/6] Documentation: merge commit-tree --[no-]gpg-sign

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/git-commit-tree.txt | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
index ec15ee8d6f..2e2c581098 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt
@@ -61,13 +61,11 @@ OPTIONS
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign commits. The `keyid` argument is optional and
 	defaults to the committer identity; if specified, it must be
-	stuck to the option without a space.
-
---no-gpg-sign::
-	Do not GPG-sign commit, to countermand a `--gpg-sign` option
-	given earlier on the command line.
+	stuck to the option without a space. `--no-gpg-sign` is useful to
+	countermand a `--gpg-sign` option given earlier on the command line.
 
 Commit Information
 ------------------
-- 
2.26.0.334.g6536db25bb

[PATCH v3 6/6] Documentation: document merge option --no-gpg-sign

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/merge-options.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 40dc4f5e8c..fb3a6e8d42 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -61,9 +61,12 @@ When not possible, refuse to merge and exit with a non-zero status.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
+--no-gpg-sign::
 	GPG-sign the resulting merge commit. The `keyid` argument is
 	optional and defaults to the committer identity; if specified,
-	it must be stuck to the option without a space.
+	it must be stuck to the option without a space. `--no-gpg-sign`
+	is useful to countermand both `commit.gpgSign` configuration variable,
+	and earlier `--gpg-sign`.
 
 --log[=<n>]::
 --no-log::
-- 
2.26.0.334.g6536db25bb

Re: [PATCH v3 0/6] Honour and Document --no-gpg-sign

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> The subcommand `git commit` supports a `--no-gpg-sign` argument, which is
> useful incases where e.g. a GPG key is specified in `.gitconfig`,
> but is located on a hardware key that may not currently be attached to
> the system.

I see the mark-up in the documentation pages have been changed, and
they all good correct.

Thanks for an update.  Will queue.

Re: [PATCH v3 0/6] Honour and Document --no-gpg-sign

[Martin Ågren]

On Fri, 3 Apr 2020 at 12:28, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
>
> Multiple commands that understand --gpg-sign but haven't understand
> --no-gpg-sign, yet.
>
> Also, document more commands that honours --no-gpg-sign
>
> Change in v3 since v2:
> - Use `backticks` instead of "dquote" for options

Cool, thanks!

> - Add missing "--no-gpg-sign::"
> - Merge git-commit-tree --[no-]gpg-sign"

Martin