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

[PATCH v3 14/17] proc-receive: acquire hook list from hook.h

From
Emily Shaffer <emilyshaffer@google.com>
Date
Dec 22, 2020, 00:04 UTC
Message-ID
<20201222000435.1529768-15-emilyshaffer@google.com>
In-Reply-To
<20201222000435.1529768-1-emilyshaffer@google.com>

The proc-receive hook differs from most other hooks Git invokes because the hook and the parent Git process engage in bidirectional communication via stdin/stdout. This bidirectional communication is unsuitable for multiple hooks, whether they are in series or in parallel, and is incompatible with run-command.h:run_processes_parallel:

- The proc-receive hook is intended to modify the state of the Git repo.
  From 'git help githooks':
    This [proc-receive] hook is responsible for updating the relevant
    references and reporting the results back to 'receive-pack'.
  This prevents parallelization and implies, at least, specific ordering
  of hook execution.
- The proc-receive hook can reject a push by aborting early with an
  error code. If a former hook ran through the entire push contents
  successfully but a later hook rejects some of the push, the repo may
  be left in a partially-updated (and corrupt) state.
- The callback model of the run_processes_parallel() API is unsuited to
  the current implementation of proc-receive, which loops through
  "send-receive-consider" with the child process. proc-receive today
  relies on stateful communication with the child process, which would be
  unwieldy to implement with callbacks and saved state.
- Additionally, run_processes_parallel() is designed to collate the
  output of many child processes into a single output (stderr or callback),
  and would require significant work to tell the caller which process sent
  the output, and indeed to collect any output before the child process
  has exited.

So, rather than using hook.h:run_hooks() to invoke the proc-receive hook, receive-pack.c can learn to ask hook.h:hook_list() for the location of a hook to run. This allows users to configure their proc-receive in a global config for all repos if they want, or a local config if they just don't want to use the hookdir. Because running more than one proc-receive hook doesn't make sense from a repo state perspective, we can explicitly ban configuring more than one proc-receive hook at a time.

If a user wants to globally configure one proc-receive hook for most of their repos, but override that hook in a single repo, they should use 'skip' to manually remove the global hook in their special repo:

~/.gitconfig:
[hook.proc-receive]
  command = /usr/bin/usual-proc-receive
~/special-repo/.git/config:
[hookcmd./usr/bin/usual-proc-receive]
  skip = true
[hook.proc-receive]
  command = /usr/bin/special-proc-receive
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/githooks.txt                |  4 ++
 builtin/receive-pack.c                    | 33 +++++++++++++++-
 t/t5411/test-0015-too-many-hooks-error.sh | 47 +++++++++++++++++++++++
 3 files changed, 82 insertions(+), 2 deletions(-)
 create mode 100644 t/t5411/test-0015-too-many-hooks-error.sh
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 2b3a74f249..2c59c537f9 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -423,6 +423,10 @@ the input.  The exit status of the 'proc-receive' hook only determines
 the success or failure of the group of commands sent to it, unless
 atomic push is in use.
 
+It is forbidden to specify more than one hook for 'proc-receive'. If a
+globally-configured 'proc-receive' must be overridden, use
+'hookcmd.<global-hook>.skip = true' to ignore it.
+
 [[post-receive]]
 post-receive
 ~~~~~~~~~~~~
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 57f3bb979c..4a6ad27404 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1145,11 +1145,40 @@ static int run_proc_receive_hook(struct command *commands,
 	int version = 0;
 	int code;
 
-	argv[0] = find_hook("proc-receive");
-	if (!argv[0]) {
+	struct strbuf hookname = STRBUF_INIT;
+	struct hook *proc_receive = NULL;
+	struct list_head *pos, *hooks;
+
+	strbuf_addstr(&hookname, "proc-receive");
+	hooks = hook_list(&hookname);
+
+	list_for_each(pos, hooks) {
+		if (proc_receive) {
+			rp_error("only one 'proc-receive' hook can be specified");
+			return -1;
+		}
+		proc_receive = list_entry(pos, struct hook, list);
+		/* check if the hookdir hook should be ignored */
+		if (proc_receive->from_hookdir) {
+			switch (configured_hookdir_opt()) {
+			case HOOKDIR_INTERACTIVE:
+			case HOOKDIR_NO:
+				proc_receive = NULL;
+				break;
+			default:
+				break;
+			}
+		}
+
+	}
+
+	if (!proc_receive) {
 		rp_error("cannot find hook 'proc-receive'");
 		return -1;
 	}
+
+
+	argv[0] = proc_receive->command.buf;
 	argv[1] = NULL;
 
 	proc.argv = argv;
diff --git a/t/t5411/test-0015-too-many-hooks-error.sh b/t/t5411/test-0015-too-many-hooks-error.sh
new file mode 100644
index 0000000000..2d64534510
--- /dev/null
+++ b/t/t5411/test-0015-too-many-hooks-error.sh
@@ -0,0 +1,47 @@
+test_expect_success "setup too  many proc-receive hooks (ok, $PROTOCOL)" '
+	write_script "proc-receive" <<-EOF &&
+	printf >&2 "# proc-receive hook\n"
+	test-tool proc-receive -v \
+		-r "ok refs/for/main/topic"
+	EOF
+
+	git -C "$upstream" config --add "hook.proc-receive.command" proc-receive &&
+	cp proc-receive "$upstream/hooks/proc-receive"
+'
+
+# Refs of upstream : main(A)
+# Refs of workbench: main(A)  tags/v123
+# git push         :                       next(A)  refs/for/main/topic(A)
+test_expect_success "proc-receive: reject more than one configured hook" '
+	test_must_fail git -C workbench push origin \
+		HEAD:next \
+		HEAD:refs/for/main/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/main/topic
+	remote: error: only one "proc-receive" hook can be specified
+	remote: # post-receive hook
+	remote: post-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	To <URL/of/upstream.git>
+	 * [new branch] HEAD -> next
+	 ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/main
+	<COMMIT-A> refs/heads/next
+	EOF
+	test_cmp expect actual
+'
+
+# Refs of upstream : main(A)             next(A)
+# Refs of workbench: main(A)  tags/v123
+test_expect_success "cleanup ($PROTOCOL)" '
+	git -C "$upstream" config --unset "hook.proc-receive.command" "proc-receive" &&
+	git -C "$upstream" update-ref -d refs/heads/next
+'

-- 
2.28.0.rc0.142.g3c755180ce-goog
Previous: Emily ShafferNext: Emily Shaffer
Message 42 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.