{"thread":{"id":"54453","subject":"[PATCH v4 3/3] t3435: add tests for rebase -r GPG signing","startedAt":"2020-10-17T23:16:09Z","lastAt":"2020-10-17T23:16:11Z","messageCount":3,"participants":["Samuel Čavoj"],"isPatch":true,"patchVersion":4,"patchTotal":3},"messages":[{"id":"407819","messageId":"20201017231557.1001728-3-samuel@cavoj.net","threadId":"54453","inReplyTo":"20201017231557.1001728-1-samuel@cavoj.net","subject":"[PATCH v4 3/3] t3435: add tests for rebase -r GPG signing","fromName":"Samuel Čavoj","fromEmail":"samuel@cavoj.net","sentAt":"2020-10-17T23:15:57Z","receivedAt":"2020-10-17T23:16:09Z","isPatch":true,"sender":{"key":"samuel@cavoj.net","avatar":"https://avatars.githubusercontent.com/u/703905?v=4"},"body":"Add test cases of various combinations of the commit.gpgsign option and\n--gpg-sign, --no-gpg-sign flags with rebase -r with the default merge\nstrategy. This excercises a different code-path from those with octopus\nmerges or overridden merge strategy with rebase -s.\n\nSigned-off-by: Samuel Čavoj <samuel@cavoj.net>\n---\nchanged v3 -> v4:\n    * new\n---\n t/t3435-rebase-gpg-sign.sh | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh\nindex 2fba21f77a..54120b09d6 100755\n--- a/t/t3435-rebase-gpg-sign.sh\n+++ b/t/t3435-rebase-gpg-sign.sh\n@@ -96,4 +96,32 @@ test_expect_success \"rebase -r, merge strategy, commit.gpgsign=true --no-gpg-sig\n \ttest_must_fail git verify-commit HEAD\n '\n \n+test_expect_success 'rebase -r --gpg-sign will sign commit' '\n+\tgit reset --hard merged &&\n+\ttest_unconfig commit.gpgsign &&\n+\tgit rebase -fr --gpg-sign --root &&\n+\tgit verify-commit HEAD\n+'\n+\n+test_expect_success 'rebase -r with commit.gpgsign=true will sign commit' '\n+\tgit reset --hard merged &&\n+\tgit config commit.gpgsign true &&\n+\tgit rebase -fr --root &&\n+\tgit verify-commit HEAD\n+'\n+\n+test_expect_success 'rebase -r --gpg-sign with commit.gpgsign=false will sign commit' '\n+\tgit reset --hard merged &&\n+\tgit config commit.gpgsign false &&\n+\tgit rebase -fr --gpg-sign --root &&\n+\tgit verify-commit HEAD\n+'\n+\n+test_expect_success \"rebase -r --no-gpg-sign with commit.gpgsign=true won't sign commit\" '\n+\tgit reset --hard merged &&\n+\tgit config commit.gpgsign true &&\n+\tgit rebase -fr --no-gpg-sign --root &&\n+\ttest_must_fail git verify-commit HEAD\n+'\n+\n test_done\n-- \n2.28.0\n\n"},{"id":"407820","messageId":"20201017231557.1001728-2-samuel@cavoj.net","threadId":"54453","inReplyTo":"20201017231557.1001728-1-samuel@cavoj.net","subject":"[PATCH v4 2/3] sequencer: pass explicit --no-gpg-sign to merge","fromName":"Samuel Čavoj","fromEmail":"samuel@cavoj.net","sentAt":"2020-10-17T23:15:56Z","receivedAt":"2020-10-17T23:16:09Z","isPatch":true,"sender":{"key":"samuel@cavoj.net","avatar":"https://avatars.githubusercontent.com/u/703905?v=4"},"body":"The merge subcommand launched for merges with non-default strategy would\nuse its own default behaviour to decide how to sign commits, regardless\nof what opts->gpg_sign was set to. For example the --no-gpg-sign flag\ngiven to rebase explicitly would get ignored, if commit.gpgsign was set\nto true.\n\nFix the issue and add a test case excercising this behaviour.\n\nSigned-off-by: Samuel Čavoj <samuel@cavoj.net>\n---\n sequencer.c                | 2 ++\n t/t3435-rebase-gpg-sign.sh | 7 +++++++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 88ccff4838..043d606829 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3678,6 +3678,8 @@ static int do_merge(struct repository *r,\n \t\tstrvec_push(&cmd.args, git_path_merge_msg(r));\n \t\tif (opts->gpg_sign)\n \t\t\tstrvec_pushf(&cmd.args, \"-S%s\", opts->gpg_sign);\n+\t\telse\n+\t\t\tstrvec_push(&cmd.args, \"--no-gpg-sign\");\n \n \t\t/* Add the tips to be merged */\n \t\tfor (j = to_merge; j; j = j->next)\ndiff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh\nindex a2b7625707..2fba21f77a 100755\n--- a/t/t3435-rebase-gpg-sign.sh\n+++ b/t/t3435-rebase-gpg-sign.sh\n@@ -89,4 +89,11 @@ test_expect_success 'rebase -r, merge strategy, commit.gpgsign=false --gpg-sign\n \tgit verify-commit HEAD\n '\n \n+test_expect_success \"rebase -r, merge strategy, commit.gpgsign=true --no-gpg-sign won't sign commit\" '\n+\tgit reset --hard merged &&\n+\tgit config commit.gpgsign true &&\n+\tgit rebase -fr --no-gpg-sign -s resolve --root &&\n+\ttest_must_fail git verify-commit HEAD\n+'\n+\n test_done\n-- \n2.28.0\n\n"},{"id":"407821","messageId":"20201017231557.1001728-1-samuel@cavoj.net","threadId":"54453","inReplyTo":null,"subject":"[PATCH v4 1/3] sequencer: fix gpg option passed to merge subcommand","fromName":"Samuel Čavoj","fromEmail":"samuel@cavoj.net","sentAt":"2020-10-17T23:15:55Z","receivedAt":"2020-10-17T23:16:11Z","isPatch":true,"sender":{"key":"samuel@cavoj.net","avatar":"https://avatars.githubusercontent.com/u/703905?v=4"},"body":"When performing a rebase with --rebase-merges using either a custom\nstrategy specified with -s or an octopus merge, and at the same time\nhaving gpgsign enabled (either rebase -S or config commit.gpgsign), the\noperation would fail on making the merge commit. Instead of \"-S%s\" with\nthe key id substituted, only the bare key id would get passed to the\nunderlying merge command, which tried to interpret it as a ref.\n\nFix the issue and add test cases as suggested by Johannes Schindelin and\nJunio C Hamano.\n\nSigned-off-by: Samuel Čavoj <samuel@cavoj.net>\n---\nchanged v3 -> v4:\n    - added explicit test_unconfig to test case\n    - use git config instead of test_config\n    - added one more test case\n---\n sequencer.c                |  2 +-\n t/t3435-rebase-gpg-sign.sh | 21 +++++++++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 00acb12496..88ccff4838 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3677,7 +3677,7 @@ static int do_merge(struct repository *r,\n \t\tstrvec_push(&cmd.args, \"-F\");\n \t\tstrvec_push(&cmd.args, git_path_merge_msg(r));\n \t\tif (opts->gpg_sign)\n-\t\t\tstrvec_push(&cmd.args, opts->gpg_sign);\n+\t\t\tstrvec_pushf(&cmd.args, \"-S%s\", opts->gpg_sign);\n \n \t\t/* Add the tips to be merged */\n \t\tfor (j = to_merge; j; j = j->next)\ndiff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh\nindex b47c59c190..a2b7625707 100755\n--- a/t/t3435-rebase-gpg-sign.sh\n+++ b/t/t3435-rebase-gpg-sign.sh\n@@ -68,4 +68,25 @@ test_expect_failure 'rebase -p --no-gpg-sign override commit.gpgsign' '\n \ttest_must_fail git verify-commit HEAD\n '\n \n+test_expect_success 'rebase -r, merge strategy, --gpg-sign will sign commit' '\n+\tgit reset --hard merged &&\n+\ttest_unconfig commit.gpgsign &&\n+\tgit rebase -fr --gpg-sign -s resolve --root &&\n+\tgit verify-commit HEAD\n+'\n+\n+test_expect_success 'rebase -r, merge strategy, commit.gpgsign=true will sign commit' '\n+\tgit reset --hard merged &&\n+\tgit config commit.gpgsign true &&\n+\tgit rebase -fr -s resolve --root &&\n+\tgit verify-commit HEAD\n+'\n+\n+test_expect_success 'rebase -r, merge strategy, commit.gpgsign=false --gpg-sign will sign commit' '\n+\tgit reset --hard merged &&\n+\tgit config commit.gpgsign false &&\n+\tgit rebase -fr --gpg-sign -s resolve --root &&\n+\tgit verify-commit HEAD\n+'\n+\n test_done\n-- \n2.28.0\n\n"}]}