diff mbox series

[v2,5/6] hook: include hooks from the config

Message ID 20210812004258.74318-6-emilyshaffer@google.com (mailing list archive)
State Superseded
Headers show
Series config-based hooks restarted | expand

Commit Message

Emily Shaffer Aug. 12, 2021, 12:42 a.m. UTC
Teach the hook.[hc] library to parse configs to populare the list of
hooks to run for a given event.

Multiple commands can be specified for a given hook by providing
multiple "hook.<friendly-name>.command = <path-to-hook>" and
"hook.<friendly-name>.event = <hook-event>" lines. Hooks will be run in
config order of the "hook.<name>.event" lines.

For example:

  $ git config --list | grep ^hook
  hook.bar.command=~/bar.sh
  hook.bar.event=pre-commit

  $ git hook run
  # Runs ~/bar.sh
  # Runs .git/hooks/pre-commit

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/config/hook.txt |   5 +
 Documentation/git-hook.txt    |  23 ++++-
 builtin/hook.c                |   5 +-
 hook.c                        | 166 +++++++++++++++++++++++++++++-----
 hook.h                        |   7 +-
 t/t1800-hook.sh               | 141 ++++++++++++++++++++++++++++-
 6 files changed, 318 insertions(+), 29 deletions(-)

Comments

Junio C Hamano Aug. 12, 2021, 8:48 p.m. UTC | #1
Emily Shaffer <emilyshaffer@google.com> writes:

> Teach the hook.[hc] library to parse configs to populare the list of
> hooks to run for a given event.
>
> Multiple commands can be specified for a given hook by providing
> multiple "hook.<friendly-name>.command = <path-to-hook>" and
> "hook.<friendly-name>.event = <hook-event>" lines. Hooks will be run in
> config order of the "hook.<name>.event" lines.
>
> For example:
>
>   $ git config --list | grep ^hook
>   hook.bar.command=~/bar.sh
>   hook.bar.event=pre-commit

Your answer might be "read the design doc", but it is unclear to me
why "bar" (friendly-name) is needed in this picture at all.  Is it
because you may want to fire more than one command for pre-commit
event?  IOW,

	[hook "bar"]
		command = bar1.sh
		command = bar2.sh
		event = pre-commit

is easier to manage with an extra level of redirection?  I doubt it
as 

	[hook "pre-commit"]
		command = bar1.sh
		command = bar2.sh

would be equally expressive and shorter.  Or would it help use case
for multiple "friendly-name" to refer to the same "event", e.g.

	[hook "xyzzy"]
		event = pre-commit
		command = xyzzy1

	[hook "frotz"]
		event = pre-commit
                command = frotz1
                command = frotz2

or something?  I am not sure if this gives us useful extra
flexibility, and if so, the extra flexibility helps us more than it
confuses us.

And moving the "event" to the second level in the configuration
hierarchy, getting rid of "friendly-name" from the design, would not
make this example unworkable, either:

>   $ git hook run
>   # Runs ~/bar.sh
>   # Runs .git/hooks/pre-commit

Again, this is not an objection wrapped in a rhetorical question.
It just is that I do not see how the extra level of redirection
helps us.

> diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt
> index 96d3d6572c..a97b980cca 100644
> --- a/Documentation/config/hook.txt
> +++ b/Documentation/config/hook.txt
> @@ -1,3 +1,8 @@
> +hook.<command>.command::
> +	A command to execute during the <command> hook event. This can be an
> +	executable on your device, a oneliner for your shell, or the name of a
> +	hookcmd. See linkgit:git-hook[1].

Please make sure you use the terminology consistently.  If the
second level is "friendly name", hook.<name>.command should be
described, instead of hook.<command>.command.

Also, to help those who are familiar with the current Git from their
use in the past 10 years or so, giving an example name from the
current system may help, e.g. when describing hook.<name>.event,
you may want to say the values are things like "pre-commit",
"receive", etc.

> +This command parses the default configuration files for pairs of configs like
> +so:
> +
> +  [hook "linter"]
> +    event = pre-commit
> +    command = ~/bin/linter --c

The above addition of .command should also have hook.<name>.event
next to it, no?

> +Conmmands are run in the order Git encounters their associated

"Conmmands -> Commands", I would think.

> +`hook.<name>.event` configs during the configuration parse (see
> +linkgit:git-config[1]).

Here you use <name>, which should be matched by the description in
the first hunk of the patch to this file.

