diff mbox series

[v2] Fix "commit-msg" hook unexpectedly called for "git pull --no-verify"

Message ID YXjzGzcXS/zPgk0W@pflmari (mailing list archive)
State New, archived
Headers show
Series [v2] Fix "commit-msg" hook unexpectedly called for "git pull --no-verify" | expand

Commit Message

Alex Riesen Oct. 27, 2021, 6:35 a.m. UTC 
The option is incorrectly 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>
---

Jeff King, Tue, Oct 26, 2021 23:16:09 +0200:
> On Tue, Oct 26, 2021 at 02:11:22PM +0200, Alex Riesen wrote:
> > +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 &&
> > +	echo false >dst/.git/hooks/commit-msg &&
> > +	chmod +x dst/.git/hooks/commit-msg &&
> 
> This script without #! should work portably, I think, though we
> generally prefer using the helper (which also handles the chmod):
> 
>   write_script dst/.git/hooks/commit-msg <<-\EOF
>   false
>   EOF
> 
> Other than that nit, this looks good to me.

Updated. Certainly looks nicer.

 builtin/pull.c          |  6 ++++++
 t/t5521-pull-options.sh | 12 ++++++++++++
 2 files changed, 18 insertions(+)

Comments

Jeff King Oct. 27, 2021, 9:06 a.m. UTC | #1 
On Wed, Oct 27, 2021 at 08:35:07AM +0200, Alex Riesen wrote:

> The option is incorrectly 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.

Thanks, this looks good to me.

-Peff
diff mbox series

Patch

diff --git a/builtin/pull.c b/builtin/pull.c
index 425950f469..428baea95b 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_no_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, "no-verify", &opt_no_verify, NULL,
+		N_("bypass pre-merge-commit and commit-msg hooks"),
+		PARSE_OPT_NOARG | PARSE_OPT_NONEG),
 	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_no_verify)
+		strvec_push(&args, opt_no_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..7d3a8ae0d3 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -225,4 +225,16 @@  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_done