git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 01/17] commit: use config-based hooks

From
Emily Shaffer <emilyshaffer@google.com>
Date
Dec 5, 2020, 01:49 UTC
Message-ID
<20201205014945.1502660-2-emilyshaffer@google.com>
In-Reply-To
<20201205014945.1502660-1-emilyshaffer@google.com>

As part of the adoption of config-based hooks, teach run_commit_hook() to call hook.h instead of run-command.h. This covers 'pre-commit', 'commit-msg', and 'prepare-commit-msg'. Additionally, ask the hook library - not run-command - whether any hooks will be run, as it's possible hooks may exist in the config but not the hookdir.

Because all but 'post-commit' hooks are expected to make some state change, force all but 'post-commit' hook to run in series. 'post-commit' "is meant primarily for notification, and cannot affect the outcome of `git commit`," so it is fine to run in parallel.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/githooks.txt                    | 10 ++++++++++
 builtin/commit.c                              | 11 +++++-----
 builtin/merge.c                               |  9 +++++----
 commit.c                                      | 20 +++++++++++++------
 commit.h                                      |  3 ++-
 sequencer.c                                   |  7 ++++---
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 17 ++++++++++++++--
 7 files changed, 56 insertions(+), 21 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index ffccfc7760..8b352be43f 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -103,6 +103,8 @@ The default 'pre-commit' hook, when enabled--and with the
 `hooks.allownonascii` config option unset or set to false--prevents
 the use of non-ASCII filenames.
 
+Hooks executed during 'pre-commit' will not be parallelized.
+
 pre-merge-commit
 ~~~~~~~~~~~~~~~~
 
@@ -125,6 +127,8 @@ need to be resolved and the result committed separately (see
 linkgit:git-merge[1]). At that point, this hook will not be executed,
 but the 'pre-commit' hook will, if it is enabled.
 
+Hooks executed during 'pre-merge-commit' will not be parallelized.
+
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~
 
@@ -150,6 +154,8 @@ be used as replacement for pre-commit hook.
 The sample `prepare-commit-msg` hook that comes with Git removes the
 help message found in the commented portion of the commit template.
 
+Hooks executed during 'prepare-commit-msg' will not be parallelized.
+
 commit-msg
 ~~~~~~~~~~
 
@@ -166,6 +172,8 @@ file.
 The default 'commit-msg' hook, when enabled, detects duplicate
 `Signed-off-by` trailers, and aborts the commit if one is found.
 
+Hooks executed during 'commit-msg' will not be parallelized.
+
 post-commit
 ~~~~~~~~~~~
 
@@ -175,6 +183,8 @@ invoked after a commit is made.
 This hook is meant primarily for notification, and cannot affect
 the outcome of `git commit`.
 
+Hooks executed during 'post-commit' will run in parallel by default.
+
 pre-rebase
 ~~~~~~~~~~
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 505fe60956..f4dea2b510 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -36,6 +36,7 @@
 #include "help.h"
 #include "commit-reach.h"
 #include "commit-graph.h"
+#include "hook.h"
 
 static const char * const builtin_commit_usage[] = {
 	N_("git commit [<options>] [--] <pathspec>..."),
@@ -699,7 +700,7 @@ 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 && run_commit_hook(use_editor, 0, index_file, "pre-commit", NULL))
 		return 0;
 
 	if (squash_message) {
@@ -983,7 +984,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		return 0;
 	}
 
