git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Support for `--no-gpg-sign` in git rebase, cherry-pick, etc
@ 2020-03-30 20:03 Dominic Chen
  2020-03-31  6:44 ` [PATCH] rebase.c: teach --no-gpg-sign to git-rebase Danh Doan
  0 siblings, 1 reply; 24+ messages in thread
From: Dominic Chen @ 2020-03-30 20:03 UTC (permalink / raw)
  To: git

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] rebase.c: teach --no-gpg-sign to git-rebase
  2020-03-30 20:03 Support for `--no-gpg-sign` in git rebase, cherry-pick, etc Dominic Chen
@ 2020-03-31  6:44 ` Danh Doan
  2020-04-01 17:47   ` Junio C Hamano
                     ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Danh Doan @ 2020-03-31  6:44 UTC (permalink / raw)
  To: Dominic Chen; +Cc: git

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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] rebase.c: teach --no-gpg-sign to git-rebase
  2020-03-31  6:44 ` [PATCH] rebase.c: teach --no-gpg-sign to git-rebase Danh Doan
@ 2020-04-01 17:47   ` Junio C Hamano
  2020-04-02  1:09     ` Danh Doan
  2020-04-02 10:15   ` [PATCH v2 0/5] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
  2 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2020-04-01 17:47 UTC (permalink / raw)
  To: Danh Doan; +Cc: Dominic Chen, git

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] rebase.c: teach --no-gpg-sign to git-rebase
  2020-04-01 17:47   ` Junio C Hamano
@ 2020-04-02  1:09     ` Danh Doan
  0 siblings, 0 replies; 24+ messages in thread
From: Danh Doan @ 2020-04-02  1:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Dominic Chen, git

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 0/5] Honour and Document --no-gpg-sign
  2020-03-31  6:44 ` [PATCH] rebase.c: teach --no-gpg-sign to git-rebase Danh Doan
  2020-04-01 17:47   ` Junio C Hamano
@ 2020-04-02 10:15   ` Đoàn Trần Công Danh
  2020-04-02 10:15     ` [PATCH v2 1/5] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
                       ` (4 more replies)
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
  2 siblings, 5 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-02 10:15 UTC (permalink / raw)
  To: git
  Cc: Đoàn Trần Công Danh, Dominic Chen, Junio C Hamano

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/5] rebase.c: honour --no-gpg-sign
  2020-04-02 10:15   ` [PATCH v2 0/5] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-02 10:15     ` Đoàn Trần Công Danh
  2020-04-03  5:21       ` Martin Ågren
  2020-04-02 10:15     ` [PATCH v2 2/5] cherry-pick/revert: honour --no-gpg-sign in all case Đoàn Trần Công Danh
                       ` (3 subsequent siblings)
  4 siblings, 1 reply; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-02 10:15 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/5] cherry-pick/revert: honour --no-gpg-sign in all case
  2020-04-02 10:15   ` [PATCH v2 0/5] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
  2020-04-02 10:15     ` [PATCH v2 1/5] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-02 10:15     ` Đoàn Trần Công Danh
  2020-04-02 10:15     ` [PATCH v2 3/5] Documentation: document am --no-gpg-sign Đoàn Trần Công Danh
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-02 10:15 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/5] Documentation: document am --no-gpg-sign
  2020-04-02 10:15   ` [PATCH v2 0/5] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
  2020-04-02 10:15     ` [PATCH v2 1/5] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
  2020-04-02 10:15     ` [PATCH v2 2/5] cherry-pick/revert: honour --no-gpg-sign in all case Đoàn Trần Công Danh