> +In general, when instructions suggest adding a script to
> +`.git/hooks/<hook-event>`, you can specify it in the config instead by running
> +`git config --add hook.<some-name>.command <path-to-script> && git config --add
> +hook.<some-name>.event <hook-event>` - this way you can share the script between
> +multiple repos. That is, `cp ~/my-script.sh ~/project/.git/hooks/pre-commit`
> +would become `git config --add hook.my-script.command ~/my-script.sh && git
> +config --add hook.my-script.event pre-commit`.

One repository may use a friendly name "xyzzy" while the other may
use "frotz" to group the hooks that trigger upon "pre-commit" event,
but unless one of the repositories change the friendly name to
match, they cannot share these configurations, no?  It seems that an
extra level of indirection is hindering sharing, rather than
helping.

> diff --git a/builtin/hook.c b/builtin/hook.c
> index 3aa65dd791..ea49dc4ef6 100644
> --- a/builtin/hook.c
> +++ b/builtin/hook.c
> @@ -49,7 +49,7 @@ static int list(int argc, const char **argv, const char *prefix)
>  	head = hook_list(hookname, 1);
>  
>  	if (list_empty(head)) {
> -		printf(_("no commands configured for hook '%s'\n"),
> +		printf(_("no hooks configured for event '%s'\n"),
>  		       hookname);
> ...
> @@ -58,7 +58,8 @@ static int list(int argc, const char **argv, const char *prefix)
>  		struct hook *item = list_entry(pos, struct hook, list);
>  		item = list_entry(pos, struct hook, list);
>  		if (item)
> -			printf("%s\n", item->hook_path);
> +			printf("%s\n", item->name ? item->name
> +						  : _("hook from hookdir"));
>  	}

I won't comment on this part as my comments on earlier patches would
probably have butchered the preimage already for this change to
survive intact ;-)
Emily Shaffer Aug. 19, 2021, 12:09 a.m. UTC | #2
On Thu, Aug 12, 2021 at 01:48:00PM -0700, Junio C Hamano wrote:
> 
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > Teach the hook.[hc] library to parse configs to populare the list of
> > hooks to run for a given event.
> >
> > Multiple commands can be specified for a given hook by providing
> > multiple "hook.<friendly-name>.command = <path-to-hook>" and
> > "hook.<friendly-name>.event = <hook-event>" lines. Hooks will be run in
> > config order of the "hook.<name>.event" lines.
> >
> > For example:
> >
> >   $ git config --list | grep ^hook
> >   hook.bar.command=~/bar.sh
> >   hook.bar.event=pre-commit
> 
> Your answer might be "read the design doc", but it is unclear to me
> why "bar" (friendly-name) is needed in this picture at all.  Is it
> because you may want to fire more than one command for pre-commit
> event?  IOW,
> 
> 	[hook "bar"]
> 		command = bar1.sh
> 		command = bar2.sh
> 		event = pre-commit
> 
> is easier to manage with an extra level of redirection?  I doubt it
> as 
> 
> 	[hook "pre-commit"]
> 		command = bar1.sh
> 		command = bar2.sh
> 
> would be equally expressive and shorter.  Or would it help use case
> for multiple "friendly-name" to refer to the same "event", e.g.
> 
> 	[hook "xyzzy"]
> 		event = pre-commit
> 		command = xyzzy1
> 
> 	[hook "frotz"]
> 		event = pre-commit
>                 command = frotz1
>                 command = frotz2
> 
> or something?  I am not sure if this gives us useful extra
> flexibility, and if so, the extra flexibility helps us more than it
> confuses us.
> 
> And moving the "event" to the second level in the configuration
> hierarchy, getting rid of "friendly-name" from the design, would not
> make this example unworkable, either:
> 
> >   $ git hook run
> >   # Runs ~/bar.sh
> >   # Runs .git/hooks/pre-commit
> 
> Again, this is not an objection wrapped in a rhetorical question.
> It just is that I do not see how the extra level of redirection
> helps us.

Please have a look at
https://lore.kernel.org/git/87fswey5wd.fsf%40evledraar.gmail.com and
replies, where Ævar and I discussed the schema change at length. I know
it is a lot of back and forth but I think it is useful to understand why
I ended up changing the schema this way.

> 
> > diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt
> > index 96d3d6572c..a97b980cca 100644
> > --- a/Documentation/config/hook.txt
> > +++ b/Documentation/config/hook.txt
> > @@ -1,3 +1,8 @@
> > +hook.<command>.command::
> > +	A command to execute during the <command> hook event. This can be an
> > +	executable on your device, a oneliner for your shell, or the name of a
> > +	hookcmd. See linkgit:git-hook[1].
> 
> Please make sure you use the terminology consistently.  If the
> second level is "friendly name", hook.<name>.command should be
> described, instead of hook.<command>.command.

