[PATCH v2] pull: warn on --verify-signatures with --rebase

[PATCH v2] pull: warn on --verify-signatures with --rebase

[Alexander 'z33ky' Hirsch]

Previously git-pull silently ignored the --verify-signatures option for
--rebase, potentially leaving users in the believe that the rebase
operation would check for valid GPG signatures.

Implementing --verify-signatures for git-rebase was talked about, but
doubts for a valid workflow rose up.
Since you usually merge other's branches into your branch you might have
an interest that their side has a valid GPG signature.
Rebasing, on the other hand, is you building something on top of
another's branch, essentially giving you the power to keep things sane.
If a valid use case is found, where --verify-signatures for git-rebase
looks sensible, that feature may be added then.

A warning was chosen in favor of emitting an error to prevent potential
breakage. A warning keeps things running, scripts for instance, while
still informing users about possibly unexpected behavior.

Signed-off-by: Alexander 'z33ky' Hirsch <1zeeky@gmail.com>
---

The warning message was changed to make it clear that the pull (and
rebase) operation still proceeds.

And the commit message was amended with more reasoning about the change
and why alternative approaches were not used.

 builtin/pull.c  |  3 +++
 t/t5520-pull.sh | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 1d7333c..b03bc39 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -815,6 +815,9 @@ static int run_rebase(const unsigned char *curr_head,
 		argv_array_push(&args, "--no-autostash");
 	else if (opt_autostash == 1)
 		argv_array_push(&args, "--autostash");
+	if (opt_verify_signatures &&
+		strcmp(opt_verify_signatures, "--verify-signatures") == 0)
+		warning(_("ignoring --verify-signatures for rebase"));
 
 	argv_array_push(&args, "--onto");
 	argv_array_push(&args, sha1_to_hex(merge_head));
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 739c089..3159956 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -341,6 +341,22 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
 	test new = "$(git show HEAD:file2)"
 '
 
+test_expect_success "pull --rebase warns on --verify-signatures" '
+	git reset --hard before-rebase &&
+	git pull --rebase --verify-signatures . copy 2>err &&
+	test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
+	test new = "$(git show HEAD:file2)" &&
+	test_i18ngrep "ignoring --verify-signatures for rebase" err
+'
+
+test_expect_success "pull --rebase does not warn on --no-verify-signatures" '
+	git reset --hard before-rebase &&
+	git pull --rebase --no-verify-signatures . copy 2>err &&
+	test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
+	test new = "$(git show HEAD:file2)" &&
+	test_i18ngrep ! "verify-signatures" err
+'
+
 # add a feature branch, keep-merge, that is merged into master, so the
 # test can try preserving the merge commit (or not) with various
 # --rebase flags/pull.rebase settings.
-- 
2.8.2

Re: [PATCH v2] pull: warn on --verify-signatures with --rebase

[Junio C Hamano]

Thanks.