-	if (!no_verify && find_hook("pre-commit")) {
+	if (!no_verify && hook_exists("pre-commit", configured_hookdir_opt())) {
 		/*
 		 * Re-read the index as pre-commit hook could have updated it,
 		 * and write it out as a tree.  We must do this before we invoke
@@ -998,7 +999,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		return 0;
 	}
 
-	if (run_commit_hook(use_editor, index_file, "prepare-commit-msg",
+	if (run_commit_hook(use_editor, 0, index_file, "prepare-commit-msg",
 			    git_path_commit_editmsg(), hook_arg1, hook_arg2, NULL))
 		return 0;
 
@@ -1015,7 +1016,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 
 	if (!no_verify &&
-	    run_commit_hook(use_editor, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) {
+	    run_commit_hook(use_editor, 0, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) {
 		return 0;
 	}
 
@@ -1701,7 +1702,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	repo_rerere(the_repository, 0);
 	run_auto_maintenance(quiet);
-	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
+	run_commit_hook(use_editor, 1, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
 		commit_post_rewrite(the_repository, current_head, &oid);
 	}
diff --git a/builtin/merge.c b/builtin/merge.c
index 1cff730715..d654b6923c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -42,6 +42,7 @@
 #include "commit-reach.h"
 #include "wt-status.h"
 #include "commit-graph.h"
+#include "hook.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -836,14 +837,14 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	struct strbuf msg = STRBUF_INIT;
 	const char *index_file = get_index_file();
 
-	if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
+	if (!no_verify && run_commit_hook(0 < option_edit, 0, index_file, "pre-merge-commit", NULL))
 		abort_commit(remoteheads, NULL);
 	/*
 	 * Re-read the index as pre-merge-commit hook could have updated it,
 	 * and write it out as a tree.  We must do this before we invoke
 	 * the editor and after we invoke run_status above.
 	 */
-	if (find_hook("pre-merge-commit"))
+	if (hook_exists("pre-merge-commit", configured_hookdir_opt()))
 		discard_cache();
 	read_cache_from(index_file);
 	strbuf_addbuf(&msg, &merge_msg);
@@ -864,7 +865,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
 	write_merge_heads(remoteheads);
 	write_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);
-	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
+	if (run_commit_hook(0 < option_edit, 0, get_index_file(), "prepare-commit-msg",
 			    git_path_merge_msg(the_repository), "merge", NULL))
 		abort_commit(remoteheads, NULL);
 	if (0 < option_edit) {
@@ -872,7 +873,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 			abort_commit(remoteheads, NULL);
 	}
 
-	if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),
+	if (!no_verify && run_commit_hook(0 < option_edit, 0, get_index_file(),
 					  "commit-msg",
 					  git_path_merge_msg(the_repository), NULL))
 		abort_commit(remoteheads, NULL);
diff --git a/commit.c b/commit.c
index fe1fa3dc41..1bad721a20 100644
--- a/commit.c
+++ b/commit.c
@@ -21,6 +21,7 @@
 #include "commit-reach.h"
 #include "run-command.h"
 #include "shallow.h"
+#include "hook.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
 
@@ -1630,25 +1631,32 @@ size_t ignore_non_trailer(const char *buf, size_t len)
 	return boc ? len - boc : len - cutoff;
 }
 
-int run_commit_hook(int editor_is_used, const char *index_file,
+int run_commit_hook(int editor_is_used, int parallelize, const char *index_file,
 		    const char *name, ...)
 {
-	struct strvec hook_env = STRVEC_INIT;
+	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_SYNC;
 	va_list args;
+	const char *arg;
 	int ret;
 
-	strvec_pushf(&hook_env, "GIT_INDEX_FILE=%s", index_file);
+	if (parallelize)
+		opt.jobs = configured_hook_jobs();
+
+	strvec_pushf(&opt.env, "GIT_INDEX_FILE=%s", index_file);
 
 	/*
 	 * Let the hook know that no editor will be launched.
 	 */
 	if (!editor_is_used)
-		strvec_push(&hook_env, "GIT_EDITOR=:");
+		strvec_push(&opt.env, "GIT_EDITOR=:");
 
 	va_start(args, name);
-	ret = run_hook_ve(hook_env.v, name, args);
+	while ((arg = va_arg(args, const char *)))
+		strvec_push(&opt.args, arg);
 	va_end(args);
-	strvec_clear(&hook_env);
+
+	ret = run_hooks(name, &opt);
+	run_hooks_opt_clear(&opt);
 
 	return ret;
 }
diff --git a/commit.h b/commit.h
index 5467786c7b..dd33ead8e0 100644
--- a/commit.h
+++ b/commit.h
@@ -352,6 +352,7 @@ int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused)
 int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused);
 
 LAST_ARG_MUST_BE_NULL
