[PATCH v2] pull: warn on --verify-signatures with --rebase
- From
Alexander 'z33ky' Hirsch <1zeeky@gmail.com>
- Date
- May 20, 2016, 21:00 UTC
- Message-ID
- <20160520210054.GA1856@netblarch.fritz.box>
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