git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Emily Shaffer <emilyshaffer@google.com>
To: git@vger.kernel.org
Cc: Emily Shaffer <emilyshaffer@google.com>
Subject: [PATCH v3 14/17] proc-receive: acquire hook list from hook.h
Date: Mon, 21 Dec 2020 16:04:32 -0800	[thread overview]
Message-ID: <20201222000435.1529768-15-emilyshaffer@google.com> (raw)
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


  parent reply	other threads:[~2020-12-22  0:07 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-14 23:25 [PATCH] commit: use config-based hooks (config-based hooks part II) Emily Shaffer
2020-10-16 18:34 ` Junio C Hamano
2020-12-05  1:49 ` [PATCH 00/17] use config-based hooks (config-based hooks part Emily Shaffer
2020-12-05  1:49   ` [PATCH 01/17] commit: use config-based hooks Emily Shaffer
2020-12-05  1:49   ` [PATCH 02/17] am: convert applypatch hooks to use config Emily Shaffer
2020-12-05  1:49   ` [PATCH 03/17] merge: use config-based hooks for post-merge hook Emily Shaffer
2020-12-05  1:49   ` [PATCH 04/17] gc: use hook library for pre-auto-gc hook Emily Shaffer
2020-12-05  1:49   ` [PATCH 05/17] rebase: teach pre-rebase to use hook.h Emily Shaffer
2020-12-05  1:49   ` [PATCH 06/17] read-cache: convert post-index-change hook to use config Emily Shaffer
2020-12-05  1:49   ` [PATCH 07/17] receive-pack: convert push-to-checkout hook to hook.h Emily Shaffer
2020-12-05  1:49   ` [PATCH 08/17] git-p4: use 'git hook' to run hooks Emily Shaffer
2020-12-16  0:27     ` Josh Steadmon
2020-12-16 20:19       ` Emily Shaffer
2020-12-05  1:49   ` [PATCH 09/17] hooks: convert 'post-checkout' hook to hook library Emily Shaffer
2020-12-05  1:49   ` [PATCH 10/17] hook: convert 'post-rewrite' hook to config Emily Shaffer
2020-12-08 23:02     ` Josh Steadmon
2020-12-15 23:42       ` Emily Shaffer
2020-12-05  1:49   ` [PATCH 11/17] transport: convert pre-push hook to use config Emily Shaffer
2020-12-05  1:49   ` [PATCH 12/17] reference-transaction: look for hooks in config Emily Shaffer
2020-12-05  1:49   ` [PATCH 13/17] receive-pack: convert 'update' hook to hook.h Emily Shaffer
2020-12-05  1:49   ` [PATCH 14/17] proc-receive: acquire hook list from hook.h Emily Shaffer
2020-12-05  1:49   ` [PATCH 15/17] post-update: use hook.h library Emily Shaffer
2020-12-05  1:49   ` [PATCH 16/17] receive-pack: convert receive hooks to hook.h Emily Shaffer
2020-12-05  1:49   ` [PATCH 17/17] run-command: stop thinking about hooks Emily Shaffer
2020-12-22  0:04   ` [PATCH v3 00/17] use config-based hooks (config-based hooks part II) Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 01/17] commit: use config-based hooks Emily Shaffer
2021-02-01 22:08       ` Junio C Hamano
2021-03-10 19:51         ` Emily Shaffer
2021-03-10 22:36           ` Junio C Hamano
2021-02-01 23:02       ` Junio C Hamano
2021-03-10 19:39         ` Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 02/17] am: convert applypatch hooks to use config Emily Shaffer
2021-02-01 22:05       ` Junio C Hamano
2020-12-22  0:04     ` [PATCH v3 03/17] merge: use config-based hooks for post-merge hook Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 04/17] gc: use hook library for pre-auto-gc hook Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 05/17] rebase: teach pre-rebase to use hook.h Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 06/17] read-cache: convert post-index-change hook to use config Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 07/17] receive-pack: convert push-to-checkout hook to hook.h Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 08/17] git-p4: use 'git hook' to run hooks Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 09/17] hooks: convert 'post-checkout' hook to hook library Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 10/17] hook: convert 'post-rewrite' hook to config Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 11/17] transport: convert pre-push hook to use config Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 12/17] reference-transaction: look for hooks in config Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 13/17] receive-pack: convert 'update' hook to hook.h Emily Shaffer
2020-12-22  0:04     ` Emily Shaffer [this message]
2020-12-22  0:04     ` [PATCH v3 15/17] post-update: use hook.h library Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 16/17] receive-pack: convert receive hooks to hook.h Emily Shaffer
2020-12-22  0:04     ` [PATCH v3 17/17] run-command: stop thinking about hooks Emily Shaffer
2020-12-28 19:59     ` [PATCH v3 00/17] use config-based hooks (config-based hooks part II) Emily Shaffer
2020-12-28 22:40     ` [PATCH v3 18/17] doc: make git-hook.txt point of truth Emily Shaffer
2020-12-28 23:15       ` Emily Shaffer
2021-02-18 22:32     ` [PATCH v3 00/17] use config-based hooks (config-based hooks part II) Josh Steadmon
2020-12-16  0:31 ` [PATCH] commit: " Josh Steadmon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201222000435.1529768-15-emilyshaffer@google.com \
    --to=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).