Thanks, this is an oversight. Will update the config/hook.txt doc in
next reroll.

> 
> Also, to help those who are familiar with the current Git from their
> use in the past 10 years or so, giving an example name from the
> current system may help, e.g. when describing hook.<name>.event,
> you may want to say the values are things like "pre-commit",
> "receive", etc.

Sure.

> 
> > +This command parses the default configuration files for pairs of configs like
> > +so:
> > +
> > +  [hook "linter"]
> > +    event = pre-commit
> > +    command = ~/bin/linter --c
> 
> The above addition of .command should also have hook.<name>.event
> next to it, no?

I don't understand the question. Doesn't this config snippet equate to
"""
  hook.linter.event=pre-commit
  hook.linter.command=~/bin/linter --c
"""
? So in this case, '<name>' is 'linter', as that's not a native Git hook.

> 
> > +Conmmands are run in the order Git encounters their associated
> 
> "Conmmands -> Commands", I would think.
ACK
> 
> > +`hook.<name>.event` configs during the configuration parse (see
> > +linkgit:git-config[1]).
> 
> Here you use <name>, which should be matched by the description in
> the first hunk of the patch to this file.
Yep.
> 
> > +In general, when instructions suggest adding a script to
> > +`.git/hooks/<hook-event>`, you can specify it in the config instead by running
> > +`git config --add hook.<some-name>.command <path-to-script> && git config --add
> > +hook.<some-name>.event <hook-event>` - this way you can share the script between
> > +multiple repos. That is, `cp ~/my-script.sh ~/project/.git/hooks/pre-commit`
> > +would become `git config --add hook.my-script.command ~/my-script.sh && git
> > +config --add hook.my-script.event pre-commit`.
> 
> One repository may use a friendly name "xyzzy" while the other may
> use "frotz" to group the hooks that trigger upon "pre-commit" event,
> but unless one of the repositories change the friendly name to
> match, they cannot share these configurations, no?  It seems that an
> extra level of indirection is hindering sharing, rather than
> helping.

Ah, I think this means the documentation isn't sufficient, if you are
asking that. Instead of explaining it in ephemeral email, I think it is
better for me to explain it in documentation reroll, and for you to then
tell me how you interpret it. I expect to send the reroll before I go
home today, since I didn't receive comments from anybody besides you so
far.

Thanks very much for the feedback on the doc - this is very useful.

 - Emily
diff mbox series

Patch

diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt
index 96d3d6572c..a97b980cca 100644
--- a/Documentation/config/hook.txt
+++ b/Documentation/config/hook.txt
@@ -1,3 +1,8 @@ 
+hook.<command>.command::
+	A command to execute during the <command> hook event. This can be an
+	executable on your device, a oneliner for your shell, or the name of a
+	hookcmd. See linkgit:git-hook[1].
+
 hook.jobs::
 	Specifies how many hooks can be run simultaneously during parallelized
 	hook execution. If unspecified, defaults to the number of processors on
diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
index 11a8b87c60..c610ed9583 100644
--- a/Documentation/git-hook.txt
+++ b/Documentation/git-hook.txt
@@ -26,12 +26,31 @@  Git is unlikely to use for a native hook later on. For example, Git is much less
 likely to create a `mytool-validate-commit` hook than it is to create a
 `validate-commit` hook.
 
+This command parses the default configuration files for pairs of configs like
+so:
+
+  [hook "linter"]
+    event = pre-commit
+    command = ~/bin/linter --c
+
+Conmmands are run in the order Git encounters their associated
+`hook.<name>.event` configs during the configuration parse (see
+linkgit:git-config[1]).
+
+In general, when instructions suggest adding a script to
+`.git/hooks/<hook-event>`, you can specify it in the config instead by running
+`git config --add hook.<some-name>.command <path-to-script> && git config --add
+hook.<some-name>.event <hook-event>` - this way you can share the script between
+multiple repos. That is, `cp ~/my-script.sh ~/project/.git/hooks/pre-commit`
+would become `git config --add hook.my-script.command ~/my-script.sh && git
+config --add hook.my-script.event pre-commit`.
+
 SUBCOMMANDS
 -----------
 
 run::
