threads / patch / 53663

patchpull: passthrough --no-verify to merge

Subject: [PATCH] pull: passthrough --no-verify to merge

## tl;dr

2 messages between Jun 11, 2020 and Jun 11, 2020. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Azat Khuzhin· Jun 11, 2020, 17:22 UTC · lore
Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com>
---
 builtin/pull.c                                   |  6 ++++++
 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 12 ++++++++++++
 2 files changed, 18 insertions(+)
Show changes to 2 files +18 −0

builtin/pull.c, t/t7503-pre-commit-and-pre-merge-commit-hooks.sh

diff --git a/builtin/pull.c b/builtin/pull.c
index 00e5857a8d..de48cab325 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -83,6 +83,7 @@ static char *opt_commit;
 static char *opt_edit;
 static char *cleanup_arg;
 static char *opt_ff;
+static char *no_verify;
 static char *opt_verify_signatures;
 static int opt_autostash = -1;
 static int config_autostash;
@@ -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", &no_verify, NULL,
+		N_("bypass 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),
@@ -691,6 +695,8 @@ static int run_merge(void)
 		argv_array_push(&args, opt_ff);
 	if (opt_verify_signatures)
 		argv_array_push(&args, opt_verify_signatures);
+	if (no_verify)
+		argv_array_push(&args, no_verify);
 	argv_array_pushv(&args, opt_strategies.argv);
 	argv_array_pushv(&args, opt_strategy_opts.argv);
 	if (opt_gpg_sign)
diff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
index b3485450a2..be5dcfcc90 100755
--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
@@ -278,4 +278,16 @@ test_expect_success 'check the author in hook' '
 	test_cmp expected_hooks actual_hooks
 '
 
+test_expect_success '--no-verify with failing hook (pull)' '
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+	cp "$HOOKDIR/fail.sample" "$PREMERGE" &&
+	git remote add origin . &&
+	test_when_finished "git remote remove origin" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git pull --no-verify --no-edit . master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_done
-- 
2.27.0
Junio C Hamano· Jun 11, 2020, 17:37 UTC · re: Azat Khuzhin · lore

Re: [PATCH] pull: passthrough --no-verify to merge

Here is where you justify why applying this patch is a good idea for the project and its users. You left it blank.

Show 5 quoted lines
> Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com>
> ---
>  builtin/pull.c                                   |  6 ++++++
>  t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 12 ++++++++++++
>  2 files changed, 18 insertions(+)
I can sort of see the convenience value of doing:
  $ git pull --no-verify

when the remote is *known* to always have problematic changes that you'd rather prefer to skip the check, instead of having to do

  $ git pull
  $ git merge --no-verify FETCH_HEAD

to see if this time their new stuff passes the check and use the option only when they are unclean.

But I am not sure if it is a good idea to let the convenience trump the code hygiene in general.

In any case, even if we assume that the convenience value outweighs the downside of allowing people to become even more sloppy, we would need an update to the documentation, no?

Would "--no-verify" work well both in the rebase mode and in the merge mode, by the way? If yes, then that would be great. If it works only with one backend but not with the other, do we silently ignore, or do we detect incompatible options and error out? That needs to be designed, documented, and tested, too.

Thanks.

← back to recent threads