[v4,0/5] config-based hooks restarted

Message

Ævar Arnfjörð Bjarmason Sept. 9, 2021, 12:41 p.m. UTC

This is a re-roll of Emily's es/config-based-hooks topic that goes on
top of the v5 of my ab/config-based-hooks-base, and which doesn't have
errors under SANITIZE=leak.

Emily: Sorry, I've got no intention to steal this one too, hopefully
you can get around to your own re-roll.

But per my
https://lore.kernel.org/git/[e-mail]/ the
lack of this re-roll is is currently blocking the pick-up of my
re-rolled v5 of ab/config-based-hooks-base at [1], as well as causing
a failure in "seen" when combined with my ab/sanitize-leak-ci (and
hn/reftable, but that's another issue...).

Junio: So hopefully you can pick up the v5[1] of the base topic now &
this preliminary v4 of es/config-based-hooks.

The range-diff below is against Emily's 30ffe98601e, i.e. her v3 at
[2].

This version is based on Emily's preliminary cf1f8e34a34
(nasamuffin/config-based-hooks-restart), which appeared to be her
August 31 rebasing addressing of many outstanding points in the v3
series.

My own changes on top of that were twofold: Adjustments to changes in
the base topic (many done to make the overall diff/changes here
smaller), and memory leak fixes to get this to pass under
SANITIZE=leak, there's various other minor but not-notable changes
here and there, see the range-diff.

1. https://lore.kernel.org/git/[e-mail]/
2. https://lore.kernel.org/git/[e-mail]/

Emily Shaffer (5):
  hook: run a list of hooks instead
  hook: allow parallel hook execution
  hook: introduce "git hook list"
  hook: include hooks from the config
  hook: allow out-of-repo 'git hook' invocations

 Documentation/config.txt      |   2 +
 Documentation/config/hook.txt |  22 +++
 Documentation/git-hook.txt    | 157 ++++++++++++++++++-
 builtin/am.c                  |   4 +-
 builtin/checkout.c            |   2 +-
 builtin/clone.c               |   2 +-
 builtin/hook.c                |  71 ++++++++-
 builtin/merge.c               |   2 +-
 builtin/rebase.c              |   2 +-
 builtin/receive-pack.c        |   9 +-
 builtin/worktree.c            |   2 +-
 commit.c                      |   2 +-
 git.c                         |   2 +-
 hook.c                        | 277 +++++++++++++++++++++++++++++-----
 hook.h                        |  45 +++++-
 read-cache.c                  |   2 +-
 refs.c                        |   2 +-
 reset.c                       |   3 +-
 sequencer.c                   |   4 +-
 t/t1800-hook.sh               | 194 +++++++++++++++++++++++-
 transport.c                   |   2 +-
 21 files changed, 734 insertions(+), 74 deletions(-)
 create mode 100644 Documentation/config/hook.txt

Range-diff against v3:
1:  6d6400329cd ! 1:  2f0cac14965 hook: run a list of hooks instead
    @@ Commit message
         executable for a single hook event.
     
         Signed-off-by: Emily Shaffer <[e-mail]>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
     
      ## builtin/hook.c ##
     @@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
    @@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
      	const char *hook_name;
     -	const char *hook_path;
     +	struct list_head *hooks;
    -+
      	struct option run_options[] = {
      		OPT_BOOL(0, "ignore-missing", &ignore_missing,
      			 N_("exit quietly with a zero exit code if the requested hook cannot be found")),
     @@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
    - 	git_config(git_default_config, NULL);
    - 
    + 	 * run_hooks() instead...
    + 	 */
      	hook_name = argv[0];
     -	if (ignore_missing)
    ++	hooks = list_hooks(hook_name);
    ++	if (list_empty(hooks)) {
    ++		clear_hook_list(hooks);
    ++
    + 		/* ... act like a plain run_hooks() under --ignore-missing */
     -		return run_hooks_oneshot(hook_name, &opt);
     -	hook_path = find_hook(hook_name);
     -	if (!hook_path) {
    -+	hooks = list_hooks(hook_name);
    -+	if (list_empty(hooks)) {
    -+		/* ... act like run_hooks_oneshot() under --ignore-missing */
     +		if (ignore_missing)
     +			return 0;
      		error("cannot find a hook named %s", hook_name);
    @@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
     
      ## hook.c ##
     @@
    - #include "hook-list.h"
    + #include "run-command.h"
      #include "config.h"
      
     +static void free_hook(struct hook *ptr)
     +{
    -+	if (ptr)
    -+		free(ptr->feed_pipe_cb_data);
    ++	if (!ptr)
    ++		return;
    ++
    ++	free(ptr->feed_pipe_cb_data);
     +	free(ptr);
     +}
     +
    @@ hook.c
     +	struct list_head *pos, *tmp;
     +	list_for_each_safe(pos, tmp, head)
     +		remove_hook(pos);
    ++	free(head);
     +}
     +
    - static int known_hook(const char *name)
    + const char *find_hook(const char *name)
      {
    - 	const char **p;
    + 	static struct strbuf path = STRBUF_INIT;
     @@ hook.c: const char *find_hook(const char *name)
      
      int hook_exists(const char *name)
      {
     -	return !!find_hook(name);
    -+	return !list_empty(list_hooks(name));
    ++	struct list_head *hooks;
    ++	int exists;
    ++
    ++	hooks = list_hooks(name);
    ++	exists = !list_empty(hooks);
    ++	clear_hook_list(hooks);
    ++
    ++	return exists;
     +}
     +
     +struct list_head *list_hooks(const char *hookname)
    @@ hook.c: static int notify_hook_finished(int result,
      }
      
     -int run_hooks(const char *hook_name, const char *hook_path,
    --	      struct run_hooks_opt *options)
     +int run_hooks(const char *hook_name, struct list_head *hooks,
    -+		    struct run_hooks_opt *options)
    + 	      struct run_hooks_opt *options)
      {
     -	struct strbuf abs_path = STRBUF_INIT;
     -	struct hook my_hook = {
    @@ hook.c: int run_hooks(const char *hook_name, const char *hook_path,
     -		my_hook.hook_path = abs_path.buf;
     -	}
     -	cb_data.run_me = &my_hook;
    -+
     +	cb_data.head = hooks;
     +	cb_data.run_me = list_first_entry(hooks, struct hook, list);
      
    @@ hook.c: int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *optio
      
     -	ret = run_hooks(hook_name, hook_path, options);
     +	ret = run_hooks(hook_name, hooks, options);
    -+
    + 
      cleanup:
      	run_hooks_opt_clear(options);
    -+	clear_hook_list(hooks);
    - 	return ret;
    - }
     
      ## hook.h ##
     @@
    @@ hook.h
      #include "run-command.h"
     +#include "list.h"
      
    - /*
    -  * Returns the path to the hook file, or NULL if the hook is missing
    -@@ hook.h: const char *find_hook(const char *name);
    - int hook_exists(const char *hookname);
    - 
      struct hook {
     +	struct list_head list;
      	/* The path to the hook */
      	const char *hook_path;
      
    -@@ hook.h: struct hook {
    - 	void *feed_pipe_cb_data;
    - };
    - 
    -+/*
    -+ * Provides a linked list of 'struct hook' detailing commands which should run
    -+ * in response to the 'hookname' event, in execution order.
    -+ */
    -+struct list_head *list_hooks(const char *hookname);
    -+
    - struct run_hooks_opt
    - {
    - 	/* Environment vars to be set for each hook */
     @@ hook.h: struct hook_cb_data {
      	/* rc reflects the cumulative failure state */
      	int rc;
    @@ hook.h: struct hook_cb_data {
      	struct hook *run_me;
      	struct run_hooks_opt *options;
      	int *invoked_hook;
    +@@ hook.h: struct hook_cb_data {
    + const char *find_hook(const char *name);
    + 
    + /**
    +- * A boolean version of find_hook()
    ++ * Provides a linked list of 'struct hook' detailing commands which should run
    ++ * in response to the 'hookname' event, in execution order.
    ++ */
    ++struct list_head *list_hooks(const char *hookname);
    ++
    ++/**
    ++ * A boolean version of list_hooks()
    +  */
    + int hook_exists(const char *hookname);
    + 
     @@ hook.h: void run_hooks_opt_clear(struct run_hooks_opt *o);
    + 
    + /**
    +  * Takes an already resolved hook found via find_hook() and runs
    +- * it. Does not call run_hooks_opt_clear() for you.
    ++ * it. Does not call run_hooks_opt_clear() for you, but does call
    ++ * clear_hook_list().
       *
       * See run_hooks_oneshot() for the simpler one-shot API.
       */
     -int run_hooks(const char *hookname, const char *hook_path,
    --	      struct run_hooks_opt *options);
     +int run_hooks(const char *hookname, struct list_head *hooks,
    -+		    struct run_hooks_opt *options);
    - 
    - /**
    -  * Calls find_hook() on your "hook_name" and runs the hooks (if any)
    -@@ hook.h: int run_hooks(const char *hookname, const char *hook_path,
    -  */
    - int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options);
    + 	      struct run_hooks_opt *options);
      
    -+/* Empties the list at 'head', calling 'free_hook()' on each entry */
    ++/**
    ++ * Empties the list at 'head', calling 'free_hook()' on each
    ++ * entry. Called implicitly by run_hooks() (and run_hooks_oneshot()).
    ++ */
     +void clear_hook_list(struct list_head *head);
     +
    - #endif
    + /**
    +  * Calls find_hook() on your "hook_name" and runs the hooks (if any)
    +  * with run_hooks().
2:  dfb995ce4d4 ! 2:  b03e70c805e hook: allow parallel hook execution
    @@ Commit message
         Signed-off-by: Emily Shaffer <[e-mail]>
         Helped-by: Ævar Arnfjörð Bjarmason <[e-mail]>
     
    + ## Documentation/config.txt ##
    +@@ Documentation/config.txt: include::config/guitool.txt[]
    + 
    + include::config/help.txt[]
    + 
    ++include::config/hook.txt[]
    ++
    + include::config/http.txt[]
    + 
    + include::config/i18n.txt[]
    +
      ## Documentation/config/hook.txt (new) ##
     @@
     +hook.jobs::
    @@ Documentation/git-hook.txt: OPTIONS
     +--jobs::
     +	Only valid for `run`.
     ++
    -+Specify how many hooks to run simultaneously. If this flag is not specified, use
    -+the value of the `hook.jobs` config. If the config is not specified, use the
    -+number of CPUs on the current system. Some hooks may be ineligible for
    -+parallelization: for example, 'commit-msg' intends hooks modify the commit
    -+message body and cannot be parallelized.
    ++Specify how many hooks to run simultaneously. If this flag is not specified,
    ++uses the value of the `hook.jobs` config, see linkgit:git-config[1]. If the
    ++config is not specified, uses the number of CPUs on the current system. Some
    ++hooks may be ineligible for parallelization: for example, 'commit-msg' intends
    ++hooks modify the commit message body and cannot be parallelized.
     +
     +CONFIGURATION
     +-------------
    @@ hook.c: static int notify_hook_finished(int result,
     +}
     +
      int run_hooks(const char *hook_name, struct list_head *hooks,
    - 		    struct run_hooks_opt *options)
    + 	      struct run_hooks_opt *options)
      {
     @@ hook.c: int run_hooks(const char *hook_name, struct list_head *hooks,
      		.options = options,
    @@ hook.c: int run_hooks(const char *hook_name, struct list_head *hooks,
      
      	if (!options)
      		BUG("a struct run_hooks_opt must be provided to run_hooks");
    - 
    --
    +@@ hook.c: int run_hooks(const char *hook_name, struct list_head *hooks,
      	cb_data.head = hooks;
      	cb_data.run_me = list_first_entry(hooks, struct hook, list);
      
    @@ hook.h: struct run_hooks_opt
     +	 */
     +	int jobs;
     +
    - 	/* Resolve and run the "absolute_path(hook)" instead of
    + 	/*
    + 	 * Resolve and run the "absolute_path(hook)" instead of
      	 * "hook". Used for "git worktree" hooks
    - 	 */
     @@ hook.h: struct run_hooks_opt
      	int *invoked_hook;
      };
      
     -#define RUN_HOOKS_OPT_INIT { \
    --	.env = STRVEC_INIT, \
    --	.args = STRVEC_INIT, \
    --}
    --
    - /*
    -  * To specify a 'struct string_list', set 'run_hooks_opt.feed_pipe_ctx' to the
    -  * string_list and set 'run_hooks_opt.feed_pipe' to 'pipe_from_string_list()'.
    -@@ hook.h: struct hook_cb_data {
    - 	int *invoked_hook;
    - };
    - 
     +#define RUN_HOOKS_OPT_INIT_SERIAL { \
     +	.jobs = 1, \
     +	.env = STRVEC_INIT, \
    @@ hook.h: struct hook_cb_data {
     +
     +#define RUN_HOOKS_OPT_INIT_PARALLEL { \
     +	.jobs = 0, \
    -+	.env = STRVEC_INIT, \
    -+	.args = STRVEC_INIT, \
    -+}
    -+
    - void run_hooks_opt_clear(struct run_hooks_opt *o);
    - 
    - /**
    + 	.env = STRVEC_INIT, \
    + 	.args = STRVEC_INIT, \
    + }
     
      ## read-cache.c ##
     @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struct lock_file *l
3:  c8a04306e90 ! 3:  3e647b8dba7 hook: introduce "git hook list"
    @@ Commit message
         hooks were configured and whether or not they will run.
     
         Signed-off-by: Emily Shaffer <[e-mail]>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
     
      ## Documentation/git-hook.txt ##
     @@ Documentation/git-hook.txt: SYNOPSIS
    @@ builtin/hook.c: static const char * const builtin_hook_run_usage[] = {
     +
     +static int list(int argc, const char **argv, const char *prefix)
     +{
    -+	struct list_head *head, *pos;
    ++	struct list_head *hooks;
    ++	struct list_head *pos;
     +	const char *hookname = NULL;
    -+	struct strbuf hookdir_annotation = STRBUF_INIT;
    -+
     +	struct option list_options[] = {
     +		OPT_END(),
     +	};
    ++	int ret = 0;
     +
     +	argc = parse_options(argc, argv, prefix, list_options,
     +			     builtin_hook_list_usage, 0);
     +
    -+	if (argc < 1)
    ++	/*
    ++	 * The only unnamed argument provided should be the hook-name; if we add
    ++	 * arguments later they probably should be caught by parse_options.
    ++	 */
    ++	if (argc != 1)
     +		usage_msg_opt(_("You must specify a hook event name to list."),
     +			      builtin_hook_list_usage, list_options);
     +
     +	hookname = argv[0];
     +
    -+	head = hook_list(hookname);
    ++	hooks = list_hooks(hookname);
     +
    -+	if (list_empty(head))
    -+		return 1;
    ++	if (list_empty(hooks)) {
    ++		ret = 1;
    ++		goto cleanup;
    ++	}
     +
    -+	list_for_each(pos, head) {
    ++	list_for_each(pos, hooks) {
     +		struct hook *item = list_entry(pos, struct hook, list);
     +		item = list_entry(pos, struct hook, list);
     +		if (item)
     +			printf("%s\n", item->hook_path);
     +	}
     +
    -+	clear_hook_list(head);
    -+	strbuf_release(&hookdir_annotation);
    ++cleanup:
    ++	clear_hook_list(hooks);
     +
    -+	return 0;
    ++	return ret;
     +}
      static int run(int argc, const char **argv, const char *prefix)
      {
    @@ builtin/hook.c: int cmd_hook(int argc, const char **argv, const char *prefix)
      		return run(argc, argv, prefix);
      
     
    - ## hook.c ##
    -@@ hook.c: struct list_head *list_hooks(const char *hookname)
    - {
    - 	struct list_head *hook_head = xmalloc(sizeof(struct list_head));
    + ## t/t1800-hook.sh ##
    +@@ t/t1800-hook.sh: test_expect_success 'git hook usage' '
    + 	test_expect_code 129 git hook run &&
    + 	test_expect_code 129 git hook run -h &&
    + 	test_expect_code 129 git hook run --unknown 2>err &&
    ++	test_expect_code 129 git hook list &&
    ++	test_expect_code 129 git hook list -h &&
    + 	grep "unknown option" err
    + '
      
    -+
    - 	INIT_LIST_HEAD(hook_head);
    +@@ t/t1800-hook.sh: test_expect_success 'git hook run -- pass arguments' '
    + 	test_cmp expect actual
    + '
      
    - 	if (!hookname)
    -@@ hook.c: struct list_head *list_hooks(const char *hookname)
    - 
    - 	if (have_git_dir()) {
    - 		const char *hook_path = find_hook(hookname);
    --
    --		/* Add the hook from the hookdir */
    - 		if (hook_path) {
    - 			struct hook *to_add = xmalloc(sizeof(*to_add));
    - 			to_add->hook_path = hook_path;
    ++test_expect_success 'git hook list: does-not-exist hook' '
    ++	test_expect_code 1 git hook list does-not-exist
    ++'
    ++
    ++test_expect_success 'git hook list: existing hook' '
    ++	cat >expect <<-\EOF &&
    ++	.git/hooks/test-hook
    ++	EOF
    ++	git hook list test-hook >actual &&
    ++	test_cmp expect actual
    ++'
    ++
    + test_expect_success 'git hook run -- out-of-repo runs excluded' '
    + 	write_script .git/hooks/test-hook <<-EOF &&
    + 	echo Test hook
4:  af14116d0fa < -:  ----------- hook: allow running non-native hooks
5:  2bbb179962e ! 4:  d0f5b30fb27 hook: include hooks from the config
    @@ Metadata
      ## Commit message ##
         hook: include hooks from the config
     
    -    Teach the hook.[hc] library to parse configs to populare the list of
    +    Teach the hook.[hc] library to parse configs to populate 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.
    +    "hook.<friendly-name>.event = <hook-event>" lines. Hooks will be started
    +    in config order of the "hook.<name>.event" lines (but may run in
    +    parallel).
     
         For example:
     
    -      $ git config --list | grep ^hook
    +      $ git config --get-regexp "^hook\."
           hook.bar.command=~/bar.sh
           hook.bar.event=pre-commit
     
    -      $ git hook run
    -      # Runs ~/bar.sh
    -      # Runs .git/hooks/pre-commit
    +      # Will run ~/bar.sh, then .git/hooks/pre-commit
    +      $ git hook run pre-commit
     
         Signed-off-by: Emily Shaffer <[e-mail]>
     
    @@ Documentation/config/hook.txt
      	hook execution. If unspecified, defaults to the number of processors on
     
      ## Documentation/git-hook.txt ##
    -@@ Documentation/git-hook.txt: 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.
    +@@ Documentation/git-hook.txt: This command is an interface to git hooks (see linkgit:githooks[5]).
    + Currently it only provides a convenience wrapper for running hooks for
    + use by git itself. In the future it might gain other functionality.
      
    -+This command parses the default configuration files for pairs of configs like
    ++It's possible to use this command to refer to hooks which are not native to Git,
    ++for example if a wrapper around Git wishes to expose hooks into its own
    ++operation in a way which is already familiar to Git users. However, wrappers
    ++invoking such hooks should be careful to name their hook events something which
    ++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 sets of configs like
     +so:
     +
     +  [hook "linter"]
     +    event = pre-commit
    -+    command = ~/bin/linter --c
    ++    command = ~/bin/linter --cpp20
     +
    -+In this example, `[hook "linter"]` represents one script - `~/bin/linter --c` -
    -+which can be shared by many repos, and even by many hook events, if appropriate.
    ++In this example, `[hook "linter"]` represents one script - `~/bin/linter
    ++--cpp20` - which can be shared by many repos, and even by many hook events, if
    ++appropriate.
    ++
    ++To add an unrelated hook which runs on a different event, for example a
    ++spell-checker for your commit messages, you would write a configuration like so:
    ++
    ++  [hook "linter"]
    ++    event = pre-commit
    ++    command = ~/bin/linter --cpp20
    ++  [hook "spellcheck"]
    ++    event = commit-msg
    ++    command = ~/bin/spellchecker
    ++
    ++With this config, when you run 'git commit', first `~/bin/linter --cpp20` will
    ++have a chance to check your files to be committed (during the `pre-commit` hook
    ++event`), and then `~/bin/spellchecker` will have a chance to check your commit
    ++message (during the `commit-msg` hook event).
     +
     +Commands are run in the order Git encounters their associated
     +`hook.<name>.event` configs during the configuration parse (see
    @@ Documentation/git-hook.txt: Git is unlikely to use for a native hook later on. F
     +  [hook "linter"]
     +    event = pre-commit
     +    event = pre-push
    -+    command = ~/bin/linter --c
    ++    command = ~/bin/linter --cpp20
     +
    -+With this config, `~/bin/linter --c` would be run by Git before a commit is
    ++With this config, `~/bin/linter --cpp20` would be run by Git before a commit is
     +generated (during `pre-commit`) as well as before a push is performed (during
     +`pre-push`).
     +
    @@ Documentation/git-hook.txt: Git is unlikely to use for a native hook later on. F
     +
     +  [hook "linter"]
     +    event = pre-commit
    -+    command = ~/bin/linter --c
    ++    command = ~/bin/linter --cpp20
     +  [hook "no-leaks"]
     +    event = pre-commit
     +    command = ~/bin/leak-detector
     +
     +With this config, before a commit is generated (during `pre-commit`), Git would
    -+first start `~/bin/linter --c` and second start `~/bin/leak-detector`. It would
    -+evaluate the output of each when deciding whether to proceed with the commit.
    ++first start `~/bin/linter --cpp20` and second start `~/bin/leak-detector`. It
    ++would evaluate the output of each when deciding whether to proceed with the
    ++commit.
     +
     +For a full list of hook events which you can set your `hook.<name>.event` to,
     +and how hooks are invoked during those events, see linkgit:githooks[5].
     +
    ++Git will ignore any `hook.<name>.event` that specifies an event it doesn't
    ++recognize. This is intended so that tools which wrap Git can use the hook
    ++infrastructure to run their own hooks; see <<WRAPPERS>> for more guidance.
    ++
     +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`.
    ++`.git/hooks/<hook-event>`, you can specify it in the config instead by running:
    ++
    ++----
    ++git config 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 hook.my-script.command ~/my-script.sh
    ++git config --add hook.my-script.event pre-commit
    ++----
     +
      SUBCOMMANDS
      -----------
    @@ Documentation/git-hook.txt: Git is unlikely to use for a native hook later on. F
      +
      Any positional arguments to the hook should be passed after an
      optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The
    +@@ Documentation/git-hook.txt: config is not specified, uses the number of CPUs on the current system. Some
    + hooks may be ineligible for parallelization: for example, 'commit-msg' intends
    + hooks modify the commit message body and cannot be parallelized.
    + 
    ++[[WRAPPERS]]
    ++WRAPPERS
    ++--------
    ++
    ++`git hook run` has been designed to make it easy for tools which wrap Git to
    ++configure and execute hooks using the Git hook infrastructure. It is possible to
    ++provide arguments, environment variables (TODO this is missing from reroll TODO),
    ++and stdin via the command line, as well as specifying parallel or series
    ++execution if the user has provided multiple hooks.
    ++
    ++Assuming your wrapper wants to support a hook named "mywrapper-start-tests", you
    ++can have your users specify their hooks like so:
    ++
    ++  [hook "setup-test-dashboard"]
    ++    event = mywrapper-start-tests
    ++    command = ~/mywrapper/setup-dashboard.py --tap
    ++
    ++Then, in your 'mywrapper' tool, you can invoke any users' configured hooks by
    ++running:
    ++
    ++----
    ++git hook run mywrapper-start-tests \
    ++  # providing something to stdin
    ++  --stdin some-tempfile-123 \
    ++  # setting an env var (TODO THIS IS MISSING TODO)
    ++  --env MYWRAPPER_EXECUTION_MODE=foo \
    ++  # execute hooks in serial
    ++  --jobs 1 \
    ++  # plus some arguments of your own...
    ++  -- \
    ++  --testname bar \
    ++  baz
    ++----
    ++
    ++Take care to name your wrapper's hook events in a way which is unlikely to
    ++overlap with Git's native hooks (see linkgit:githooks[5]) - a hook event named
    ++`mywrappertool-validate-commit` is much less likely to be added to native Git
    ++than a hook event named `validate-commit`. If Git begins to use a hook event
    ++named the same thing as your wrapper hook, it may invoke your users' hooks in
    ++unintended and unsupported ways.
    ++
    + CONFIGURATION
    + -------------
    + include::config/hook.txt[]
     
      ## builtin/hook.c ##
     @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
    @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
     +						  : _("hook from hookdir"));
      	}
      
    - 	clear_hook_list(head);
    + cleanup:
     
      ## hook.c ##
     @@ hook.c: static void free_hook(struct hook *ptr)
    + 	if (!ptr)
    + 		return;
    + 
    ++	free(ptr->name);
    + 	free(ptr->feed_pipe_cb_data);
      	free(ptr);
      }
      
    @@ hook.c: static void free_hook(struct hook *ptr)
     +
     +	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;
    ++		to_add = xcalloc(1, sizeof(*to_add));
    ++		to_add->name = xstrdup_or_null(name);
     +	}
     +
     +	list_add_tail(&to_add->list, head);
    @@ hook.c: static void free_hook(struct hook *ptr)
      {
      	struct hook *hook_to_remove = list_entry(to_remove, struct hook, list);
     @@ hook.c: int hook_exists(const char *name)
    + 	return exists;
    + }
      
    - struct hook_config_cb
    - {
    --	struct strbuf *hook_key;
    ++struct hook_config_cb
    ++{
     +	const char *hook_event;
    - 	struct list_head *list;
    - };
    - 
    ++	struct list_head *list;
    ++};
    ++
     +/*
     + * 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.c: int hook_exists(const char *name)
     +	 */
     +	strbuf_add(&subsection_cpy, subsection, subsection_len);
     +
    -+	append_or_move_hook(data->list, strbuf_detach(&subsection_cpy, NULL));
    -+
    ++	append_or_move_hook(data->list, subsection_cpy.buf);
    ++	strbuf_release(&subsection_cpy);
     +
     +	return 0;
     +}
     +
      struct list_head *list_hooks(const char *hookname)
    - {
    - 	if (!known_hook(hookname))
    -@@ hook.c: struct list_head *list_hooks(const char *hookname)
    - struct list_head *list_hooks_gently(const char *hookname)
      {
      	struct list_head *hook_head = xmalloc(sizeof(struct list_head));
     +	struct hook_config_cb cb_data = {
    @@ hook.c: struct list_head *list_hooks(const char *hookname)
      		BUG("null hookname was provided to hook_list()!");
      
     -	if (have_git_dir()) {
    --		const char *hook_path = find_hook_gently(hookname);
    +-		const char *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;
    @@ hook.c: struct list_head *list_hooks(const char *hookname)
     -			list_add_tail(&to_add->list, hook_head);
     -		}
     -	}
    -+	/* 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. The placeholder makes it easier to
     +	 * allocate work in pick_next_hook. */
    -+	if (find_hook_gently(hookname))
    ++	if (find_hook(hookname))
     +		append_or_move_hook(hook_head, NULL);
      
      	return hook_head;
    @@ hook.c: 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. */
    ++	/*
    ++	 * to enable oneliners, let config-specified hooks run in shell.
    ++	 * config-specified hooks have a name.
    ++	 */
     +	cp->use_shell = !!run_me->name;
     +
      	/* add command */
    @@ hook.c: static int pick_next_hook(struct child_process *cp,
     +		}
     +
     +		strvec_push(&cp->args, command);
    ++		free(command);
    ++		strbuf_release(&cmd_key);
     +	} 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.
    ++		 * whether the hook name is known or not. Validation was
    ++		 * performed earlier in list_hooks().
     +		 */
    -+		hook_path = find_hook_gently(hook_cb->hook_name);
    ++		hook_path = find_hook(hook_cb->hook_name);
     +		if (!hook_path)
     +			BUG("hookdir hook in hook list but no hookdir hook present in filesystem");
     +
    @@ hook.c: static int notify_start_failure(struct strbuf *out,
      }
     
      ## hook.h ##
    -@@ hook.h: int hook_exists(const char *hookname);
    +@@
      
      struct hook {
      	struct list_head list;
    @@ hook.h: int hook_exists(const char *hookname);
     +	 * The friendly name of the hook. NULL indicates the hook is from the
     +	 * hookdir.
     +	 */
    -+	const char *name;
    ++	char *name;
      
      	/*
      	 * Use this to keep state for your feed_pipe_fn if you are using
     
      ## t/t1800-hook.sh ##
     @@
    - #!/bin/bash
    + #!/bin/sh
      
     -test_description='git-hook command'
     +test_description='git-hook command and config-managed multihooks'
    @@ t/t1800-hook.sh
      . ./test-lib.sh
      
     +setup_hooks () {
    ++	test_config hook.ghi.command "/path/ghi"
     +	test_config hook.ghi.event pre-commit --add
    -+	test_config hook.ghi.command "/path/ghi" --add
    ++	test_config hook.ghi.event test-hook --add
    ++	test_config_global hook.def.command "/path/def"
     +	test_config_global hook.def.event pre-commit --add
    -+	test_config_global hook.def.command "/path/def" --add
     +}
     +
     +setup_hookdir () {
    @@ t/t1800-hook.sh
      	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
    - '
    + 	test_expect_code 129 git hook list &&
    + 	test_expect_code 129 git hook list -h &&
    +@@ t/t1800-hook.sh: test_expect_success 'git hook list: does-not-exist hook' '
    + 
    + test_expect_success 'git hook list: existing hook' '
    + 	cat >expect <<-\EOF &&
    +-	.git/hooks/test-hook
    ++	hook from hookdir
    + 	EOF
    + 	git hook list test-hook >actual &&
    + 	test_cmp expect actual
     @@ t/t1800-hook.sh: test_expect_success 'stdin to hooks' '
      	test_cmp expect actual
      '
    @@ t/t1800-hook.sh: test_expect_success 'stdin to hooks' '
     +test_expect_success 'git hook list orders by config order' '
     +	setup_hooks &&
     +
    -+	cat >expected <<-EOF &&
    ++	cat >expected <<-\EOF &&
     +	def
     +	ghi
     +	EOF
    @@ t/t1800-hook.sh: test_expect_success 'stdin to hooks' '
     +	# configuring it locally.
     +	test_config hook.def.event "pre-commit" --add &&
     +
    -+	cat >expected <<-EOF &&
    ++	cat >expected <<-\EOF &&
     +	ghi
     +	def
     +	EOF
    @@ t/t1800-hook.sh: test_expect_success 'stdin to hooks' '
     +	test_cmp expected actual
     +'
     +
    ++test_expect_success 'hook can be configured for multiple events' '
    ++	setup_hooks &&
    ++
    ++	# 'ghi' should be included in both 'pre-commit' and 'test-hook'
    ++	git hook list pre-commit >actual &&
    ++	grep "ghi" actual &&
    ++	git hook list test-hook >actual &&
    ++	grep "ghi" actual
    ++'
    ++
     +test_expect_success 'git hook list shows hooks from the hookdir' '
     +	setup_hookdir &&
     +
    -+	cat >expected <<-EOF &&
    ++	cat >expected <<-\EOF &&
     +	hook from hookdir
     +	EOF
     +
    @@ t/t1800-hook.sh: test_expect_success 'stdin to hooks' '
     +'
     +
     +test_expect_success 'inline hook definitions resolve paths' '
    -+	write_script sample-hook.sh <<-EOF &&
    ++	write_script sample-hook.sh <<-\EOF &&
     +	echo \"Sample Hook\"
     +	EOF
     +
    @@ t/t1800-hook.sh: test_expect_success 'stdin to hooks' '
     +	test_config hook.stdin-b.event "test-hook" --add &&
     +	test_config hook.stdin-b.command "xargs -P1 -I% echo b%" --add &&
     +
    -+	cat >input <<-EOF &&
    ++	cat >input <<-\EOF &&
     +	1
     +	2
     +	3
     +	EOF
     +
    -+	cat >expected <<-EOF &&
    ++	cat >expected <<-\EOF &&
     +	a1
     +	a2
     +	a3
    @@ t/t1800-hook.sh: test_expect_success 'stdin to hooks' '
     +	echo 3
     +	EOF
     +
    -+	cat >expected <<-EOF &&
    ++	cat >expected <<-\EOF &&
     +	1
     +	2
     +	3
    @@ t/t1800-hook.sh: test_expect_success 'stdin to hooks' '
     +
     +	rm -rf .git/hooks
     +'
    ++
    ++test_expect_success 'rejects hooks with no commands configured' '
    ++	test_config hook.broken.event "test-hook" &&
    ++
    ++	echo broken >expected &&
    ++	git hook list test-hook >actual &&
    ++	test_cmp expected actual &&
    ++	test_must_fail git hook run test-hook
    ++'
    ++
      test_done
6:  30ffe98601e ! 5:  5d5e9726fd8 hook: allow out-of-repo 'git hook' invocations
    @@ git.c: static struct cmd_struct commands[] = {
      	{ "init-db", cmd_init_db },
     
      ## hook.c ##
    -@@ hook.c: struct list_head *list_hooks_gently(const char *hookname)
    +@@ hook.c: struct list_head *list_hooks(const char *hookname)
      
      	/* Add the hook from the hookdir. The placeholder makes it easier to
      	 * allocate work in pick_next_hook. */
    --	if (find_hook_gently(hookname))
    -+	if (have_git_dir() && find_hook_gently(hookname))
    +-	if (find_hook(hookname))
    ++	if (have_git_dir() && find_hook(hookname))
      		append_or_move_hook(hook_head, NULL);
      
      	return hook_head;
     
      ## t/t1800-hook.sh ##
    -@@ t/t1800-hook.sh: test_expect_success 'git hook run -- pass arguments' '
    +@@ t/t1800-hook.sh: test_expect_success 'git hook list: existing hook' '
      	test_cmp expect actual
      '