[PATCH v4 0/5] config-based hooks restarted
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Sep 9, 2021, 12:41 UTC
- Message-ID
- <cover-v4-0.5-00000000000-20210909T122802Z-avarab@gmail.com>
- In-Reply-To
- <20210819033450.3382652-1-emilyshaffer@google.com>
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/87sfyfgtfh.fsf@evledraar.gmail.com/ 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/cover-v5-00.36-00000000000-20210902T125110Z-avarab@gmail.com/ 2. https://lore.kernel.org/git/20210819033450.3382652-1-emilyshaffer@google.com/
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 <emilyshaffer@google.com>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## 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 <emilyshaffer@google.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ ## 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 <emilyshaffer@google.com>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## 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 <emilyshaffer@google.com>
@@ 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
'
-- 2.33.0.867.g88ec4638586