-int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);
+int run_commit_hook(int editor_is_used, int parallelize, const char *index_file,
+		    const char *name, ...);
 
 #endif /* COMMIT_H */
diff --git a/sequencer.c b/sequencer.c
index 8909a46770..5a98fd2fbc 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -34,6 +34,7 @@
 #include "commit-reach.h"
 #include "rebase-interactive.h"
 #include "reset.h"
+#include "hook.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -1203,7 +1204,7 @@ static int run_prepare_commit_msg_hook(struct repository *r,
 	} else {
 		arg1 = "message";
 	}
-	if (run_commit_hook(0, r->index_file, "prepare-commit-msg", name,
+	if (run_commit_hook(0, 0, r->index_file, "prepare-commit-msg", name,
 			    arg1, arg2, NULL))
 		ret = error(_("'prepare-commit-msg' hook failed"));
 
@@ -1438,7 +1439,7 @@ static int try_to_commit(struct repository *r,
 		}
 	}
 
-	if (find_hook("prepare-commit-msg")) {
+	if (hook_exists("prepare-commit-msg", configured_hookdir_opt())) {
 		res = run_prepare_commit_msg_hook(r, msg, hook_commit);
 		if (res)
 			goto out;
@@ -1528,7 +1529,7 @@ static int try_to_commit(struct repository *r,
 		goto out;
 	}
 
-	run_commit_hook(0, r->index_file, "post-commit", NULL);
+	run_commit_hook(0, 1, r->index_file, "post-commit", NULL);
 	if (flags & AMEND_MSG)
 		commit_post_rewrite(r, current_head, oid);
 
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..fc93bc3d23 100755
--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
@@ -5,8 +5,8 @@ test_description='pre-commit and pre-merge-commit hooks'
 . ./test-lib.sh
 
 HOOKDIR="$(git rev-parse --git-dir)/hooks"
-PRECOMMIT="$HOOKDIR/pre-commit"
-PREMERGE="$HOOKDIR/pre-merge-commit"
+PRECOMMIT="$(pwd)/$HOOKDIR/pre-commit"
+PREMERGE="$(pwd)/$HOOKDIR/pre-merge-commit"
 
 # Prepare sample scripts that write their $0 to actual_hooks
 test_expect_success 'sample script setup' '
@@ -103,6 +103,19 @@ test_expect_success 'with succeeding hook' '
 	test_cmp expected_hooks actual_hooks
 '
 
+# NEEDSWORK: when 'git hook add' and 'git hook remove' have been added, use that
+# instead
+test_expect_success 'with succeeding hook (config-based)' '
+	test_when_finished "git config --unset hook.pre-commit.command success.sample" &&
+	test_when_finished "rm -f expected_hooks actual_hooks" &&
+	git config hook.pre-commit.command "$HOOKDIR/success.sample" &&
+	echo "$HOOKDIR/success.sample" >expected_hooks &&
+	echo "more" >>file &&
+	git add file &&
+	git commit -m "more" &&
+	test_cmp expected_hooks actual_hooks
+'
+
 test_expect_success 'with succeeding hook (merge)' '
 	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
 	cp "$HOOKDIR/success.sample" "$PREMERGE" &&
-- 
2.28.0.rc0.142.g3c755180ce-goog
Previous: Emily ShafferNext: Emily Shaffer
Message 4 of 53 in “commit: use config-based hooks (config-based hooks part II)”
  1. commit: use config-based hooks (config-based hooks part II)Emily Shaffer, Oct 14, 2020
  2. Junio C HamanoOct 16, 2020
  3. 00/17 use config-based hooks (config-based hooks partEmily Shaffer, Dec 5, 2020
  4. 01/17 commit: use config-based hooksEmily Shaffer, Dec 5, 2020
  5. 03/17 merge: use config-based hooks for post-merge hookEmily Shaffer, Dec 5, 2020
  6. 02/17 am: convert applypatch hooks to use configEmily Shaffer, Dec 5, 2020
  7. 05/17 rebase: teach pre-rebase to use hook.hEmily Shaffer, Dec 5, 2020
  8. 04/17 gc: use hook library for pre-auto-gc hookEmily Shaffer, Dec 5, 2020
  9. 15/17 post-update: use hook.h libraryEmily Shaffer, Dec 5, 2020
  10. 06/17 read-cache: convert post-index-change hook to use configEmily Shaffer, Dec 5, 2020
  11. 07/17 receive-pack: convert push-to-checkout hook to hook.hEmily Shaffer, Dec 5, 2020
  12. 14/17 proc-receive: acquire hook list from hook.hEmily Shaffer, Dec 5, 2020
  13. 09/17 hooks: convert 'post-checkout' hook to hook libraryEmily Shaffer, Dec 5, 2020
  14. 12/17 reference-transaction: look for hooks in configEmily Shaffer, Dec 5, 2020
  15. 11/17 transport: convert pre-push hook to use configEmily Shaffer, Dec 5, 2020
  16. 08/17 git-p4: use 'git hook' to run hooksEmily Shaffer, Dec 5, 2020
  17. Josh SteadmonDec 16, 2020
  18. Emily ShafferDec 16, 2020
  19. 10/17 hook: convert 'post-rewrite' hook to configEmily Shaffer, Dec 5, 2020
  20. Josh SteadmonDec 8, 2020
  21. Emily ShafferDec 15, 2020
  22. 13/17 receive-pack: convert 'update' hook to hook.hEmily Shaffer, Dec 5, 2020
  23. 16/17 receive-pack: convert receive hooks to hook.hEmily Shaffer, Dec 5, 2020
  24. 17/17 run-command: stop thinking about hooksEmily Shaffer, Dec 5, 2020
  25. 00/17 use config-based hooks (config-based hooks part II)Emily Shaffer, Dec 22, 2020
  26. 04/17 gc: use hook library for pre-auto-gc hookEmily Shaffer, Dec 22, 2020
  27. 01/17 commit: use config-based hooksEmily Shaffer, Dec 22, 2020
  28. Junio C HamanoFeb 1, 2021
  29. Emily ShafferMar 10, 2021
  30. Junio C HamanoMar 10, 2021
  31. Junio C HamanoFeb 1, 2021
  32. Emily ShafferMar 10, 2021
  33. 03/17 merge: use config-based hooks for post-merge hookEmily Shaffer, Dec 22, 2020
  34. 06/17 read-cache: convert post-index-change hook to use configEmily Shaffer, Dec 22, 2020
  35. 02/17 am: convert applypatch hooks to use configEmily Shaffer, Dec 22, 2020
  36. Junio C HamanoFeb 1, 2021
  37. 05/17 rebase: teach pre-rebase to use hook.hEmily Shaffer, Dec 22, 2020
  38. 07/17 receive-pack: convert push-to-checkout hook to hook.hEmily Shaffer, Dec 22, 2020
  39. 09/17 hooks: convert 'post-checkout' hook to hook libraryEmily Shaffer, Dec 22, 2020
  40. 08/17 git-p4: use 'git hook' to run hooksEmily Shaffer, Dec 22, 2020
  41. 13/17 receive-pack: convert 'update' hook to hook.hEmily Shaffer, Dec 22, 2020
  42. 14/17 proc-receive: acquire hook list from hook.hEmily Shaffer, Dec 22, 2020
  43. 10/17 hook: convert 'post-rewrite' hook to configEmily Shaffer, Dec 22, 2020
  44. 16/17 receive-pack: convert receive hooks to hook.hEmily Shaffer, Dec 22, 2020
  45. 12/17 reference-transaction: look for hooks in configEmily Shaffer, Dec 22, 2020
  46. 17/17 run-command: stop thinking about hooksEmily Shaffer, Dec 22, 2020
  47. 15/17 post-update: use hook.h libraryEmily Shaffer, Dec 22, 2020
  48. 11/17 transport: convert pre-push hook to use configEmily Shaffer, Dec 22, 2020
  49. Emily ShafferDec 28, 2020
  50. 18/17 doc: make git-hook.txt point of truthEmily Shaffer, Dec 28, 2020
  51. Emily ShafferDec 28, 2020
  52. Josh SteadmonFeb 18, 2021
  53. Josh SteadmonDec 16, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.