{"thread":{"id":"53663","subject":"[PATCH] pull: passthrough --no-verify to merge","startedAt":"2020-06-11T17:22:14Z","lastAt":"2020-06-11T17:38:07Z","messageCount":2,"participants":["Azat Khuzhin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"399553","messageId":"20200611172201.962109-1-a3at.mail@gmail.com","threadId":"53663","inReplyTo":null,"subject":"[PATCH] pull: passthrough --no-verify to merge","fromName":"Azat Khuzhin","fromEmail":"a3at.mail@gmail.com","sentAt":"2020-06-11T17:22:01Z","receivedAt":"2020-06-11T17:22:14Z","isPatch":true,"sender":{"key":"a3at.mail@gmail.com","avatar":null},"body":"Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com>\n---\n builtin/pull.c                                   |  6 ++++++\n t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 12 ++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 00e5857a8d..de48cab325 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -83,6 +83,7 @@ static char *opt_commit;\n static char *opt_edit;\n static char *cleanup_arg;\n static char *opt_ff;\n+static char *no_verify;\n static char *opt_verify_signatures;\n static int opt_autostash = -1;\n static int config_autostash;\n@@ -160,6 +161,9 @@ static struct option pull_options[] = {\n \tOPT_PASSTHRU(0, \"ff-only\", &opt_ff, NULL,\n \t\tN_(\"abort if fast-forward is not possible\"),\n \t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG),\n+\tOPT_PASSTHRU(0, \"no-verify\", &no_verify, NULL,\n+\t\tN_(\"bypass pre-merge-commit and commit-msg hooks\"),\n+\t\tPARSE_OPT_NOARG),\n \tOPT_PASSTHRU(0, \"verify-signatures\", &opt_verify_signatures, NULL,\n \t\tN_(\"verify that the named commit has a valid GPG signature\"),\n \t\tPARSE_OPT_NOARG),\n@@ -691,6 +695,8 @@ static int run_merge(void)\n \t\targv_array_push(&args, opt_ff);\n \tif (opt_verify_signatures)\n \t\targv_array_push(&args, opt_verify_signatures);\n+\tif (no_verify)\n+\t\targv_array_push(&args, no_verify);\n \targv_array_pushv(&args, opt_strategies.argv);\n \targv_array_pushv(&args, opt_strategy_opts.argv);\n \tif (opt_gpg_sign)\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex b3485450a2..be5dcfcc90 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -278,4 +278,16 @@ test_expect_success 'check the author in hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success '--no-verify with failing hook (pull)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/fail.sample\" \"$PREMERGE\" &&\n+\tgit remote add origin . &&\n+\ttest_when_finished \"git remote remove origin\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit pull --no-verify --no-edit . master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_done\n-- \n2.27.0\n\n"},{"id":"399554","messageId":"CAPc5daXjJY42Ou-P71G6Tmo-Qgf2CeLT9dCiw63urg2cZ1eiNw@mail.gmail.com","threadId":"53663","inReplyTo":"20200611172201.962109-1-a3at.mail@gmail.com","subject":"Re: [PATCH] pull: passthrough --no-verify to merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-11T17:37:51Z","receivedAt":"2020-06-11T17:38:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is where you justify why applying this patch is a good idea\nfor the project and its users. You left it blank.\n\n> Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com>\n> ---\n>  builtin/pull.c                                   |  6 ++++++\n>  t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 12 ++++++++++++\n>  2 files changed, 18 insertions(+)\n\nI can sort of see the convenience value of doing:\n\n  $ git pull --no-verify\n\nwhen the remote is *known* to always have problematic changes\nthat you'd rather prefer to skip the check, instead of having to do\n\n  $ git pull\n  $ git merge --no-verify FETCH_HEAD\n\nto see if this time their new stuff passes the check and use the\noption only when they are unclean.\n\nBut I am not sure if it is a good idea to let the convenience\ntrump the code hygiene in general.\n\nIn any case, even if we assume that the convenience value\noutweighs the downside of allowing people to become even more\nsloppy, we would need an update to the documentation, no?\n\nWould \"--no-verify\" work well both in the rebase mode and in\nthe merge mode, by the way? If yes, then that would be great.\nIf it works only with one backend but not with the other, do we\nsilently ignore, or do we detect incompatible options and\nerror out? That needs to be designed, documented, and\ntested, too.\n\nThanks.\n"}]}