| Message ID | 28ebbfe72a04b787fb92702199efea663a6b7ee5.1585909453.git.congdanhqx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Honour and Document --no-gpg-sign | expand |
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 >
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.
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
{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