@ 2020-04-02 10:15     ` Đoàn Trần Công Danh
  2020-04-02 10:15     ` [PATCH v2 4/5] Documentation: reword commit --no-gpg-sign Đoàn Trần Công Danh
  2020-04-02 10:15     ` [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign Đoàn Trần Công Danh
  4 siblings, 0 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-02 10:15 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 4/5] Documentation: reword commit --no-gpg-sign
  2020-04-02 10:15   ` [PATCH v2 0/5] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
                       ` (2 preceding siblings ...)
  2020-04-02 10:15     ` [PATCH v2 3/5] Documentation: document am --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-02 10:15     ` Đoàn Trần Công Danh
  2020-04-02 10:15     ` [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign Đoàn Trần Công Danh
  4 siblings, 0 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-02 10:15 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign
  2020-04-02 10:15   ` [PATCH v2 0/5] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
                       ` (3 preceding siblings ...)
  2020-04-02 10:15     ` [PATCH v2 4/5] Documentation: reword commit --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-02 10:15     ` Đoàn Trần Công Danh
  2020-04-02 18:07       ` Junio C Hamano
  4 siblings, 1 reply; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-02 10:15 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign
  2020-04-02 10:15     ` [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-02 18:07       ` Junio C Hamano
  2020-04-03  0:25         ` Danh Doan
  0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2020-04-02 18:07 UTC (permalink / raw)
  To: Đoàn Trần Công Danh; +Cc: git

Đ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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign
  2020-04-02 18:07       ` Junio C Hamano
@ 2020-04-03  0:25         ` Danh Doan
  0 siblings, 0 replies; 24+ messages in thread
From: Danh Doan @ 2020-04-03  0:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/5] rebase.c: honour --no-gpg-sign
  2020-04-02 10:15     ` [PATCH v2 1/5] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-03  5:21       ` Martin Ågren
  0 siblings, 0 replies; 24+ messages in thread
From: Martin Ågren @ 2020-04-03  5:21 UTC (permalink / raw)
  To: Đoàn Trần Công Danh; +Cc: Git Mailing List

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 0/6] Honour and Document --no-gpg-sign
  2020-03-31  6:44 ` [PATCH] rebase.c: teach --no-gpg-sign to git-rebase Danh Doan
  2020-04-01 17:47   ` Junio C Hamano
  2020-04-02 10:15   ` [PATCH v2 0/5] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-03 10:28   ` Đoàn Trần Công Danh
  2020-04-03 10:28     ` [PATCH v3 1/6] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
                       ` (7 more replies)
  2 siblings, 8 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-03 10:28 UTC (permalink / raw)
  To: git
  Cc: Đoàn Trần Công Danh, Dominic Chen,
	Junio C Hamano, Martin Ågren

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 1/6] rebase.c: honour --no-gpg-sign
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-03 10:28     ` Đoàn Trần Công Danh
  2020-04-03 10:28     ` [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case Đoàn Trần Công Danh
                       ` (6 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-03 10:28 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
  2020-04-03 10:28     ` [PATCH v3 1/6] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-03 10:28     ` Đoàn Trần Công Danh
  2020-04-03 13:43       ` Phillip Wood
  2020-04-03 10:28     ` [PATCH v3 3/6] Documentation: document am --no-gpg-sign Đoàn Trần Công Danh
                       ` (5 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-03 10:28 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 3/6] Documentation: document am --no-gpg-sign
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
  2020-04-03 10:28     ` [PATCH v3 1/6] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
  2020-04-03 10:28     ` [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case Đoàn Trần Công Danh
@ 2020-04-03 10:28     ` Đoàn Trần Công Danh
  2020-04-03 10:28     ` [PATCH v3 4/6] Documentation: reword commit --no-gpg-sign Đoàn Trần Công Danh
                       ` (4 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-03 10:28 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 4/6] Documentation: reword commit --no-gpg-sign
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
                       ` (2 preceding siblings ...)
  2020-04-03 10:28     ` [PATCH v3 3/6] Documentation: document am --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-03 10:28     ` Đoàn Trần Công Danh
  2020-04-03 10:28     ` [PATCH v3 5/6] Documentation: merge commit-tree --[no-]gpg-sign Đoàn Trần Công Danh
                       ` (3 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-03 10:28 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 5/6] Documentation: merge commit-tree --[no-]gpg-sign
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
                       ` (3 preceding siblings ...)
  2020-04-03 10:28     ` [PATCH v3 4/6] Documentation: reword commit --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-03 10:28     ` Đoàn Trần Công Danh
  2020-04-03 10:28     ` [PATCH v3 6/6] Documentation: document merge option --no-gpg-sign Đoàn Trần Công Danh
                       ` (2 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-03 10:28 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 6/6] Documentation: document merge option --no-gpg-sign
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
                       ` (4 preceding siblings ...)
  2020-04-03 10:28     ` [PATCH v3 5/6] Documentation: merge commit-tree --[no-]gpg-sign Đoàn Trần Công Danh
@ 2020-04-03 10:28     ` Đoàn Trần Công Danh
  2020-04-03 18:40     ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Junio C Hamano
  2020-04-04 14:36     ` Martin Ågren
  7 siblings, 0 replies; 24+ messages in thread
From: Đoàn Trần Công Danh @ 2020-04-03 10:28 UTC (permalink / raw)
  To: git; +Cc: Đ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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case
  2020-04-03 10:28     ` [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case Đoàn Trần Công Danh
@ 2020-04-03 13:43       ` Phillip Wood
  2020-04-03 14:26         ` Danh Doan
  0 siblings, 1 reply; 24+ messages in thread
From: Phillip Wood @ 2020-04-03 13:43 UTC (permalink / raw)
  To: Đoàn Trần Công Danh, git

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
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case
  2020-04-03 13:43       ` Phillip Wood
@ 2020-04-03 14:26         ` Danh Doan
  0 siblings, 0 replies; 24+ messages in thread
From: Danh Doan @ 2020-04-03 14:26 UTC (permalink / raw)
  To: phillip.wood; +Cc: git

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/6] Honour and Document --no-gpg-sign
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
                       ` (5 preceding siblings ...)
  2020-04-03 10:28     ` [PATCH v3 6/6] Documentation: document merge option --no-gpg-sign Đoàn Trần Công Danh
@ 2020-04-03 18:40     ` Junio C Hamano
  2020-04-04 14:36     ` Martin Ågren
  7 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2020-04-03 18:40 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: git, Dominic Chen, Martin Ågren

Đ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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/6] Honour and Document --no-gpg-sign
  2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
                       ` (6 preceding siblings ...)
  2020-04-03 18:40     ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Junio C Hamano
@ 2020-04-04 14:36     ` Martin Ågren
  7 siblings, 0 replies; 24+ messages in thread
From: Martin Ågren @ 2020-04-04 14:36 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Git Mailing List, Dominic Chen, Junio C Hamano

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2020-04-04 14:36 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 20:03 Support for `--no-gpg-sign` in git rebase, cherry-pick, etc Dominic Chen
2020-03-31  6:44 ` [PATCH] rebase.c: teach --no-gpg-sign to git-rebase Danh Doan
2020-04-01 17:47   ` Junio C Hamano
2020-04-02  1:09     ` Danh Doan
2020-04-02 10:15   ` [PATCH v2 0/5] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
2020-04-02 10:15     ` [PATCH v2 1/5] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
2020-04-03  5:21       ` Martin Ågren
2020-04-02 10:15     ` [PATCH v2 2/5] cherry-pick/revert: honour --no-gpg-sign in all case Đoàn Trần Công Danh
2020-04-02 10:15     ` [PATCH v2 3/5] Documentation: document am --no-gpg-sign Đoàn Trần Công Danh
2020-04-02 10:15     ` [PATCH v2 4/5] Documentation: reword commit --no-gpg-sign Đoàn Trần Công Danh
2020-04-02 10:15     ` [PATCH v2 5/5] Documentation: document merge option --no-gpg-sign Đoàn Trần Công Danh
2020-04-02 18:07       ` Junio C Hamano
2020-04-03  0:25         ` Danh Doan
2020-04-03 10:28   ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Đoàn Trần Công Danh
2020-04-03 10:28     ` [PATCH v3 1/6] rebase.c: honour --no-gpg-sign Đoàn Trần Công Danh
2020-04-03 10:28     ` [PATCH v3 2/6] cherry-pick/revert: honour --no-gpg-sign in all case Đoàn Trần Công Danh
2020-04-03 13:43       ` Phillip Wood
2020-04-03 14:26         ` Danh Doan
2020-04-03 10:28     ` [PATCH v3 3/6] Documentation: document am --no-gpg-sign Đoàn Trần Công Danh
2020-04-03 10:28     ` [PATCH v3 4/6] Documentation: reword commit --no-gpg-sign Đoàn Trần Công Danh
2020-04-03 10:28     ` [PATCH v3 5/6] Documentation: merge commit-tree --[no-]gpg-sign Đoàn Trần Công Danh
2020-04-03 10:28     ` [PATCH v3 6/6] Documentation: document merge option --no-gpg-sign Đoàn Trần Công Danh
2020-04-03 18:40     ` [PATCH v3 0/6] Honour and Document --no-gpg-sign Junio C Hamano
2020-04-04 14:36     ` Martin Ågren

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 NNTP newsgroup(s).