Re: [PATCH v1 03/19] rebase -i: reword executes pre-commit hook on interim commit

On Tue, Jul 29, 2014 at 01:18:03AM +0200, Fabian Ruch wrote:

quoted

Specify the git-commit option `--no-verify` to disable the pre-commit
hook when editing the log message. Because `--no-verify` also skips
the commit-msg hook, execute the hook from within
git-rebase--interactive after the commit is created. Fortunately, the
commit message is still available in `$GIT_DIR/COMMIT_EDITMSG` after
git-commit terminates. Caveat: In case the commit-msg hook finds the
new log message ill-formatted, the user is only notified of the
failed commit-msg hook but the log message is used for the commit
anyway. git-commit ought to offer more fine-grained control over
which hooks are executed.

Thanks for a nice explanation of the tradeoff. Have you looked at adding
an option to git-commit? We already have --no-post-rewrite. I think you
would just need:

diff --git a/builtin/commit.c b/builtin/commit.c
index 5ed6036..f7af220 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -102,6 +102,7 @@ static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int no_post_rewrite, allow_empty_message;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
 static char *sign_commit;
+static int no_pre_commit;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -661,7 +662,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	/* This checks and barfs if author is badly specified */
 	determine_author_info(author_ident);
 
-	if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
+	if (!no_verify && !no_pre_commit &&
+	    run_commit_hook(use_editor, index_file, "pre-commit", NULL))
 		return 0;
 
 	if (squash_message) {

@@ -1604,6 +1606,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 			 N_("terminate entries with NUL")),
 		OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
 		OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
+		OPT_BOOL(0, "no-pre-commit", &no_pre_commit, N_("bypass pre-commit hook")),
 		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, N_("mode"), N_("show untracked files, optional modes: all, normal, no. (Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
 		/* end commit contents options */
 

though I would also not be opposed to some more uniform hook selection
mechanism (e.g., "--no-verify=pre-commit" or something).

diff --git a/builtin/commit.c b/builtin/commit.c
index 5e2221c..813aa78 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -98,12 +98,27 @@ static char *edit_message, *use_message;
 static char *fixup_message, *squash_message;
 static int all, also, interactive, patch_interactive, only, amend, signoff;
 static int edit_flag = -1; /* unspecified */
-static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
+static int quiet, verbose, allow_empty, dry_run, renew_authorship;
 static int no_post_rewrite, allow_empty_message;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
 static char *sign_commit;
 
 /*
+ * The verify variable is interpreted as a bitmap of enabled commit
+ * verification hooks according to the legend below.
+ *
+ * By default, the pre-commit and commit-msg hooks are enabled. This
+ * is represented by both the PRE_COMMIT and COMMIT_MSG bits being
+ * set.
+ *
+ * The bitmap is changed through the command line options
+ * --no-verify, --no-pre-commit and --no-commit-msg.
+ */
+#define PRE_COMMIT (1<<0)
+#define COMMIT_MSG (1<<1)
+static int verify = PRE_COMMIT | COMMIT_MSG;
+
+/*
  * The default commit message cleanup mode will remove the lines
  * beginning with # (shell comments) and leading and trailing
  * whitespaces (empty lines or containing only whitespaces)

@@ -667,7 +682,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	/* This checks and barfs if author is badly specified */
 	determine_author_info(author_ident);
 
-	if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
+	if (verify & PRE_COMMIT &&
+	    run_commit_hook(use_editor, index_file, "pre-commit", NULL))
 		return 0;
 
 	if (squash_message) {

@@ -968,7 +984,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		}
 	}
 
-	if (!no_verify &&
+	if (verify & COMMIT_MSG &&
 	    run_commit_hook(use_editor, index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
 		return 0;
 	}

@@ -1597,7 +1613,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "interactive", &interactive, N_("interactively add files")),
 		OPT_BOOL('p', "patch", &patch_interactive, N_("interactively add changes")),
 		OPT_BOOL('o', "only", &only, N_("commit only specified files")),
-		OPT_BOOL('n', "no-verify", &no_verify, N_("bypass pre-commit hook")),
+		OPT_NEGBIT('n', "no-verify", &verify,
+			   N_("synonym for --no-pre-commit --no-commit-msg"),
+			   PRE_COMMIT | COMMIT_MSG),
 		OPT_BOOL(0, "dry-run", &dry_run, N_("show what would be committed")),
 		OPT_SET_INT(0, "short", &status_format, N_("show status concisely"),
 			    STATUS_FORMAT_SHORT),

@@ -1610,6 +1628,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('z', "null", &s.null_termination,
 			 N_("terminate entries with NUL")),
 		OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
+		OPT_NEGBIT(0, "no-pre-commit", &verify,
+			   N_("bypass pre-commit hook"),
+			   PRE_COMMIT),
+		OPT_NEGBIT(0, "no-commit-msg", &verify,
+			   N_("bypass commit-msg hook"),
+			   COMMIT_MSG),
 		OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
 		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, N_("mode"), N_("show untracked files, optional modes: all, normal, no. (Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
 		/* end commit contents options */