-	Run the `<hook-name>` hook. See linkgit:githooks[5] for
-	the hook names we support.
+	Runs hooks configured for `<hook-name>`, in the order they are
+	discovered during the config parse.
 +
 Any positional arguments to the hook should be passed after an
 optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The
diff --git a/builtin/hook.c b/builtin/hook.c
index 3aa65dd791..ea49dc4ef6 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -49,7 +49,7 @@  static int list(int argc, const char **argv, const char *prefix)
 	head = hook_list(hookname, 1);
 
 	if (list_empty(head)) {
-		printf(_("no commands configured for hook '%s'\n"),
+		printf(_("no hooks configured for event '%s'\n"),
 		       hookname);
 		return 0;
 	}
@@ -58,7 +58,8 @@  static int list(int argc, const char **argv, const char *prefix)
 		struct hook *item = list_entry(pos, struct hook, list);
 		item = list_entry(pos, struct hook, list);
 		if (item)
-			printf("%s\n", item->hook_path);
+			printf("%s\n", item->name ? item->name
+						  : _("hook from hookdir"));
 	}
 
 	clear_hook_list(head);
diff --git a/hook.c b/hook.c
index e5acd02a50..51ada266bc 100644
--- a/hook.c
+++ b/hook.c
@@ -12,6 +12,50 @@  static void free_hook(struct hook *ptr)
 	free(ptr);
 }
 
