| Message ID | YXrFy9I1KPz3IZyp@pflmari (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove negation from the commit and merge option "--no-verify" | expand |
Alex Riesen <alexander.riesen@cetitec.com> writes: > Subject: Re: [PATCH 2/2] Fix "commit-msg" hook unexpectedly called for "git pull --no-verify" Perhaps Subject: [PATCH] pull: honor --no-verify and do not call the commit-msg hook instead. > From: Alex Riesen <raa.lkml@gmail.com> > > The option was incorrectly auto-translated to "--no-verify-signatures", > which causes the unexpected effect of the hook being called. > And an even more unexpected effect of disabling verification of signatures. > > The manual page describes the option to behave same as the similarly > named option of "git merge", which seems to be the original intention > of this option in the "pull" command. > > Signed-off-by: Alexander Riesen <raa.lkml@gmail.com> > --- > builtin/pull.c | 6 ++++++ > t/t5521-pull-options.sh | 24 ++++++++++++++++++++++++ > 2 files changed, 30 insertions(+) > > diff --git a/builtin/pull.c b/builtin/pull.c > index 425950f469..e783da10b2 100644 > --- a/builtin/pull.c > +++ b/builtin/pull.c > @@ -84,6 +84,7 @@ static char *opt_edit; > static char *cleanup_arg; > static char *opt_ff; > static char *opt_verify_signatures; > +static char *opt_verify; > static int opt_autostash = -1; > static int config_autostash; > static int check_trust_level = 1; > @@ -160,6 +161,9 @@ static struct option pull_options[] = { > OPT_PASSTHRU(0, "ff-only", &opt_ff, NULL, > N_("abort if fast-forward is not possible"), > PARSE_OPT_NOARG | PARSE_OPT_NONEG), > + OPT_PASSTHRU(0, "verify", &opt_verify, NULL, > + N_("control use of pre-merge-commit and commit-msg hooks"), > + PARSE_OPT_NOARG), > OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL, > N_("verify that the named commit has a valid GPG signature"), > PARSE_OPT_NOARG), > @@ -688,6 +692,8 @@ static int run_merge(void) > strvec_pushf(&args, "--cleanup=%s", cleanup_arg); > if (opt_ff) > strvec_push(&args, opt_ff); > + if (opt_verify) > + strvec_push(&args, opt_verify); > if (opt_verify_signatures) > strvec_push(&args, opt_verify_signatures); Looks quite straight-forward, especially that now this just mimicks how --[no-]verify-signatures is passed through. Thanks, will queue.
Junio C Hamano, Thu, Oct 28, 2021 18:51:17 +0200: > Alex Riesen <alexander.riesen@cetitec.com> writes: > > > Subject: Re: [PATCH 2/2] Fix "commit-msg" hook unexpectedly called for "git pull --no-verify" > > Perhaps > > Subject: [PATCH] pull: honor --no-verify and do not call the commit-msg hook > > instead. Looks fine from my side. Shall I resend? > Looks quite straight-forward, especially that now this just mimicks > how --[no-]verify-signatures is passed through. > > Thanks, will queue. Or did you queue it as is? Regards, Alex
Alex Riesen <alexander.riesen@cetitec.com> writes: > Junio C Hamano, Thu, Oct 28, 2021 18:51:17 +0200: >> Alex Riesen <alexander.riesen@cetitec.com> writes: >> >> > Subject: Re: [PATCH 2/2] Fix "commit-msg" hook unexpectedly called for "git pull --no-verify" >> >> Perhaps >> >> Subject: [PATCH] pull: honor --no-verify and do not call the commit-msg hook >> >> instead. > > Looks fine from my side. Shall I resend? If you are OK with the updated text, then I can locally amend.
Junio C Hamano, Thu, Oct 28, 2021 21:25:13 +0200: > Alex Riesen <alexander.riesen@cetitec.com> writes: > > > Junio C Hamano, Thu, Oct 28, 2021 18:51:17 +0200: > >> Alex Riesen <alexander.riesen@cetitec.com> writes: > >> > >> > Subject: Re: [PATCH 2/2] Fix "commit-msg" hook unexpectedly called for "git pull --no-verify" > >> > >> Perhaps > >> > >> Subject: [PATCH] pull: honor --no-verify and do not call the commit-msg hook > >> > >> instead. > > > > Looks fine from my side. Shall I resend? > > If you are OK with the updated text, then I can locally amend. Yes, of course! Thanks!
diff --git a/builtin/pull.c b/builtin/pull.c index 425950f469..e783da10b2 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -84,6 +84,7 @@ static char *opt_edit; static char *cleanup_arg; static char *opt_ff; static char *opt_verify_signatures; +static char *opt_verify; static int opt_autostash = -1; static int config_autostash; static int check_trust_level = 1; @@ -160,6 +161,9 @@ static struct option pull_options[] = { OPT_PASSTHRU(0, "ff-only", &opt_ff, NULL, N_("abort if fast-forward is not possible"), PARSE_OPT_NOARG | PARSE_OPT_NONEG), + OPT_PASSTHRU(0, "verify", &opt_verify, NULL, + N_("control use of pre-merge-commit and commit-msg hooks"), + PARSE_OPT_NOARG), OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL, N_("verify that the named commit has a valid GPG signature"), PARSE_OPT_NOARG), @@ -688,6 +692,8 @@ static int run_merge(void) strvec_pushf(&args, "--cleanup=%s", cleanup_arg); if (opt_ff) strvec_push(&args, opt_ff); + if (opt_verify) + strvec_push(&args, opt_verify); if (opt_verify_signatures) strvec_push(&args, opt_verify_signatures); strvec_pushv(&args, opt_strategies.v); diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index db1a381cd9..22cf1b2cf7 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -225,4 +225,28 @@ test_expect_success 'git pull --no-signoff flag cancels --signoff flag' ' test_must_be_empty actual ' +test_expect_success 'git pull --no-verify flag passed to merge' ' + test_when_finished "rm -fr src dst actual" && + git init src && + test_commit -C src one && + git clone src dst && + write_script dst/.git/hooks/commit-msg <<-\EOF && + false + EOF + test_commit -C src two && + git -C dst pull --no-ff --no-verify +' + +test_expect_success 'git pull --no-verify --verify passed to merge' ' + test_when_finished "rm -fr src dst actual" && + git init src && + test_commit -C src one && + git clone src dst && + write_script dst/.git/hooks/commit-msg <<-\EOF && + false + EOF + test_commit -C src two && + test_must_fail git -C dst pull --no-ff --no-verify --verify +' + test_done