+/*
+ * Walks the linked list at 'head' to check if any hook named 'name'
+ * already exists. Returns a pointer to that hook if so, otherwise returns NULL.
+ */
+static struct hook *find_hook_by_name(struct list_head *head,
+					 const char *name)
+{
+	struct list_head *pos = NULL, *tmp = NULL;
+	struct hook *found = NULL;
+
+	list_for_each_safe(pos, tmp, head) {
+		struct hook *it = list_entry(pos, struct hook, list);
+		if (!strcmp(it->name, name)) {
+			list_del(pos);
+			found = it;
+			break;
+		}
+	}
+	return found;
+}
+
+/*
+ * Adds a hook if it's not already in the list, or moves it to the tail of the
+ * list if it was already there. name == NULL indicates it's from the hookdir;
+ * just append it in this case.
+ */
+static void append_or_move_hook(struct list_head *head, const char *name)
+{
+	struct hook *to_add = NULL;
+
+	/* if it's not from hookdir, check if the hook is already in the list */
+	if (name)
+		to_add = find_hook_by_name(head, name);
+
+	if (!to_add) {
+		/* adding a new hook, not moving an old one */
+		to_add = xmalloc(sizeof(*to_add));
+		to_add->name = name;
+		to_add->feed_pipe_cb_data = NULL;
+	}
+
+	list_add_tail(&to_add->list, head);
+}
+
 static void remove_hook(struct list_head *to_remove)
 {
 	struct hook *hook_to_remove = list_entry(to_remove, struct hook, list);
@@ -99,38 +143,80 @@  const char *find_hook_gently(const char *name)
 
 int hook_exists(const char *name)
 {
-	return !!find_hook(name);
+	return !list_empty(hook_list(name, 0));
 }
 
 struct hook_config_cb
 {
-	struct strbuf *hook_key;
+	const char *hook_event;
 	struct list_head *list;
 };
 
-struct list_head* hook_list(const char* hookname, int allow_unknown)
+/*
+ * Callback for git_config which adds configured hooks to a hook list.  Hooks
+ * can be configured by specifying both hook.<friend-name>.command = <path> and
+ * hook.<friendly-name>.event = <hook-event>.
+ */
+static int hook_config_lookup(const char *key, const char *value, void *cb_data)
+{
+	struct hook_config_cb *data = cb_data;
+	const char *subsection, *parsed_key;
+	size_t subsection_len = 0;
+	struct strbuf subsection_cpy = STRBUF_INIT;
+
+	/*
+	 * Don't bother doing the expensive parse if there's no
+	 * chance that the config matches 'hook.myhook.event = hook_event'.
+	 */
+	if (!value || strcmp(value, data->hook_event))
+		return 0;
+
+	/* Looking for "hook.friendlyname.event = hook_event" */
+	if (parse_config_key(key,
+			    "hook",
+			    &subsection,
+			    &subsection_len,
+			    &parsed_key) ||
+	    strcmp(parsed_key, "event"))
+		return 0;
+
+	/*
+	 * 'subsection' is a pointer to the internals of 'key', which we don't
+	 * own the memory for. Copy it away to the hook list.
+	 */
+	strbuf_add(&subsection_cpy, subsection, subsection_len);
+
+	append_or_move_hook(data->list, strbuf_detach(&subsection_cpy, NULL));
+
+
+	return 0;
+}
+
+struct list_head* hook_list(const char *hookname, int allow_unknown)
 {
 	struct list_head *hook_head = xmalloc(sizeof(struct list_head));
-	const char *hook_path;
+	struct hook_config_cb cb_data = {
+		.hook_event = hookname,
+		.list = hook_head,
+	};
 
+	if (!allow_unknown && !known_hook(hookname))
+		die(_("Don't recognize hook event '%s'. "
+		      "Is it documented in 'githooks.txt'?"),
+		      hookname);
 
 	INIT_LIST_HEAD(hook_head);
 
 	if (!hookname)
 		return NULL;
 
-	if (allow_unknown)
-		hook_path = find_hook_gently(hookname);
-	else
-		hook_path = find_hook(hookname);
+	/* Add the hooks from the config, e.g. hook.myhook.event = pre-commit */
+	git_config(hook_config_lookup, &cb_data);
 
-	/* Add the hook from the hookdir */
-	if (hook_path) {
-		struct hook *to_add = xmalloc(sizeof(*to_add));
-		to_add->hook_path = hook_path;
-		to_add->feed_pipe_cb_data = NULL;
-		list_add_tail(&to_add->list, hook_head);
-	}
+	/* Add the hook from the hookdir. The placeholder makes it easier to
+	 * allocate work in pick_next_hook. */
+	if (find_hook_gently(hookname))
+		append_or_move_hook(hook_head, NULL);
 
 	return hook_head;
 }
@@ -191,11 +277,43 @@  static int pick_next_hook(struct child_process *cp,
 	cp->trace2_hook_name = hook_cb->hook_name;
 	cp->dir = hook_cb->options->dir;
 
+	/* to enable oneliners, let config-specified hooks run in shell.
+	 * config-specified hooks have a name. */
+	cp->use_shell = !!run_me->name;
+
 	/* add command */
-	if (hook_cb->options->absolute_path)
-		strvec_push(&cp->args, absolute_path(run_me->hook_path));
-	else
-		strvec_push(&cp->args, run_me->hook_path);
+	if (run_me->name) {
+		/* ...from config */
+		struct strbuf cmd_key = STRBUF_INIT;
+		char *command = NULL;
+
+		strbuf_addf(&cmd_key, "hook.%s.command", run_me->name);
+		if (git_config_get_string(cmd_key.buf, &command)) {
+			/* TODO test me! */
+			die(_("'hook.%s.command' must be configured "
+			      "or 'hook.%s.event' must be removed; aborting.\n"),
+			    run_me->name, run_me->name);
+		}
+
+		strvec_push(&cp->args, command);
+	} else {
+		/* ...from hookdir. */
+		const char *hook_path = NULL;
+		/*
+		 *
+		 * At this point we are already running, so don't validate
+		 * whether the hook name is known or not.
+		 */
+		hook_path = find_hook_gently(hook_cb->hook_name);
+		if (!hook_path)
+			BUG("hookdir hook in hook list but no hookdir hook present in filesystem");
+
+		if (hook_cb->options->absolute_path)
+			hook_path = absolute_path(hook_path);
+
+		strvec_push(&cp->args, hook_path);
+	}
+
 
 	/*
 	 * add passed-in argv, without expanding - let the user get back
@@ -225,8 +343,11 @@  static int notify_start_failure(struct strbuf *out,
 
 	hook_cb->rc |= 1;
 
-	strbuf_addf(out, _("Couldn't start hook '%s'\n"),
-		    attempted->hook_path);
+	if (attempted->name)
+		strbuf_addf(out, _("Couldn't start hook '%s'\n"),
+		    attempted->name);
+	else
+		strbuf_addstr(out, _("Couldn't start hook from hooks directory\n"));
 
 	return 1;
 }
@@ -320,7 +441,8 @@  int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options)
 		BUG("choose only one method to populate stdin");
 
 	/*
-	 * 'git hooks run <hookname>' uses run_found_hooks, so we don't need to
+	 * 'git hooks run <hookname>' uses run_found_hooks, and we want to make
+	 * sure internal callers are using known hooks, so we don't need to
 	 * allow unknown hooknames here.
 	 */
 	hooks = hook_list(hook_name, 0);
diff --git a/hook.h b/hook.h
index ffa96c6e4d..a6263864b9 100644
--- a/hook.h
+++ b/hook.h
@@ -27,8 +27,11 @@  int hook_exists(const char *hookname);
 
 struct hook {
 	struct list_head list;
-	/* The path to the hook */
-	const char *hook_path;
+	/*
+	 * The friendly name of the hook. NULL indicates the hook is from the
+	 * hookdir.
+	 */
+	const char *name;
 
 	/*
 	 * Use this to keep state for your feed_pipe_fn if you are using
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 217db848b3..ef2432f53a 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -1,13 +1,29 @@ 
 #!/bin/bash
 
-test_description='git-hook command'
+test_description='git-hook command and config-managed multihooks'
 
 . ./test-lib.sh
 
+setup_hooks () {
+	test_config hook.ghi.event pre-commit --add
+	test_config hook.ghi.command "/path/ghi" --add
+	test_config_global hook.def.event pre-commit --add
+	test_config_global hook.def.command "/path/def" --add
+}
+
+setup_hookdir () {
+	mkdir .git/hooks
+	write_script .git/hooks/pre-commit <<-EOF
+	echo \"Legacy Hook\"
+	EOF
+	test_when_finished rm -rf .git/hooks
+}
+
 test_expect_success 'git hook usage' '
 	test_expect_code 129 git hook &&
 	test_expect_code 129 git hook run &&
 	test_expect_code 129 git hook run -h &&
+	test_expect_code 129 git hook list -h &&
 	test_expect_code 129 git hook run --unknown 2>err &&
 	grep "unknown option" err
 '
@@ -153,4 +169,127 @@  test_expect_success 'stdin to hooks' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git hook list orders by config order' '
+	setup_hooks &&
+
+	cat >expected <<-EOF &&
+	def
+	ghi
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'git hook list reorders on duplicate event declarations' '
+	setup_hooks &&
+
+	# 'def' is usually configured globally; move it to the end by
+	# configuring it locally.
+	test_config hook.def.event "pre-commit" --add &&
+
+	cat >expected <<-EOF &&
+	ghi
+	def
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'git hook list shows hooks from the hookdir' '
+	setup_hookdir &&
+
+	cat >expected <<-EOF &&
+	hook from hookdir
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'inline hook definitions execute oneliners' '
+	test_config hook.oneliner.event "pre-commit" &&
+	test_config hook.oneliner.command "echo \"Hello World\"" &&
+
+	echo "Hello World" >expected &&
+
+	# hooks are run with stdout_to_stderr = 1
+	git hook run pre-commit 2>actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'inline hook definitions resolve paths' '
+	write_script sample-hook.sh <<-EOF &&
+	echo \"Sample Hook\"
+	EOF
+
+	test_when_finished "rm sample-hook.sh" &&
+
+	test_config hook.sample-hook.event pre-commit &&
+	test_config hook.sample-hook.command "\"$(pwd)/sample-hook.sh\"" &&
+
+	echo \"Sample Hook\" >expected &&
+
+	# hooks are run with stdout_to_stderr = 1
+	git hook run pre-commit 2>actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'hookdir hook included in git hook run' '
+	setup_hookdir &&
+
+	echo \"Legacy Hook\" >expected &&
+
+	# hooks are run with stdout_to_stderr = 1
+	git hook run pre-commit 2>actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'stdin to multiple hooks' '
+	test_config hook.stdin-a.event "test-hook" --add &&
+	test_config hook.stdin-a.command "xargs -P1 -I% echo a%" --add &&
+	test_config hook.stdin-b.event "test-hook" --add &&
+	test_config hook.stdin-b.command "xargs -P1 -I% echo b%" --add &&
+
+	cat >input <<-EOF &&
+	1
+	2
+	3
+	EOF
+
+	cat >expected <<-EOF &&
+	a1
+	a2
+	a3
+	b1
+	b2
+	b3
+	EOF
+
+	git hook run --to-stdin=input test-hook 2>actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'multiple hooks in series' '
+	test_config hook.series-1.event "test-hook" &&
+	test_config hook.series-1.command "echo 1" --add &&
+	test_config hook.series-2.event "test-hook" &&
+	test_config hook.series-2.command "echo 2" --add &&
+	mkdir .git/hooks &&
+	write_script .git/hooks/test-hook <<-EOF &&
+	echo 3
+	EOF
+
+	cat >expected <<-EOF &&
+	1
+	2
+	3
+	EOF
+
+	git hook run -j1 test-hook 2>actual &&
+	test_cmp expected actual &&
+
+	rm -rf .git/hooks
+'
 test_done