[PATCH 00/10] config-hook cleanups and two small 'git hook list' features

[PATCH 00/10] config-hook cleanups and two small 'git hook list' features

[Adrian Ratiu]

Hello everyone,

As promised I've spun-off v3 of the config series [1] into its own standalone
patch series after v2 landed in next.

This is mostly minor cleanups and refactorings + two minor feature additions
to `git hook list`, which resulted from the previous review discussions:

1. The ability to show the config scope (--show-scope).
2. The ability to show which hooks are disabled.

This is based on next because the config hooks support is only in next.

I have pushed the branch to Github [2] and provided a clean CI run [3].

Big thank-you's to all who contributed to this up to now,
Adrian

1: https://lore.kernel.org/git/20260204165126.1548805-1-adrian.ratiu@collabora.com/T/#mafc63b5b32a78e48a703cccca3b7d6d9fe668fa2
2: https://github.com/10ne1/git/tree/dev/aratiu/config-cleanups-v1
3: https://github.com/10ne1/git/actions/runs/22833022192

Adrian Ratiu (10):
  hook: move unsorted_string_list_remove() to string-list.[ch]
  hook: fix minor style issues
  hook: rename cb_data_free/alloc -> hook_data_free/alloc
  hook: detect & emit two more bugs
  hook: replace hook_list_clear() -> string_list_clear_func()
  hook: make consistent use of friendly-name in docs
  t1800: add test to verify hook execution ordering
  hook: refactor hook_config_cache from strmap to named struct
  hook: show config scope in git hook list
  hook: show disabled hooks in "git hook list"

 Documentation/config/hook.adoc |  30 ++---
 Documentation/git-hook.adoc    |  15 ++-
 builtin/hook.c                 |  28 +++--
 builtin/receive-pack.c         |  11 +-
 hook.c                         | 220 +++++++++++++++++++++------------
 hook.h                         |  39 ++++--
 refs.c                         |   3 +-
 repository.h                   |   3 +-
 string-list.c                  |   9 ++
 string-list.h                  |   8 ++
 t/t1800-hook.sh                |  83 ++++++++++++-
 transport.c                    |   3 +-
 12 files changed, 321 insertions(+), 131 deletions(-)

-- 
2.52.0.732.gb351b5166d.dirty

[PATCH 01/10] hook: move unsorted_string_list_remove() to string-list.[ch]

[Adrian Ratiu]

Move the convenience wrapper from hook to string-list since
it's a more suitable place. Add a doc comment to the header.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c        | 8 --------
 string-list.c | 9 +++++++++
 string-list.h | 8 ++++++++
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/hook.c b/hook.c
index 2c8252b2c4..313a6b9937 100644
--- a/hook.c
+++ b/hook.c
@@ -110,14 +110,6 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	string_list_append(hook_list, hook_path)->util = h;
 }
 
-static void unsorted_string_list_remove(struct string_list *list,
-					const char *str)
-{
-	struct string_list_item *item = unsorted_string_list_lookup(list, str);
-	if (item)
-		unsorted_string_list_delete_item(list, item - list->items, 0);
-}
-
 /*
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
diff --git a/string-list.c b/string-list.c
index fffa2ad4b6..d260b873c8 100644
--- a/string-list.c
+++ b/string-list.c
@@ -281,6 +281,15 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_
 	list->nr--;
 }
 
+void unsorted_string_list_remove(struct string_list *list, const char *str,
+				 int free_util)
+{
+	struct string_list_item *item = unsorted_string_list_lookup(list, str);
+	if (item)
+		unsorted_string_list_delete_item(list, item - list->items,
+						 free_util);
+}
+
 /*
  * append a substring [p..end] to list; return number of things it
  * appended to the list.
diff --git a/string-list.h b/string-list.h
index 3ad862a187..b86ee7c099 100644
--- a/string-list.h
+++ b/string-list.h
@@ -265,6 +265,14 @@ struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
  */
 void unsorted_string_list_delete_item(struct string_list *list, int i, int free_util);
 
+/**
+ * Remove the first item matching `str` from an unsorted string_list.
+ * No-op if `str` is not found. If `free_util` is non-zero, the `util`
+ * pointer of the removed item is freed before deletion.
+ */
+void unsorted_string_list_remove(struct string_list *list, const char *str,
+				 int free_util);
+
 /**
  * Split string into substrings on characters in `delim` and append the
  * substrings to `list`.  The input string is not modified.
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH 01/10] hook: move unsorted_string_list_remove() to string-list.[ch]

[SZEDER Gábor]

On Mon, Mar 09, 2026 at 02:54:07AM +0200, Adrian Ratiu wrote:
> Move the convenience wrapper from hook to string-list since
> it's a more suitable place. Add a doc comment to the header.

unsorted_string_list_remove() in string-list has a 'free_util'
parameter that didn't exist in its original version in 'hook.c', but
it's not mentioned in the commit message.
Furthermore, none of the function's callsites are adjusted to the new
parameter, and the build fails with:

  hook.c: In function ‘hook_config_lookup_all’:
  hook.c:151:33: error: too few arguments to function ‘unsorted_string_list_remove’
    151 |                                 unsorted_string_list_remove(e->value, hook_name);
        |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  In file included from hook.h:5,
                   from hook.c:5:
  string-list.h:273:6: note: declared here
    273 | void unsorted_string_list_remove(struct string_list *list, const char *str,
        |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  hook.c:163:25: error: too few arguments to function ‘unsorted_string_list_remove’
    163 |                         unsorted_string_list_remove(hooks, hook_name);
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  string-list.h:273:6: note: declared here
    273 | void unsorted_string_list_remove(struct string_list *list, const char *str,
        |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  hook.c:180:25: error: too few arguments to function ‘unsorted_string_list_remove’
    180 |                         unsorted_string_list_remove(&data->disabled_hooks,
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  string-list.h:273:6: note: declared here
    273 | void unsorted_string_list_remove(struct string_list *list, const char *str,
        |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  make: *** [Makefile:2917: hook.o] Error 1


> Suggested-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
>  hook.c        | 8 --------
>  string-list.c | 9 +++++++++
>  string-list.h | 8 ++++++++
>  3 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/hook.c b/hook.c
> index 2c8252b2c4..313a6b9937 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -110,14 +110,6 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
>  	string_list_append(hook_list, hook_path)->util = h;
>  }
>  
> -static void unsorted_string_list_remove(struct string_list *list,
> -					const char *str)
> -{
> -	struct string_list_item *item = unsorted_string_list_lookup(list, str);
> -	if (item)
> -		unsorted_string_list_delete_item(list, item - list->items, 0);
> -}
> -
>  /*
>   * Callback struct to collect all hook.* keys in a single config pass.
>   * commands: friendly-name to command map.
> diff --git a/string-list.c b/string-list.c
> index fffa2ad4b6..d260b873c8 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -281,6 +281,15 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_
>  	list->nr--;
>  }
>  
> +void unsorted_string_list_remove(struct string_list *list, const char *str,
> +				 int free_util)
> +{
> +	struct string_list_item *item = unsorted_string_list_lookup(list, str);
> +	if (item)
> +		unsorted_string_list_delete_item(list, item - list->items,
> +						 free_util);
> +}
> +
>  /*
>   * append a substring [p..end] to list; return number of things it
>   * appended to the list.
> diff --git a/string-list.h b/string-list.h
> index 3ad862a187..b86ee7c099 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -265,6 +265,14 @@ struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
>   */
>  void unsorted_string_list_delete_item(struct string_list *list, int i, int free_util);
>  
> +/**
> + * Remove the first item matching `str` from an unsorted string_list.
> + * No-op if `str` is not found. If `free_util` is non-zero, the `util`
> + * pointer of the removed item is freed before deletion.
> + */
> +void unsorted_string_list_remove(struct string_list *list, const char *str,
> +				 int free_util);
> +
>  /**
>   * Split string into substrings on characters in `delim` and append the
>   * substrings to `list`.  The input string is not modified.
> -- 
> 2.52.0.732.gb351b5166d.dirty
> 

Re: [PATCH 01/10] hook: move unsorted_string_list_remove() to string-list.[ch]

[Adrian Ratiu]

On Tue, 10 Mar 2026, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Mon, Mar 09, 2026 at 02:54:07AM +0200, Adrian Ratiu wrote:
>> Move the convenience wrapper from hook to string-list since
>> it's a more suitable place. Add a doc comment to the header.
>
> unsorted_string_list_remove() in string-list has a 'free_util'
> parameter that didn't exist in its original version in 'hook.c', but
> it's not mentioned in the commit message.
> Furthermore, none of the function's callsites are adjusted to the new
> parameter, and the build fails with:
>
>   hook.c: In function ‘hook_config_lookup_all’:
>   hook.c:151:33: error: too few arguments to function ‘unsorted_string_list_remove’
>     151 |                                 unsorted_string_list_remove(e->value, hook_name);
>         |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   In file included from hook.h:5,
>                    from hook.c:5:
>   string-list.h:273:6: note: declared here
>     273 | void unsorted_string_list_remove(struct string_list *list, const char *str,
>         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   hook.c:163:25: error: too few arguments to function ‘unsorted_string_list_remove’
>     163 |                         unsorted_string_list_remove(hooks, hook_name);
>         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   string-list.h:273:6: note: declared here
>     273 | void unsorted_string_list_remove(struct string_list *list, const char *str,
>         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   hook.c:180:25: error: too few arguments to function ‘unsorted_string_list_remove’
>     180 |                         unsorted_string_list_remove(&data->disabled_hooks,
>         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   string-list.h:273:6: note: declared here
>     273 | void unsorted_string_list_remove(struct string_list *list, const char *str,
>         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   make: *** [Makefile:2917: hook.o] Error 1

Excellent catch, thanks!

I have been moving a lot of code around for this series and I ended up
updating the call sites in a later commit, causing this breakage.

Will fix in v2 as well as document the introduction of the new parameter
in the commit message.

[PATCH 02/10] hook: fix minor style issues

[Adrian Ratiu]

Fix some minor style nits pointed by Patrick and Junio:
  * Use CALLOC_ARRAY instead of xcalloc.
  * Init struct members during declaration.
  * Simplify if condition boolean logic.
  * Missing curly braces in if/else stmts.
  * Unnecessary header includes.
  * Capitalization in error/warn messages.
  * Comment spelling: free'd -> freed.

These contain no logic changes, the code behaves the same as before.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c         |  6 ++----
 builtin/receive-pack.c | 11 +++++++----
 hook.c                 | 25 +++++++++++++------------
 refs.c                 |  3 ++-
 t/t1800-hook.sh        |  2 +-
 transport.c            |  3 ++-
 6 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index 83020dfb4f..c622a7399c 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -5,8 +5,6 @@
 #include "gettext.h"
 #include "hook.h"
 #include "parse-options.h"
-#include "strvec.h"
-#include "abspath.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
 	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
@@ -51,7 +49,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	 * 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."),
+		usage_msg_opt(_("you must specify a hook event name to list."),
 			      builtin_hook_list_usage, list_options);
 
 	hookname = argv[0];
@@ -59,7 +57,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	head = list_hooks(repo, hookname, NULL);
 
 	if (!head->nr) {
-		warning(_("No hooks found for event '%s'"), hookname);
+		warning(_("no hooks found for event '%s'"), hookname);
 		ret = 1; /* no hooks found */
 		goto cleanup;
 	}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d6225df890..4b63ccdfa3 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -904,7 +904,8 @@ static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_
 static void *receive_hook_feed_state_alloc(void *feed_pipe_ctx)
 {
 	struct receive_hook_feed_state *init_state = feed_pipe_ctx;
-	struct receive_hook_feed_state *data = xcalloc(1, sizeof(*data));
+	struct receive_hook_feed_state *data;
+	CALLOC_ARRAY(data, 1);
 	data->report = init_state->report;
 	data->cmd = init_state->cmd;
 	data->skip_broken = init_state->skip_broken;
@@ -928,7 +929,11 @@ static int run_receive_hook(struct command *commands,
 {
 	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
 	struct command *iter = commands;
-	struct receive_hook_feed_state feed_init_state = { 0 };
+	struct receive_hook_feed_state feed_init_state = {
+		.cmd = commands,
+		.skip_broken = skip_broken,
+		.buf = STRBUF_INIT,
+	};
 	struct async sideband_async;
 	int sideband_async_started = 0;
 	int saved_stderr = -1;
@@ -961,8 +966,6 @@ static int run_receive_hook(struct command *commands,
 	prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started);
 
 	/* set up stdin callback */
-	feed_init_state.cmd = commands;
-	feed_init_state.skip_broken = skip_broken;
 	opt.feed_pipe_ctx = &feed_init_state;
 	opt.feed_pipe = feed_receive_hook_cb;
 	opt.feed_pipe_cb_data_alloc = receive_hook_feed_state_alloc;
diff --git a/hook.c b/hook.c
index 313a6b9937..c5a6788dd5 100644
--- a/hook.c
+++ b/hook.c
@@ -57,9 +57,9 @@ static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
 	if (!h)
 		return;
 
-	if (h->kind == HOOK_TRADITIONAL)
+	if (h->kind == HOOK_TRADITIONAL) {
 		free((void *)h->u.traditional.path);
-	else if (h->kind == HOOK_CONFIGURED) {
+	} else if (h->kind == HOOK_CONFIGURED) {
 		free((void *)h->u.configured.friendly_name);
 		free((void *)h->u.configured.command);
 	}
@@ -91,7 +91,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	if (!hook_path)
 		return;
 
-	h = xcalloc(1, sizeof(struct hook));
+	CALLOC_ARRAY(h, 1);
 
 	/*
 	 * If the hook is to run in a specific dir, a relative path can
@@ -154,7 +154,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 				strmap_get(&data->event_hooks, value);
 
 			if (!hooks) {
-				hooks = xcalloc(1, sizeof(*hooks));
+				CALLOC_ARRAY(hooks, 1);
 				string_list_init_dup(hooks);
 				strmap_put(&data->event_hooks, value, hooks);
 			}
@@ -227,7 +227,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 	/* Construct the cache from parsed configs. */
 	strmap_for_each_entry(&cb_data.event_hooks, &iter, e) {
 		struct string_list *hook_names = e->value;
-		struct string_list *hooks = xcalloc(1, sizeof(*hooks));
+		struct string_list *hooks;
+		CALLOC_ARRAY(hooks, 1);
 
 		string_list_init_dup(hooks);
 
@@ -281,7 +282,7 @@ static struct strmap *get_hook_config_cache(struct repository *r)
 		 * it just once on the first call.
 		 */
 		if (!r->hook_config_cache) {
-			r->hook_config_cache = xcalloc(1, sizeof(*cache));
+			CALLOC_ARRAY(r->hook_config_cache, 1);
 			strmap_init(r->hook_config_cache);
 			build_hook_config_map(r, r->hook_config_cache);
 		}
@@ -289,9 +290,9 @@ static struct strmap *get_hook_config_cache(struct repository *r)
 	} else {
 		/*
 		 * Out-of-repo calls (no gitdir) allocate and return a temporary
-		 * map cache which gets free'd immediately by the caller.
+		 * cache which gets freed immediately by the caller.
 		 */
-		cache = xcalloc(1, sizeof(*cache));
+		CALLOC_ARRAY(cache, 1);
 		strmap_init(cache);
 		build_hook_config_map(r, cache);
 	}
@@ -311,7 +312,8 @@ static void list_hooks_add_configured(struct repository *r,
 	for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) {
 		const char *friendly_name = configured_hooks->items[i].string;
 		const char *command = configured_hooks->items[i].util;
-		struct hook *hook = xcalloc(1, sizeof(struct hook));
+		struct hook *hook;
+		CALLOC_ARRAY(hook, 1);
 
 		if (options && options->feed_pipe_cb_data_alloc)
 			hook->feed_pipe_cb_data =
@@ -343,7 +345,7 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 	if (!hookname)
 		BUG("null hookname was provided to hook_list()!");
 
-	hook_head = xmalloc(sizeof(struct string_list));
+	CALLOC_ARRAY(hook_head, 1);
 	string_list_init_dup(hook_head);
 
 	/* Add hooks from the config, e.g. hook.myhook.event = pre-commit */
@@ -493,8 +495,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
 	 * Ensure cb_data copy and free functions are either provided together,
 	 * or neither one is provided.
 	 */
-	if ((options->feed_pipe_cb_data_alloc && !options->feed_pipe_cb_data_free) ||
-	    (!options->feed_pipe_cb_data_alloc && options->feed_pipe_cb_data_free))
+	if (!options->feed_pipe_cb_data_alloc != !options->feed_pipe_cb_data_free)
 		BUG("feed_pipe_cb_data_alloc and feed_pipe_cb_data_free must be set together");
 
 	if (options->invoked_hook)
diff --git a/refs.c b/refs.c
index 6fb8f9d10c..33c7961802 100644
--- a/refs.c
+++ b/refs.c
@@ -2591,7 +2591,8 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
 
 static void *transaction_feed_cb_data_alloc(void *feed_pipe_ctx UNUSED)
 {
-	struct transaction_feed_cb_data *data = xmalloc(sizeof(*data));
+	struct transaction_feed_cb_data *data;
+	CALLOC_ARRAY(data, 1);
 	strbuf_init(&data->buf, 0);
 	data->index = 0;
 	return data;
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index b1583e9ef9..952bf97b86 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -34,7 +34,7 @@ test_expect_success 'git hook usage' '
 
 test_expect_success 'git hook list: nonexistent hook' '
 	cat >stderr.expect <<-\EOF &&
-	warning: No hooks found for event '\''test-hook'\''
+	warning: no hooks found for event '\''test-hook'\''
 	EOF
 	test_expect_code 1 git hook list test-hook 2>stderr.actual &&
 	test_cmp stderr.expect stderr.actual
diff --git a/transport.c b/transport.c
index 107f4fa5dc..56a4015389 100644
--- a/transport.c
+++ b/transport.c
@@ -1360,7 +1360,8 @@ static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void
 
 static void *pre_push_hook_data_alloc(void *feed_pipe_ctx)
 {
-	struct feed_pre_push_hook_data *data = xmalloc(sizeof(*data));
+	struct feed_pre_push_hook_data *data;
+	CALLOC_ARRAY(data, 1);
 	strbuf_init(&data->buf, 0);
 	data->refs = (struct ref *)feed_pipe_ctx;
 	return data;
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH 02/10] hook: fix minor style issues

[Eric Sunshine]

On Sun, Mar 8, 2026 at 8:54 PM Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
> Fix some minor style nits pointed by Patrick and Junio:
>   ...
>   * Capitalization in error/warn messages.
>   ...
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> diff --git a/builtin/hook.c b/builtin/hook.c
> @@ -51,7 +49,7 @@ static int list(int argc, const char **argv, const char *prefix,
>         if (argc != 1)
> -               usage_msg_opt(_("You must specify a hook event name to list."),
> +               usage_msg_opt(_("you must specify a hook event name to list."),
>                               builtin_hook_list_usage, list_options);

You will also want to drop the full-stop (".") from the message
according to style guidelines.

[PATCH 03/10] hook: rename cb_data_free/alloc -> hook_data_free/alloc

[Adrian Ratiu]

Rename the hook callback function types to use the hook prefix.

This is a style fix with no logic changes.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c |  4 ++--
 hook.h | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hook.c b/hook.c
index c5a6788dd5..bc1c45b16d 100644
--- a/hook.c
+++ b/hook.c
@@ -52,7 +52,7 @@ const char *find_hook(struct repository *r, const char *name)
 	return path.buf;
 }
 
-static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
+static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
 {
 	if (!h)
 		return;
@@ -70,7 +70,7 @@ static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
 	free(h);
 }
 
-void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free)
+void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free)
 {
 	struct string_list_item *item;
 
diff --git a/hook.h b/hook.h
index e949f5d488..e514c1b45b 100644
--- a/hook.h
+++ b/hook.h
@@ -43,8 +43,8 @@ struct hook {
 	void *feed_pipe_cb_data;
 };
 
-typedef void (*cb_data_free_fn)(void *data);
-typedef void *(*cb_data_alloc_fn)(void *init_ctx);
+typedef void (*hook_data_free_fn)(void *data);
+typedef void *(*hook_data_alloc_fn)(void *init_ctx);
 
 struct run_hooks_opt
 {
@@ -132,14 +132,14 @@ struct run_hooks_opt
 	 *
 	 * The `feed_pipe_ctx` pointer can be used to pass initialization data.
 	 */
-	cb_data_alloc_fn feed_pipe_cb_data_alloc;
+	hook_data_alloc_fn feed_pipe_cb_data_alloc;
 
 	/**
 	 * Called to free the memory initialized by `feed_pipe_cb_data_alloc`.
 	 *
 	 * Must always be provided when `feed_pipe_cb_data_alloc` is provided.
 	 */
-	cb_data_free_fn feed_pipe_cb_data_free;
+	hook_data_free_fn feed_pipe_cb_data_free;
 };
 
 #define RUN_HOOKS_OPT_INIT { \
@@ -189,7 +189,7 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
  * Frees the memory allocated for the hook list, including the `struct hook`
  * items and their internal state.
  */
-void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free);
+void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
 
 /**
  * Frees the hook configuration cache stored in `struct repository`.
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH 03/10] hook: rename cb_data_free/alloc -> hook_data_free/alloc

[Patrick Steinhardt]

On Mon, Mar 09, 2026 at 02:54:09AM +0200, Adrian Ratiu wrote:
> diff --git a/hook.h b/hook.h
> index e949f5d488..e514c1b45b 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -43,8 +43,8 @@ struct hook {
>  	void *feed_pipe_cb_data;
>  };
>  
> -typedef void (*cb_data_free_fn)(void *data);
> -typedef void *(*cb_data_alloc_fn)(void *init_ctx);
> +typedef void (*hook_data_free_fn)(void *data);
> +typedef void *(*hook_data_alloc_fn)(void *init_ctx);
>  
>  struct run_hooks_opt
>  {

Ah, this here is also a small style issue that you may want to fix up in
the preceding patch. For structures, the curly brace goes on the same
line.

Patrick

Re: [PATCH 03/10] hook: rename cb_data_free/alloc -> hook_data_free/alloc

[Adrian Ratiu]

On Wed, 11 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Mon, Mar 09, 2026 at 02:54:09AM +0200, Adrian Ratiu wrote:
>> diff --git a/hook.h b/hook.h
>> index e949f5d488..e514c1b45b 100644
>> --- a/hook.h
>> +++ b/hook.h
>> @@ -43,8 +43,8 @@ struct hook {
>>  	void *feed_pipe_cb_data;
>>  };
>>  
>> -typedef void (*cb_data_free_fn)(void *data);
>> -typedef void *(*cb_data_alloc_fn)(void *init_ctx);
>> +typedef void (*hook_data_free_fn)(void *data);
>> +typedef void *(*hook_data_alloc_fn)(void *init_ctx);
>>  
>>  struct run_hooks_opt
>>  {
>
> Ah, this here is also a small style issue that you may want to fix up in
> the preceding patch. For structures, the curly brace goes on the same
> line.

Ack, will fix this in v2 together with the rest of style issues pointed
out. Thanks!

[PATCH 04/10] hook: detect & emit two more bugs

[Adrian Ratiu]

Trigger a bug when an unknown hook type is encountered while
setting up hook execution.

Also issue a bug if a configured hook is enabled without a cmd.

Mostly useful for defensive coding.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hook.c b/hook.c
index bc1c45b16d..b8ed4d79e2 100644
--- a/hook.c
+++ b/hook.c
@@ -408,7 +408,11 @@ static int pick_next_hook(struct child_process *cp,
 	} else if (h->kind == HOOK_CONFIGURED) {
 		/* to enable oneliners, let config-specified hooks run in shell. */
 		cp->use_shell = true;
+		if (!h->u.configured.command)
+			BUG("non-disabled HOOK_CONFIGURED hook has no command");
 		strvec_push(&cp->args, h->u.configured.command);
+	} else {
+		BUG("unknown hook kind");
 	}
 
 	if (!cp->args.nr)
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH 05/10] hook: replace hook_list_clear() -> string_list_clear_func()

[Adrian Ratiu]

Replace the custom function with string_list_clear_func() which
is a more common pattern for clearing a string_list.

To be able to do this, rework hook_clear() into hook_free(), so
it can be passed to string_list_clear_func().

A slight complication is the need to keep a copy of the internal
cb data free() pointer, however I think it's worth it since the
API becomes cleaner, e.g. no more calls with NULL function args
like hook_list_clear(hooks, NULL).

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c |  2 +-
 hook.c         | 50 +++++++++++++++++++++++++++++---------------------
 hook.h         | 20 ++++++++++++++------
 3 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index c622a7399c..8fc647a4de 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -78,7 +78,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	}
 
 cleanup:
-	hook_list_clear(head, NULL);
+	string_list_clear_func(head, hook_free);
 	free(head);
 	return ret;
 }
diff --git a/hook.c b/hook.c
index b8ed4d79e2..f6bb1999ae 100644
--- a/hook.c
+++ b/hook.c
@@ -52,8 +52,14 @@ const char *find_hook(struct repository *r, const char *name)
 	return path.buf;
 }
 
-static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
+/*
+ * Frees a struct hook stored as the util pointer of a string_list_item.
+ * Suitable for use as a string_list_clear_func_t callback.
+ */
+void hook_free(void *p, const char *str UNUSED)
 {
+	struct hook *h = p;
+
 	if (!h)
 		return;
 
@@ -64,22 +70,12 @@ static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
 		free((void *)h->u.configured.command);
 	}
 
-	if (cb_data_free)
-		cb_data_free(h->feed_pipe_cb_data);
+	if (h->data_free && h->feed_pipe_cb_data)
+		h->data_free(h->feed_pipe_cb_data);
 
 	free(h);
 }
 
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free)
-{
-	struct string_list_item *item;
-
-	for_each_string_list_item(item, hooks)
-		hook_clear(item->util, cb_data_free);
-
-	string_list_clear(hooks, 0);
-}
-
 /* Helper to detect and add default "traditional" hooks from the hookdir. */
 static void list_hooks_add_default(struct repository *r, const char *hookname,
 				   struct string_list *hook_list,
@@ -100,9 +96,15 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	if (options && options->dir)
 		hook_path = absolute_path(hook_path);
 
-	/* Setup per-hook internal state cb data */
-	if (options && options->feed_pipe_cb_data_alloc)
+	/*
+	 * Setup per-hook internal state callback data.
+	 * When provided, the alloc/free callbacks are always provided
+	 * together, so use them to alloc/free the internal hook state.
+	 */
+	if (options && options->feed_pipe_cb_data_alloc) {
 		h->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx);
+		h->data_free = options->feed_pipe_cb_data_free;
+	}
 
 	h->kind = HOOK_TRADITIONAL;
 	h->u.traditional.path = xstrdup(hook_path);
@@ -148,7 +150,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			struct strmap_entry *e;
 
 			strmap_for_each_entry(&data->event_hooks, &iter, e)
-				unsorted_string_list_remove(e->value, hook_name);
+				unsorted_string_list_remove(e->value, hook_name, 0);
 		} else {
 			struct string_list *hooks =
 				strmap_get(&data->event_hooks, value);
@@ -160,7 +162,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			}
 
 			/* Re-insert if necessary to preserve last-seen order. */
-			unsorted_string_list_remove(hooks, hook_name);
+			unsorted_string_list_remove(hooks, hook_name, 0);
 			string_list_append(hooks, hook_name);
 		}
 	} else if (!strcmp(subkey, "command")) {
@@ -178,7 +180,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			break;
 		case 1: /* enabled: undo a prior disabled entry */
 			unsorted_string_list_remove(&data->disabled_hooks,
-						    hook_name);
+						    hook_name, 0);
 			break;
 		default:
 			break; /* ignore unrecognised values */
@@ -315,10 +317,16 @@ static void list_hooks_add_configured(struct repository *r,
 		struct hook *hook;
 		CALLOC_ARRAY(hook, 1);
 
-		if (options && options->feed_pipe_cb_data_alloc)
+		/*
+		 * When provided, the alloc/free callbacks are always provided
+		 * together, so use them to alloc/free the internal hook state.
+		 */
+		if (options && options->feed_pipe_cb_data_alloc) {
 			hook->feed_pipe_cb_data =
 				options->feed_pipe_cb_data_alloc(
 					options->feed_pipe_ctx);
+			hook->data_free = options->feed_pipe_cb_data_free;
+		}
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
@@ -361,7 +369,7 @@ int hook_exists(struct repository *r, const char *name)
 {
 	struct string_list *hooks = list_hooks(r, name, NULL);
 	int exists = hooks->nr > 0;
-	hook_list_clear(hooks, NULL);
+	string_list_clear_func(hooks, hook_free);
 	free(hooks);
 	return exists;
 }
@@ -515,7 +523,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
 	run_processes_parallel(&opts);
 	ret = cb_data.rc;
 cleanup:
-	hook_list_clear(cb_data.hook_command_list, options->feed_pipe_cb_data_free);
+	string_list_clear_func(cb_data.hook_command_list, hook_free);
 	free(cb_data.hook_command_list);
 	run_hooks_opt_clear(options);
 	return ret;
diff --git a/hook.h b/hook.h
index e514c1b45b..168c6495a4 100644
--- a/hook.h
+++ b/hook.h
@@ -7,6 +7,9 @@
 
 struct repository;
 
+typedef void (*hook_data_free_fn)(void *data);
+typedef void *(*hook_data_alloc_fn)(void *init_ctx);
+
 /**
  * Represents a hook command to be run.
  * Hooks can be:
@@ -41,10 +44,15 @@ struct hook {
 	 * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it.
 	 */
 	void *feed_pipe_cb_data;
-};
 
-typedef void (*hook_data_free_fn)(void *data);
-typedef void *(*hook_data_alloc_fn)(void *init_ctx);
+	/**
+	 * Callback to free `feed_pipe_cb_data`.
+	 *
+	 * It is called automatically and points to the `feed_pipe_cb_data_free`
+	 * provided via the `run_hook_opt` parameter.
+	 */
+	hook_data_free_fn data_free;
+};
 
 struct run_hooks_opt
 {
@@ -186,10 +194,10 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 			       struct run_hooks_opt *options);
 
 /**
- * Frees the memory allocated for the hook list, including the `struct hook`
- * items and their internal state.
+ * Frees a struct hook stored as the util pointer of a string_list_item.
+ * Suitable for use as a string_list_clear_func_t callback.
  */
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
+void hook_free(void *p, const char *str UNUSED);
 
 /**
  * Frees the hook configuration cache stored in `struct repository`.
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH 05/10] hook: replace hook_list_clear() -> string_list_clear_func()

[Eric Sunshine]

On Sun, Mar 8, 2026 at 8:55 PM Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
> Replace the custom function with string_list_clear_func() which
> is a more common pattern for clearing a string_list.
>
> To be able to do this, rework hook_clear() into hook_free(), so
> it can be passed to string_list_clear_func().
>
> A slight complication is the need to keep a copy of the internal
> cb data free() pointer, however I think it's worth it since the
> API becomes cleaner, e.g. no more calls with NULL function args
> like hook_list_clear(hooks, NULL).
>
> Suggested-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> diff --git a/hook.h b/hook.h
> @@ -186,10 +194,10 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
>  /**
> + * Frees a struct hook stored as the util pointer of a string_list_item.
> + * Suitable for use as a string_list_clear_func_t callback.
>   */
> +void hook_free(void *p, const char *str UNUSED);

See [*] regarding UNUSED in header file.

[*]: https://lore.kernel.org/git/xmqqcy1g25fl.fsf@gitster.g/

Re: [PATCH 05/10] hook: replace hook_list_clear() -> string_list_clear_func()

[Adrian Ratiu]

On Sun, 08 Mar 2026, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sun, Mar 8, 2026 at 8:55 PM Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
>> Replace the custom function with string_list_clear_func() which
>> is a more common pattern for clearing a string_list.
>>
>> To be able to do this, rework hook_clear() into hook_free(), so
>> it can be passed to string_list_clear_func().
>>
>> A slight complication is the need to keep a copy of the internal
>> cb data free() pointer, however I think it's worth it since the
>> API becomes cleaner, e.g. no more calls with NULL function args
>> like hook_list_clear(hooks, NULL).
>>
>> Suggested-by: Patrick Steinhardt <ps@pks.im>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
>> ---
>> diff --git a/hook.h b/hook.h
>> @@ -186,10 +194,10 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
>>  /**
>> + * Frees a struct hook stored as the util pointer of a string_list_item.
>> + * Suitable for use as a string_list_clear_func_t callback.
>>   */
>> +void hook_free(void *p, const char *str UNUSED);
>
> See [*] regarding UNUSED in header file.
>
> [*]: https://lore.kernel.org/git/xmqqcy1g25fl.fsf@gitster.g/

Thank you for pointing this out!

I will fix it in v2 together with your other suggestion.

[PATCH 06/10] hook: make consistent use of friendly-name in docs

[Adrian Ratiu]

Both `name` and `friendly-name` is being used. Standardize on
`friendly-name` for consistency since name is rather generic,
even when used in the hooks namespace.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 Documentation/config/hook.adoc | 30 +++++++++++++++---------------
 Documentation/git-hook.adoc    |  6 +++---
 hook.c                         |  2 +-
 hook.h                         |  2 +-
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc
index 64e845a260..9e78f26439 100644
--- a/Documentation/config/hook.adoc
+++ b/Documentation/config/hook.adoc
@@ -1,23 +1,23 @@
-hook.<name>.command::
-	The command to execute for `hook.<name>`. `<name>` is a unique
-	"friendly" name that identifies this hook. (The hook events that
-	trigger the command are configured with `hook.<name>.event`.) The
-	value can be an executable path or a shell oneliner. If more than
-	one value is specified for the same `<name>`, only the last value
-	parsed is used. See linkgit:git-hook[1].
+hook.<friendly-name>.command::
+	The command to execute for `hook.<friendly-name>`. `<friendly-name>`
+	is a unique name that identifies this hook. The hook events that
+	trigger the command are configured with `hook.<friendly-name>.event`.
+	The value can be an executable path or a shell oneliner. If more than
+	one value is specified for the same `<friendly-name>`, only the last
+	value parsed is used. See linkgit:git-hook[1].
 
-hook.<name>.event::
-	The hook events that trigger `hook.<name>`. The value is the name
-	of a hook event, like "pre-commit" or "update". (See
+hook.<friendly-name>.event::
+	The hook events that trigger `hook.<friendly-name>`. The value is the
+	name of a hook event, like "pre-commit" or "update". (See
 	linkgit:githooks[5] for a complete list of hook events.) On the
-	specified event, the associated `hook.<name>.command` is executed.
-	This is a multi-valued key. To run `hook.<name>` on multiple
+	specified event, the associated `hook.<friendly-name>.command` is executed.
+	This is a multi-valued key. To run `hook.<friendly-name>` on multiple
 	events, specify the key more than once. An empty value resets
 	the list of events, clearing any previously defined events for
-	`hook.<name>`. See linkgit:git-hook[1].
+	`hook.<friendly-name>`. See linkgit:git-hook[1].
 
-hook.<name>.enabled::
-	Whether the hook `hook.<name>` is enabled. Defaults to `true`.
+hook.<friendly-name>.enabled::
+	Whether the hook `hook.<friendly-name>` is enabled. Defaults to `true`.
 	Set to `false` to disable the hook without removing its
 	configuration. This is particularly useful when a hook is defined
 	in a system or global config file and needs to be disabled for a
diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index 12d2701b52..966388660a 100644
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -44,7 +44,7 @@ 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
+`hook.<friendly-name>.event` configs during the configuration parse (see
 linkgit:git-config[1]). Although multiple `hook.linter.event` configs can be
 added, only one `hook.linter.command` event is valid - Git uses "last-one-wins"
 to determine which command to run.
@@ -76,10 +76,10 @@ 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,
+For a full list of hook events which you can set your `hook.<friendly-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
+Git will ignore any `hook.<friendly-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.
 
diff --git a/hook.c b/hook.c
index f6bb1999ae..7f89ae9cc2 100644
--- a/hook.c
+++ b/hook.c
@@ -116,7 +116,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
  * event_hooks: event-name to list of friendly-names map.
- * disabled_hooks: set of friendly-names with hook.name.enabled = false.
+ * disabled_hooks: set of friendly-names with hook.<friendly-name>.enabled = false.
  */
 struct hook_all_config_cb {
 	struct strmap commands;
diff --git a/hook.h b/hook.h
index 168c6495a4..49b40d949b 100644
--- a/hook.h
+++ b/hook.h
@@ -14,7 +14,7 @@ typedef void *(*hook_data_alloc_fn)(void *init_ctx);
  * Represents a hook command to be run.
  * Hooks can be:
  * 1. "traditional" (found in the hooks directory)
- * 2. "configured" (defined in Git's configuration via hook.<name>.event).
+ * 2. "configured" (defined in Git's configuration via hook.<friendly-name>.event).
  * The 'kind' field determines which part of the union 'u' is valid.
  */
 struct hook {
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH 07/10] t1800: add test to verify hook execution ordering

[Adrian Ratiu]

There is a documented expectation that configured hooks are
run before the hook from the hookdir. Add a test for it.

While at it, I noticed that `git hook list -h` runs twice
in the `git hook usage` test, so remove one invocation.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 t/t1800-hook.sh | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 952bf97b86..7eee84fc39 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -25,7 +25,6 @@ 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 &&
 	test_expect_code 129 git hook list &&
 	test_expect_code 129 git hook list -h &&
@@ -381,6 +380,34 @@ test_expect_success 'globally disabled hook can be re-enabled locally' '
 	test_cmp expected actual
 '
 
+test_expect_success 'configured hooks run before hookdir hook' '
+	setup_hookdir &&
+	test_config hook.first.event "pre-commit" &&
+	test_config hook.first.command "echo first" &&
+	test_config hook.second.event "pre-commit" &&
+	test_config hook.second.command "echo second" &&
+
+	cat >expected <<-\EOF &&
+	first
+	second
+	hook from hookdir
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual &&
+
+	# "Legacy Hook" is the output of the hookdir pre-commit script
+	# written by setup_hookdir() above.
+	cat >expected <<-\EOF &&
+	first
+	second
+	"Legacy Hook"
+	EOF
+
+	git hook run pre-commit 2>actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'git hook run a hook with a bad shebang' '
 	test_when_finished "rm -rf bad-hooks" &&
 	mkdir bad-hooks &&
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH 08/10] hook: refactor hook_config_cache from strmap to named struct

[Adrian Ratiu]

Replace the raw `struct strmap *hook_config_cache` in `struct repository`
with a `struct hook_config_cache` which wraps the strmap in a named field.

Replace the bare `char *command` util pointer stored in each string_list
item with a heap-allocated `struct hook_config_cache_entry` that carries
that command string.

This is just a refactoring with no behavior changes, to give the cache
struct room to grow so it can carry the additional hook metadata we'll
be adding in the following commits.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c       | 61 +++++++++++++++++++++++++++++++++-------------------
 hook.h       | 10 ++++++++-
 repository.h |  3 ++-
 3 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/hook.c b/hook.c
index 7f89ae9cc2..4fe50aa38c 100644
--- a/hook.c
+++ b/hook.c
@@ -112,6 +112,15 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	string_list_append(hook_list, hook_path)->util = h;
 }
 
+/*
+ * Cache entry stored as the .util pointer of string_list items inside the
+ * hook config cache. For now carries only the command for the hook. Next
+ * commits will add more data.
+ */
+struct hook_config_cache_entry {
+	char *command;
+};
+
 /*
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
@@ -194,26 +203,32 @@ static int hook_config_lookup_all(const char *key, const char *value,
 /*
  * The hook config cache maps each hook event name to a string_list where
  * every item's string is the hook's friendly-name and its util pointer is
- * the corresponding command string. Both strings are owned by the map.
+ * a hook_config_cache_entry. All strings are owned by the map.
  *
  * Disabled hooks and hooks missing a command are already filtered out at
  * parse time, so callers can iterate the list directly.
  */
-void hook_cache_clear(struct strmap *cache)
+void hook_cache_clear(struct hook_config_cache *cache)
 {
 	struct hashmap_iter iter;
 	struct strmap_entry *e;
 
-	strmap_for_each_entry(cache, &iter, e) {
+	strmap_for_each_entry(&cache->hooks, &iter, e) {
 		struct string_list *hooks = e->value;
-		string_list_clear(hooks, 1); /* free util (command) pointers */
+		for (size_t i = 0; i < hooks->nr; i++) {
+			struct hook_config_cache_entry *entry = hooks->items[i].util;
+			free(entry->command);
+			free(entry);
+		}
+		string_list_clear(hooks, 0);
 		free(hooks);
 	}
-	strmap_clear(cache, 0);
+	strmap_clear(&cache->hooks, 0);
 }
 
 /* Populate `cache` with the complete hook configuration */
-static void build_hook_config_map(struct repository *r, struct strmap *cache)
+static void build_hook_config_map(struct repository *r,
+				  struct hook_config_cache *cache)
 {
 	struct hook_all_config_cb cb_data;
 	struct hashmap_iter iter;
@@ -236,6 +251,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 
 		for (size_t i = 0; i < hook_names->nr; i++) {
 			const char *hname = hook_names->items[i].string;
+			struct hook_config_cache_entry *entry;
 			char *command;
 
 			/* filter out disabled hooks */
@@ -249,12 +265,13 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 				      "'hook.%s.event' must be removed;"
 				      " aborting."), hname, hname);
 
-			/* util stores the command; owned by the cache. */
-			string_list_append(hooks, hname)->util =
-				xstrdup(command);
+			/* util stores a cache entry; owned by the cache. */
+			CALLOC_ARRAY(entry, 1);
+			entry->command = xstrdup(command);
+			string_list_append(hooks, hname)->util = entry;
 		}
 
-		strmap_put(cache, e->key, hooks);
+		strmap_put(&cache->hooks, e->key, hooks);
 	}
 
 	strmap_clear(&cb_data.commands, 1);
@@ -267,25 +284,25 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 }
 
 /*
- * Return the hook config map for `r`, populating it first if needed.
+ * Return the hook config cache for `r`, populating it first if needed.
  *
  * Out-of-repo calls (r->gitdir == NULL) allocate and return a temporary
- * cache map; the caller is responsible for freeing it with
+ * cache; the caller is responsible for freeing it with
  * hook_cache_clear() + free().
  */
-static struct strmap *get_hook_config_cache(struct repository *r)
+static struct hook_config_cache *get_hook_config_cache(struct repository *r)
 {
-	struct strmap *cache = NULL;
+	struct hook_config_cache *cache = NULL;
 
 	if (r && r->gitdir) {
 		/*
-		 * For in-repo calls, the map is stored in r->hook_config_cache,
-		 * so repeated invocations don't parse the configs, so allocate
+		 * For in-repo calls, the cache is stored in r->hook_config_cache,
+		 * so repeated invocations don't parse the configs; allocate
 		 * it just once on the first call.
 		 */
 		if (!r->hook_config_cache) {
 			CALLOC_ARRAY(r->hook_config_cache, 1);
-			strmap_init(r->hook_config_cache);
+			strmap_init(&r->hook_config_cache->hooks);
 			build_hook_config_map(r, r->hook_config_cache);
 		}
 		cache = r->hook_config_cache;
@@ -295,7 +312,7 @@ static struct strmap *get_hook_config_cache(struct repository *r)
 		 * cache which gets freed immediately by the caller.
 		 */
 		CALLOC_ARRAY(cache, 1);
-		strmap_init(cache);
+		strmap_init(&cache->hooks);
 		build_hook_config_map(r, cache);
 	}
 
@@ -307,13 +324,13 @@ static void list_hooks_add_configured(struct repository *r,
 				      struct string_list *list,
 				      struct run_hooks_opt *options)
 {
-	struct strmap *cache = get_hook_config_cache(r);
-	struct string_list *configured_hooks = strmap_get(cache, hookname);
+	struct hook_config_cache *cache = get_hook_config_cache(r);
+	struct string_list *configured_hooks = strmap_get(&cache->hooks, hookname);
 
 	/* Iterate through configured hooks and initialize internal states */
 	for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) {
 		const char *friendly_name = configured_hooks->items[i].string;
-		const char *command = configured_hooks->items[i].util;
+		struct hook_config_cache_entry *entry = configured_hooks->items[i].util;
 		struct hook *hook;
 		CALLOC_ARRAY(hook, 1);
 
@@ -330,7 +347,7 @@ static void list_hooks_add_configured(struct repository *r,
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
-		hook->u.configured.command = xstrdup(command);
+		hook->u.configured.command = xstrdup(entry->command);
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
diff --git a/hook.h b/hook.h
index 49b40d949b..4d0c22f1dc 100644
--- a/hook.h
+++ b/hook.h
@@ -199,11 +199,19 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
  */
 void hook_free(void *p, const char *str UNUSED);
 
+/**
+ * Persistent cache for hook configuration, stored on `struct repository`.
+ * Populated lazily on first hook use and freed by repo_clear().
+ */
+struct hook_config_cache {
+	struct strmap hooks; /* maps event name -> string_list of hooks */
+};
+
 /**
  * Frees the hook configuration cache stored in `struct repository`.
  * Called by repo_clear().
  */
-void hook_cache_clear(struct strmap *cache);
+void hook_cache_clear(struct hook_config_cache *cache);
 
 /**
  * Returns the path to the hook file, or NULL if the hook is missing
diff --git a/repository.h b/repository.h
index 078059a6e0..3fd73d2c54 100644
--- a/repository.h
+++ b/repository.h
@@ -12,6 +12,7 @@ struct lock_file;
 struct pathspec;
 struct object_database;
 struct submodule_cache;
+struct hook_config_cache;
 struct promisor_remote_config;
 struct remote_state;
 
@@ -170,7 +171,7 @@ struct repository {
 	 * Lazily-populated cache mapping hook event names to configured hooks.
 	 * NULL until first hook use.
 	 */
-	struct strmap *hook_config_cache;
+	struct hook_config_cache *hook_config_cache;
 
 	/* Configurations related to promisor remotes. */
 	char *repository_format_partial_clone;
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH 08/10] hook: refactor hook_config_cache from strmap to named struct

[Junio C Hamano]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> Replace the raw `struct strmap *hook_config_cache` in `struct repository`
> with a `struct hook_config_cache` which wraps the strmap in a named field.
>
> Replace the bare `char *command` util pointer stored in each string_list
> item with a heap-allocated `struct hook_config_cache_entry` that carries
> that command string.
>
> This is just a refactoring with no behavior changes, to give the cache
> struct room to grow so it can carry the additional hook metadata we'll
> be adding in the following commits.
>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
>  hook.c       | 61 +++++++++++++++++++++++++++++++++-------------------
>  hook.h       | 10 ++++++++-
>  repository.h |  3 ++-
>  3 files changed, 50 insertions(+), 24 deletions(-)

Hmph, so originally we pointed directly at a "struct strmap"
instance from inside a "struct repository", and the strmap mapped a
hook's event name to the command to run, which is a raw "char *".

In order to future expand the attribute we keep track of for each
hook beyond a simple string, we replace the "char *" with a pointer
to "struct hook_config_cache_entry".  That part makes quite a lot of
sense.

However, I'm less convinced about the value of wrapping the `strmap`
itself in a new `struct hook_config_cache`. Since the "growth" in
later patches happens entirely within the entry struct, this top-level
wrapper feels like unnecessary boilerplate that adds an extra layer of
indirection without a clear benefit. It might be better to keep the
cache as a bare `strmap` and only introduce the entry struct.

Re: [PATCH 08/10] hook: refactor hook_config_cache from strmap to named struct

[Adrian Ratiu]

On Mon, 09 Mar 2026, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
>
>> Replace the raw `struct strmap *hook_config_cache` in `struct repository`
>> with a `struct hook_config_cache` which wraps the strmap in a named field.
>>
>> Replace the bare `char *command` util pointer stored in each string_list
>> item with a heap-allocated `struct hook_config_cache_entry` that carries
>> that command string.
>>
>> This is just a refactoring with no behavior changes, to give the cache
>> struct room to grow so it can carry the additional hook metadata we'll
>> be adding in the following commits.
>>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
>> ---
>>  hook.c       | 61 +++++++++++++++++++++++++++++++++-------------------
>>  hook.h       | 10 ++++++++-
>>  repository.h |  3 ++-
>>  3 files changed, 50 insertions(+), 24 deletions(-)
>
> Hmph, so originally we pointed directly at a "struct strmap"
> instance from inside a "struct repository", and the strmap mapped a
> hook's event name to the command to run, which is a raw "char *".
>
> In order to future expand the attribute we keep track of for each
> hook beyond a simple string, we replace the "char *" with a pointer
> to "struct hook_config_cache_entry".  That part makes quite a lot of
> sense.
>
> However, I'm less convinced about the value of wrapping the `strmap`
> itself in a new `struct hook_config_cache`. Since the "growth" in
> later patches happens entirely within the entry struct, this top-level
> wrapper feels like unnecessary boilerplate that adds an extra layer of
> indirection without a clear benefit. It might be better to keep the
> cache as a bare `strmap` and only introduce the entry struct.

Yes, I think this can work and is also simpler.

Will do in v2, thanks for the suggestion!

[PATCH 09/10] hook: show config scope in git hook list

[Adrian Ratiu]

Users running "git hook list" can see which hooks are configured but
have no way to tell at which config scope (local, global, system...)
each hook was defined.

Store the scope from ctx->kvi->scope in the single-pass config callback,
then carry it through the cache to the hook structs, so we can expose it
to users via the "git hook list --show-scope" flag, which mirrors the
existing git config --show-scope convention.

Without the flag the output is unchanged.

Example usage:
$ git hook list --show-scope pre-commit
linter (global)
no-leaks (local)
hook from hookdir

Traditional hooks from the hookdir are unaffected by --show-scope since
the config scope concept does not apply to them.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 Documentation/git-hook.adoc |  9 +++++++--
 builtin/hook.c              | 14 ++++++++++++--
 hook.c                      | 24 ++++++++++++++++++++----
 hook.h                      |  2 ++
 t/t1800-hook.sh             | 19 +++++++++++++++++++
 5 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index 966388660a..4d4e728327 100644
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
-'git hook' list [-z] <hook-name>
+'git hook' list [-z] [--show-scope] <hook-name>
 
 DESCRIPTION
 -----------
@@ -113,7 +113,7 @@ Any positional arguments to the hook should be passed after a
 mandatory `--` (or `--end-of-options`, see linkgit:gitcli[7]). See
 linkgit:githooks[5] for arguments hooks might expect (if any).
 
-list [-z]::
+list [-z] [--show-scope]::
 	Print a list of hooks which will be run on `<hook-name>` event. If no
 	hooks are configured for that event, print a warning and return 1.
 	Use `-z` to terminate output lines with NUL instead of newlines.
@@ -134,6 +134,11 @@ OPTIONS
 -z::
 	Terminate "list" output lines with NUL instead of newlines.
 
+--show-scope::
+	For "list"; print the config scope (e.g. `local`, `global`, `system`)
+	in parentheses after the friendly name of each configured hook, to show
+	where it was defined. Traditional hooks from the hookdir are unaffected.
+
 WRAPPERS
 --------
 
diff --git a/builtin/hook.c b/builtin/hook.c
index 8fc647a4de..c806640361 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -9,7 +9,7 @@
 #define BUILTIN_HOOK_RUN_USAGE \
 	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
 #define BUILTIN_HOOK_LIST_USAGE \
-	N_("git hook list [-z] <hook-name>")
+	N_("git hook list [-z] [--show-scope] <hook-name>")
 
 static const char * const builtin_hook_usage[] = {
 	BUILTIN_HOOK_RUN_USAGE,
@@ -33,11 +33,14 @@ static int list(int argc, const char **argv, const char *prefix,
 	struct string_list_item *item;
 	const char *hookname = NULL;
 	int line_terminator = '\n';
+	int show_scope = 0;
 	int ret = 0;
 
 	struct option list_options[] = {
 		OPT_SET_INT('z', NULL, &line_terminator,
 			    N_("use NUL as line terminator"), '\0'),
+		OPT_BOOL(0, "show-scope", &show_scope,
+			 N_("show the config scope that defined each hook")),
 		OPT_END(),
 	};
 
@@ -70,7 +73,14 @@ static int list(int argc, const char **argv, const char *prefix,
 			printf("%s%c", _("hook from hookdir"), line_terminator);
 			break;
 		case HOOK_CONFIGURED:
-			printf("%s%c", h->u.configured.friendly_name, line_terminator);
+			if (show_scope)
+				printf("%s (%s)%c",
+				       h->u.configured.friendly_name,
+				       config_scope_name(h->u.configured.scope),
+				       line_terminator);
+			else
+				printf("%s%c", h->u.configured.friendly_name,
+				       line_terminator);
 			break;
 		default:
 			BUG("unknown hook kind");
diff --git a/hook.c b/hook.c
index 4fe50aa38c..2c03baeaac 100644
--- a/hook.c
+++ b/hook.c
@@ -114,11 +114,11 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 
 /*
  * Cache entry stored as the .util pointer of string_list items inside the
- * hook config cache. For now carries only the command for the hook. Next
- * commits will add more data.
+ * hook config cache.
  */
 struct hook_config_cache_entry {
 	char *command;
+	enum config_scope scope;
 };
 
 /*
@@ -135,7 +135,7 @@ struct hook_all_config_cb {
 
 /* repo_config() callback that collects all hook.* configuration in one pass. */
 static int hook_config_lookup_all(const char *key, const char *value,
-				  const struct config_context *ctx UNUSED,
+				  const struct config_context *ctx,
 				  void *cb_data)
 {
 	struct hook_all_config_cb *data = cb_data;
@@ -172,7 +172,19 @@ static int hook_config_lookup_all(const char *key, const char *value,
 
 			/* Re-insert if necessary to preserve last-seen order. */
 			unsorted_string_list_remove(hooks, hook_name, 0);
-			string_list_append(hooks, hook_name);
+
+			if (!ctx->kvi)
+				BUG("hook config callback called without key-value info");
+
+			/*
+			 * Stash the config scope in the util pointer for
+			 * later retrieval in build_hook_config_map(). This
+			 * intermediate struct is transient and never leaves
+			 * that function, so we pack the enum value into the
+			 * pointer rather than heap-allocating a wrapper.
+			 */
+			string_list_append(hooks, hook_name)->util =
+				(void *)(uintptr_t)ctx->kvi->scope;
 		}
 	} else if (!strcmp(subkey, "command")) {
 		/* Store command overwriting the old value */
@@ -251,6 +263,8 @@ static void build_hook_config_map(struct repository *r,
 
 		for (size_t i = 0; i < hook_names->nr; i++) {
 			const char *hname = hook_names->items[i].string;
+			enum config_scope scope =
+				(enum config_scope)(uintptr_t)hook_names->items[i].util;
 			struct hook_config_cache_entry *entry;
 			char *command;
 
@@ -268,6 +282,7 @@ static void build_hook_config_map(struct repository *r,
 			/* util stores a cache entry; owned by the cache. */
 			CALLOC_ARRAY(entry, 1);
 			entry->command = xstrdup(command);
+			entry->scope = scope;
 			string_list_append(hooks, hname)->util = entry;
 		}
 
@@ -348,6 +363,7 @@ static void list_hooks_add_configured(struct repository *r,
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
 		hook->u.configured.command = xstrdup(entry->command);
+		hook->u.configured.scope = entry->scope;
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
diff --git a/hook.h b/hook.h
index 4d0c22f1dc..0d711ed21a 100644
--- a/hook.h
+++ b/hook.h
@@ -1,5 +1,6 @@
 #ifndef HOOK_H
 #define HOOK_H
+#include "config.h"
 #include "strvec.h"
 #include "run-command.h"
 #include "string-list.h"
@@ -29,6 +30,7 @@ struct hook {
 		struct {
 			const char *friendly_name;
 			const char *command;
+			enum config_scope scope;
 		} configured;
 	} u;
 
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 7eee84fc39..aed07575e3 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -408,6 +408,25 @@ test_expect_success 'configured hooks run before hookdir hook' '
 	test_cmp expected actual
 '
 
+test_expect_success 'git hook list --show-scope shows config scope' '
+	test_config_global hook.global-hook.command "echo global" &&
+	test_config_global hook.global-hook.event test-hook --add &&
+	test_config hook.local-hook.command "echo local" &&
+	test_config hook.local-hook.event test-hook --add &&
+
+	cat >expected <<-\EOF &&
+	global-hook (global)
+	local-hook (local)
+	EOF
+	git hook list --show-scope test-hook >actual &&
+	test_cmp expected actual &&
+
+	# without --show-scope the scope must not appear
+	git hook list test-hook >actual &&
+	test_grep ! "(global)" actual &&
+	test_grep ! "(local)" actual
+'
+
 test_expect_success 'git hook run a hook with a bad shebang' '
 	test_when_finished "rm -rf bad-hooks" &&
 	mkdir bad-hooks &&
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH 09/10] hook: show config scope in git hook list

[Junio C Hamano]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> +list [-z] [--show-scope]::
>  	Print a list of hooks which will be run on `<hook-name>` event. If no
>  	hooks are configured for that event, print a warning and return 1.
>  	Use `-z` to terminate output lines with NUL instead of newlines.
> ...
>  		case HOOK_CONFIGURED:
> -			printf("%s%c", h->u.configured.friendly_name, line_terminator);
> +			if (show_scope)
> +				printf("%s (%s)%c",
> +				       h->u.configured.friendly_name,
> +				       config_scope_name(h->u.configured.scope),
> +				       line_terminator);
> +			else
> +				printf("%s%c", h->u.configured.friendly_name,
> +				       line_terminator);
>  			break;

Everything in this patch was as expected (the most important of
which is where the data is kept, which is in the new structure
hook_config_cache_entry that was introduced in the previous step for
use case like this), except for the above bit.

I wonder if this already interacts well enough with "-z", or a minor
tweak would make it better?  Wouldn't a machine consumer expect that
friendly name and cope be given as two separate and easily parseable
fields in the same record?

Re: [PATCH 09/10] hook: show config scope in git hook list

[Adrian Ratiu]

On Mon, 09 Mar 2026, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
>
>> +list [-z] [--show-scope]::
>>  	Print a list of hooks which will be run on `<hook-name>` event. If no
>>  	hooks are configured for that event, print a warning and return 1.
>>  	Use `-z` to terminate output lines with NUL instead of newlines.
>> ...
>>  		case HOOK_CONFIGURED:
>> -			printf("%s%c", h->u.configured.friendly_name, line_terminator);
>> +			if (show_scope)
>> +				printf("%s (%s)%c",
>> +				       h->u.configured.friendly_name,
>> +				       config_scope_name(h->u.configured.scope),
>> +				       line_terminator);
>> +			else
>> +				printf("%s%c", h->u.configured.friendly_name,
>> +				       line_terminator);
>>  			break;
>
> Everything in this patch was as expected (the most important of
> which is where the data is kept, which is in the new structure
> hook_config_cache_entry that was introduced in the previous step for
> use case like this), except for the above bit.
>
> I wonder if this already interacts well enough with "-z", or a minor
> tweak would make it better?  Wouldn't a machine consumer expect that
> friendly name and cope be given as two separate and easily parseable
> fields in the same record?

Yes, out of all the logic in this series, these printf's are the ones
I'm most unsure about.

I'll try to come up with something better for machine parsing in v2.

Suggestions are very much welcome btw. :)

Re: [PATCH 09/10] hook: show config scope in git hook list

[Patrick Steinhardt]

On Mon, Mar 09, 2026 at 02:54:15AM +0200, Adrian Ratiu wrote:
> diff --git a/builtin/hook.c b/builtin/hook.c
> index 8fc647a4de..c806640361 100644
> --- a/builtin/hook.c
> +++ b/builtin/hook.c
> @@ -70,7 +73,14 @@ static int list(int argc, const char **argv, const char *prefix,
>  			printf("%s%c", _("hook from hookdir"), line_terminator);
>  			break;
>  		case HOOK_CONFIGURED:
> -			printf("%s%c", h->u.configured.friendly_name, line_terminator);
> +			if (show_scope)
> +				printf("%s (%s)%c",
> +				       h->u.configured.friendly_name,
> +				       config_scope_name(h->u.configured.scope),
> +				       line_terminator);

Are we sure that this is always unambiguous? Can the friendly name for
example contain a space itself, or is it possible that the scope gets
extended eventually so that parsing becomes ambiguous?

I'm not sure about this myself, but that may indicate that we should
maybe also separate the name and scope with a NUL byte.

> diff --git a/hook.c b/hook.c
> index 4fe50aa38c..2c03baeaac 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -172,7 +172,19 @@ static int hook_config_lookup_all(const char *key, const char *value,
>  
>  			/* Re-insert if necessary to preserve last-seen order. */
>  			unsorted_string_list_remove(hooks, hook_name, 0);
> -			string_list_append(hooks, hook_name);
> +
> +			if (!ctx->kvi)
> +				BUG("hook config callback called without key-value info");
> +
> +			/*
> +			 * Stash the config scope in the util pointer for
> +			 * later retrieval in build_hook_config_map(). This
> +			 * intermediate struct is transient and never leaves
> +			 * that function, so we pack the enum value into the
> +			 * pointer rather than heap-allocating a wrapper.
> +			 */
> +			string_list_append(hooks, hook_name)->util =
> +				(void *)(uintptr_t)ctx->kvi->scope;
>  		}
>  	} else if (!strcmp(subkey, "command")) {
>  		/* Store command overwriting the old value */

Okay. This is a bit ugly, but I guess it should work in practice? The
alternative would be to allocate the scope and store the pointer here.

Patrick

Re: [PATCH 09/10] hook: show config scope in git hook list

[Adrian Ratiu]

On Wed, 11 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Mon, Mar 09, 2026 at 02:54:15AM +0200, Adrian Ratiu wrote:
>> diff --git a/builtin/hook.c b/builtin/hook.c
>> index 8fc647a4de..c806640361 100644
>> --- a/builtin/hook.c
>> +++ b/builtin/hook.c
>> @@ -70,7 +73,14 @@ static int list(int argc, const char **argv, const char *prefix,
>>  			printf("%s%c", _("hook from hookdir"), line_terminator);
>>  			break;
>>  		case HOOK_CONFIGURED:
>> -			printf("%s%c", h->u.configured.friendly_name, line_terminator);
>> +			if (show_scope)
>> +				printf("%s (%s)%c",
>> +				       h->u.configured.friendly_name,
>> +				       config_scope_name(h->u.configured.scope),
>> +				       line_terminator);
>
> Are we sure that this is always unambiguous? Can the friendly name for
> example contain a space itself, or is it possible that the scope gets
> extended eventually so that parsing becomes ambiguous?

Indeed, good catch, yes, friendly-name can contain a space and the scope
can also be extended, so this is not very parseable as Junio also
pointed out in the other message.

We need to come up with something better. :)

> I'm not sure about this myself, but that may indicate that we should
> maybe also separate the name and scope with a NUL byte.

Looking closer at how git config --show-scope does it, I think we could
mirror that, i.e. print the scope as a tab separated prefix.

That would also avoid an awkward intra-line NUL separator, since NUL is
already used for line termination with -z.

I have no strong opinions on this btw and am very open to suggestions.

>> +			if (!ctx->kvi)
>> +				BUG("hook config callback called without key-value info");
>> +
>> +			/*
>> +			 * Stash the config scope in the util pointer for
>> +			 * later retrieval in build_hook_config_map(). This
>> +			 * intermediate struct is transient and never leaves
>> +			 * that function, so we pack the enum value into the
>> +			 * pointer rather than heap-allocating a wrapper.
>> +			 */
>> +			string_list_append(hooks, hook_name)->util =
>> +				(void *)(uintptr_t)ctx->kvi->scope;
>>  		}
>>  	} else if (!strcmp(subkey, "command")) {
>>  		/* Store command overwriting the old value */
>
> Okay. This is a bit ugly, but I guess it should work in practice? The
> alternative would be to allocate the scope and store the pointer here.

Yes, I just tried the simplest thing and it worked. If we need more than
just the scope, then we could heap-allocate a struct wrapper and deal
with the associated memory management complexity.

[PATCH 10/10] hook: show disabled hooks in "git hook list"

[Adrian Ratiu]

Disabled hooks were filtered out of the cache entirely, making them
invisible to "git hook list". Keep them in the cache with a new
"disabled" flag which is propagated to the respective struct hook.

"git hook list" now shows disabled hooks annotated with "(disabled)"
in the config order. With --show-scope, it looks like:

$ git hook list --show-scope pre-commit
linter (global)
no-leaks (local, disabled)
hook from hookdir

A disabled hook without a command issues a warning instead of the
fatal "hook.X.command must be configured" error. We could also throw
an error, however it seemd a bit excessive to me in this case.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c  | 18 ++++++++++-------
 hook.c          | 54 +++++++++++++++++++++++++++++++++----------------
 hook.h          |  1 +
 t/t1800-hook.sh | 33 +++++++++++++++++++++++++++---
 4 files changed, 79 insertions(+), 27 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index c806640361..ff446948fa 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -72,16 +72,20 @@ static int list(int argc, const char **argv, const char *prefix,
 		case HOOK_TRADITIONAL:
 			printf("%s%c", _("hook from hookdir"), line_terminator);
 			break;
-		case HOOK_CONFIGURED:
-			if (show_scope)
-				printf("%s (%s)%c",
-				       h->u.configured.friendly_name,
-				       config_scope_name(h->u.configured.scope),
+		case HOOK_CONFIGURED: {
+			const char *name = h->u.configured.friendly_name;
+			const char *scope = show_scope ?
+				config_scope_name(h->u.configured.scope) : NULL;
+			if (scope)
+				printf("%s (%s%s)%c", name, scope,
+				       h->u.configured.disabled ? ", disabled" : "",
 				       line_terminator);
+			else if (h->u.configured.disabled)
+				printf("%s (disabled)%c", name, line_terminator);
 			else
-				printf("%s%c", h->u.configured.friendly_name,
-				       line_terminator);
+				printf("%s%c", name, line_terminator);
 			break;
+		}
 		default:
 			BUG("unknown hook kind");
 		}
diff --git a/hook.c b/hook.c
index 2c03baeaac..4f4f060156 100644
--- a/hook.c
+++ b/hook.c
@@ -119,6 +119,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 struct hook_config_cache_entry {
 	char *command;
 	enum config_scope scope;
+	int disabled;
 };
 
 /*
@@ -217,8 +218,10 @@ static int hook_config_lookup_all(const char *key, const char *value,
  * every item's string is the hook's friendly-name and its util pointer is
  * a hook_config_cache_entry. All strings are owned by the map.
  *
- * Disabled hooks and hooks missing a command are already filtered out at
- * parse time, so callers can iterate the list directly.
+ * Disabled hooks are kept in the cache with entry->disabled set, so that
+ * "git hook list" can display them. Hooks missing a command are filtered
+ * out at build time; if a disabled hook has no command it is silently
+ * skipped rather than triggering a fatal error.
  */
 void hook_cache_clear(struct hook_config_cache *cache)
 {
@@ -268,21 +271,26 @@ static void build_hook_config_map(struct repository *r,
 			struct hook_config_cache_entry *entry;
 			char *command;
 
-			/* filter out disabled hooks */
-			if (unsorted_string_list_lookup(&cb_data.disabled_hooks,
-							hname))
-				continue;
+			int is_disabled =
+				!!unsorted_string_list_lookup(
+					&cb_data.disabled_hooks, hname);
 
 			command = strmap_get(&cb_data.commands, hname);
-			if (!command)
-				die(_("'hook.%s.command' must be configured or "
-				      "'hook.%s.event' must be removed;"
-				      " aborting."), hname, hname);
+			if (!command) {
+				if (is_disabled)
+					warning(_("disabled hook '%s' has no "
+						  "command configured"), hname);
+				else
+					die(_("'hook.%s.command' must be configured or "
+					      "'hook.%s.event' must be removed;"
+					      " aborting."), hname, hname);
+			}
 
 			/* util stores a cache entry; owned by the cache. */
 			CALLOC_ARRAY(entry, 1);
-			entry->command = xstrdup(command);
+			entry->command = command ? xstrdup(command) : NULL;
 			entry->scope = scope;
+			entry->disabled = is_disabled;
 			string_list_append(hooks, hname)->util = entry;
 		}
 
@@ -362,8 +370,10 @@ static void list_hooks_add_configured(struct repository *r,
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
-		hook->u.configured.command = xstrdup(entry->command);
+		hook->u.configured.command =
+			entry->command ? xstrdup(entry->command) : NULL;
 		hook->u.configured.scope = entry->scope;
+		hook->u.configured.disabled = entry->disabled;
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
@@ -401,7 +411,16 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 int hook_exists(struct repository *r, const char *name)
 {
 	struct string_list *hooks = list_hooks(r, name, NULL);
-	int exists = hooks->nr > 0;
+	int exists = 0;
+
+	for (size_t i = 0; i < hooks->nr; i++) {
+		struct hook *h = hooks->items[i].util;
+		if (h->kind == HOOK_TRADITIONAL ||
+		    !h->u.configured.disabled) {
+			exists = 1;
+			break;
+		}
+	}
 	string_list_clear_func(hooks, hook_free);
 	free(hooks);
 	return exists;
@@ -416,10 +435,11 @@ static int pick_next_hook(struct child_process *cp,
 	struct string_list *hook_list = hook_cb->hook_command_list;
 	struct hook *h;
 
-	if (hook_cb->hook_to_run_index >= hook_list->nr)
-		return 0;
-
-	h = hook_list->items[hook_cb->hook_to_run_index++].util;
+	do {
+		if (hook_cb->hook_to_run_index >= hook_list->nr)
+			return 0;
+		h = hook_list->items[hook_cb->hook_to_run_index++].util;
+	} while (h->kind == HOOK_CONFIGURED && h->u.configured.disabled);
 
 	cp->no_stdin = 1;
 	strvec_pushv(&cp->env, hook_cb->options->env.v);
diff --git a/hook.h b/hook.h
index 0d711ed21a..0432df963f 100644
--- a/hook.h
+++ b/hook.h
@@ -31,6 +31,7 @@ struct hook {
 			const char *friendly_name;
 			const char *command;
 			enum config_scope scope;
+			int disabled;
 		} configured;
 	} u;
 
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index aed07575e3..cbf7d2bf80 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -357,7 +357,15 @@ test_expect_success 'disabled hook is not run' '
 	test_must_be_empty actual
 '
 
-test_expect_success 'disabled hook does not appear in git hook list' '
+test_expect_success 'disabled hook with no command warns' '
+	test_config hook.nocommand.event "pre-commit" &&
+	test_config hook.nocommand.enabled false &&
+
+	git hook list pre-commit 2>actual &&
+	test_grep "disabled hook.*nocommand.*no command configured" actual
+'
+
+test_expect_success 'disabled hook appears as disabled in git hook list' '
 	test_config hook.active.event "pre-commit" &&
 	test_config hook.active.command "echo active" &&
 	test_config hook.inactive.event "pre-commit" &&
@@ -365,8 +373,27 @@ test_expect_success 'disabled hook does not appear in git hook list' '
 	test_config hook.inactive.enabled false &&
 
 	git hook list pre-commit >actual &&
-	test_grep "active" actual &&
-	test_grep ! "inactive" actual
+	test_grep "^active$" actual &&
+	test_grep "^inactive (disabled)$" actual
+'
+
+test_expect_success 'disabled hook shows scope with --show-scope' '
+	test_config hook.myhook.event "pre-commit" &&
+	test_config hook.myhook.command "echo hi" &&
+	test_config hook.myhook.enabled false &&
+
+	git hook list --show-scope pre-commit >actual &&
+	test_grep "myhook (local, disabled)" actual
+'
+
+test_expect_success 'disabled configured hook is not reported as existing by hook_exists' '
+	test_when_finished "rm -f git-bugreport-hook-exists-test.txt" &&
+	test_config hook.linter.event "pre-commit" &&
+	test_config hook.linter.command "echo lint" &&
+	test_config hook.linter.enabled false &&
+
+	git bugreport -s hook-exists-test &&
+	test_grep ! "pre-commit" git-bugreport-hook-exists-test.txt
 '
 
 test_expect_success 'globally disabled hook can be re-enabled locally' '
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH 10/10] hook: show disabled hooks in "git hook list"

[Patrick Steinhardt]

On Mon, Mar 09, 2026 at 02:54:16AM +0200, Adrian Ratiu wrote:
> diff --git a/builtin/hook.c b/builtin/hook.c
> index c806640361..ff446948fa 100644
> --- a/builtin/hook.c
> +++ b/builtin/hook.c
> @@ -72,16 +72,20 @@ static int list(int argc, const char **argv, const char *prefix,
>  		case HOOK_TRADITIONAL:
>  			printf("%s%c", _("hook from hookdir"), line_terminator);
>  			break;
> -		case HOOK_CONFIGURED:
> -			if (show_scope)
> -				printf("%s (%s)%c",
> -				       h->u.configured.friendly_name,
> -				       config_scope_name(h->u.configured.scope),
> +		case HOOK_CONFIGURED: {
> +			const char *name = h->u.configured.friendly_name;
> +			const char *scope = show_scope ?
> +				config_scope_name(h->u.configured.scope) : NULL;
> +			if (scope)
> +				printf("%s (%s%s)%c", name, scope,
> +				       h->u.configured.disabled ? ", disabled" : "",
>  				       line_terminator);
> +			else if (h->u.configured.disabled)
> +				printf("%s (disabled)%c", name, line_terminator);
>  			else
> -				printf("%s%c", h->u.configured.friendly_name,
> -				       line_terminator);
> +				printf("%s%c", name, line_terminator);
>  			break;
> +		}
>  		default:
>  			BUG("unknown hook kind");
>  		}

Hm. This starts to feel less and less like an interface that can easily
be parsed by a machine, even with "-z". I guess this partly comes from
our insistence to reinvent the wheel in Git instead of just using
something like JSON :/

> diff --git a/hook.c b/hook.c
> index 2c03baeaac..4f4f060156 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -119,6 +119,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
>  struct hook_config_cache_entry {
>  	char *command;
>  	enum config_scope scope;
> +	int disabled;
>  };
>  
>  /*

Is there any reason this is an `int` and not a `bool`?

> @@ -217,8 +218,10 @@ static int hook_config_lookup_all(const char *key, const char *value,
>   * every item's string is the hook's friendly-name and its util pointer is
>   * a hook_config_cache_entry. All strings are owned by the map.
>   *
> - * Disabled hooks and hooks missing a command are already filtered out at
> - * parse time, so callers can iterate the list directly.
> + * Disabled hooks are kept in the cache with entry->disabled set, so that
> + * "git hook list" can display them. Hooks missing a command are filtered
> + * out at build time; if a disabled hook has no command it is silently

What exactly does "build time" refer to? To me this reads like invoking
make :)

> + * skipped rather than triggering a fatal error.
>   */
>  void hook_cache_clear(struct hook_config_cache *cache)
>  {
> @@ -268,21 +271,26 @@ static void build_hook_config_map(struct repository *r,
>  			struct hook_config_cache_entry *entry;
>  			char *command;
>  
> -			/* filter out disabled hooks */
> -			if (unsorted_string_list_lookup(&cb_data.disabled_hooks,
> -							hname))
> -				continue;
> +			int is_disabled =
> +				!!unsorted_string_list_lookup(
> +					&cb_data.disabled_hooks, hname);
>  
>  			command = strmap_get(&cb_data.commands, hname);
> -			if (!command)
> -				die(_("'hook.%s.command' must be configured or "
> -				      "'hook.%s.event' must be removed;"
> -				      " aborting."), hname, hname);
> +			if (!command) {
> +				if (is_disabled)
> +					warning(_("disabled hook '%s' has no "
> +						  "command configured"), hname);
> +				else
> +					die(_("'hook.%s.command' must be configured or "
> +					      "'hook.%s.event' must be removed;"
> +					      " aborting."), hname, hname);
> +			}
>  
>  			/* util stores a cache entry; owned by the cache. */
>  			CALLOC_ARRAY(entry, 1);
> -			entry->command = xstrdup(command);
> +			entry->command = command ? xstrdup(command) : NULL;

You can use `xstrdup_or_null()` here.

>  			entry->scope = scope;
> +			entry->disabled = is_disabled;
>  			string_list_append(hooks, hname)->util = entry;
>  		}
>  
> @@ -401,7 +411,16 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
>  int hook_exists(struct repository *r, const char *name)
>  {
>  	struct string_list *hooks = list_hooks(r, name, NULL);
> -	int exists = hooks->nr > 0;
> +	int exists = 0;
> +
> +	for (size_t i = 0; i < hooks->nr; i++) {
> +		struct hook *h = hooks->items[i].util;
> +		if (h->kind == HOOK_TRADITIONAL ||
> +		    !h->u.configured.disabled) {

Is the first condition required? I would expect that `disabled` would
always be false for traditional hooks.

> +			exists = 1;
> +			break;
> +		}
> +	}
>  	string_list_clear_func(hooks, hook_free);
>  	free(hooks);
>  	return exists;
> diff --git a/hook.h b/hook.h
> index 0d711ed21a..0432df963f 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -31,6 +31,7 @@ struct hook {
>  			const char *friendly_name;
>  			const char *command;
>  			enum config_scope scope;
> +			int disabled;
>  		} configured;
>  	} u;
>  

Same question here regarding the type of the struct member.

Patrick

Re: [PATCH 10/10] hook: show disabled hooks in "git hook list"

[Adrian Ratiu]

On Wed, 11 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Mon, Mar 09, 2026 at 02:54:16AM +0200, Adrian Ratiu wrote:
>> diff --git a/builtin/hook.c b/builtin/hook.c
>> index c806640361..ff446948fa 100644
>> --- a/builtin/hook.c
>> +++ b/builtin/hook.c
>> @@ -72,16 +72,20 @@ static int list(int argc, const char **argv, const char *prefix,
>>  		case HOOK_TRADITIONAL:
>>  			printf("%s%c", _("hook from hookdir"), line_terminator);
>>  			break;
>> -		case HOOK_CONFIGURED:
>> -			if (show_scope)
>> -				printf("%s (%s)%c",
>> -				       h->u.configured.friendly_name,
>> -				       config_scope_name(h->u.configured.scope),
>> +		case HOOK_CONFIGURED: {
>> +			const char *name = h->u.configured.friendly_name;
>> +			const char *scope = show_scope ?
>> +				config_scope_name(h->u.configured.scope) : NULL;
>> +			if (scope)
>> +				printf("%s (%s%s)%c", name, scope,
>> +				       h->u.configured.disabled ? ", disabled" : "",
>>  				       line_terminator);
>> +			else if (h->u.configured.disabled)
>> +				printf("%s (disabled)%c", name, line_terminator);
>>  			else
>> -				printf("%s%c", h->u.configured.friendly_name,
>> -				       line_terminator);
>> +				printf("%s%c", name, line_terminator);
>>  			break;
>> +		}
>>  		default:
>>  			BUG("unknown hook kind");
>>  		}
>
> Hm. This starts to feel less and less like an interface that can easily
> be parsed by a machine, even with "-z". I guess this partly comes from
> our insistence to reinvent the wheel in Git instead of just using
> something like JSON :/

Yes, I agree, a structured output format like JSON would be ideal in
this case.

Please see my previous patch suggestion of mirroring the existing git
config --show-scope by using tab separated prefixes. Maybe we could do
that here as well.

Suggestions welcome.

>> diff --git a/hook.c b/hook.c
>> index 2c03baeaac..4f4f060156 100644
>> --- a/hook.c
>> +++ b/hook.c
>> @@ -119,6 +119,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
>>  struct hook_config_cache_entry {
>>  	char *command;
>>  	enum config_scope scope;
>> +	int disabled;
>>  };
>>  
>>  /*
>
> Is there any reason this is an `int` and not a `bool`?

No, I'll change it to bool in v2, it was just an oversight on my part.

>> @@ -217,8 +218,10 @@ static int hook_config_lookup_all(const char *key, const char *value,
>>   * every item's string is the hook's friendly-name and its util pointer is
>>   * a hook_config_cache_entry. All strings are owned by the map.
>>   *
>> - * Disabled hooks and hooks missing a command are already filtered out at
>> - * parse time, so callers can iterate the list directly.
>> + * Disabled hooks are kept in the cache with entry->disabled set, so that
>> + * "git hook list" can display them. Hooks missing a command are filtered
>> + * out at build time; if a disabled hook has no command it is silently
>
> What exactly does "build time" refer to? To me this reads like invoking
> make :)

This is one of my pet peeves: I (mis)use "build time" a lot.

build time in this case == cache construction time. :)

The comment is wrong anyway, because in the end I decided to issue a
warning (see code immediately below) instead of silently ignoring.

I'll reword this in v2. Thanks!

>> + * skipped rather than triggering a fatal error.
>>   */
>>  void hook_cache_clear(struct hook_config_cache *cache)
>>  {
>> @@ -268,21 +271,26 @@ static void build_hook_config_map(struct repository *r,
>>  			struct hook_config_cache_entry *entry;
>>  			char *command;
>>  
>> -			/* filter out disabled hooks */
>> -			if (unsorted_string_list_lookup(&cb_data.disabled_hooks,
>> -							hname))
>> -				continue;
>> +			int is_disabled =
>> +				!!unsorted_string_list_lookup(
>> +					&cb_data.disabled_hooks, hname);
>>  
>>  			command = strmap_get(&cb_data.commands, hname);
>> -			if (!command)
>> -				die(_("'hook.%s.command' must be configured or "
>> -				      "'hook.%s.event' must be removed;"
>> -				      " aborting."), hname, hname);
>> +			if (!command) {
>> +				if (is_disabled)
>> +					warning(_("disabled hook '%s' has no "
>> +						  "command configured"), hname);
>> +				else
>> +					die(_("'hook.%s.command' must be configured or "
>> +					      "'hook.%s.event' must be removed;"
>> +					      " aborting."), hname, hname);
>> +			}
>>  
>>  			/* util stores a cache entry; owned by the cache. */
>>  			CALLOC_ARRAY(entry, 1);
>> -			entry->command = xstrdup(command);
>> +			entry->command = command ? xstrdup(command) : NULL;
>
> You can use `xstrdup_or_null()` here.

Ack, will do in v2.

>>  			entry->scope = scope;
>> +			entry->disabled = is_disabled;
>>  			string_list_append(hooks, hname)->util = entry;
>>  		}
>>  
>> @@ -401,7 +411,16 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
>>  int hook_exists(struct repository *r, const char *name)
>>  {
>>  	struct string_list *hooks = list_hooks(r, name, NULL);
>> -	int exists = hooks->nr > 0;
>> +	int exists = 0;
>> +
>> +	for (size_t i = 0; i < hooks->nr; i++) {
>> +		struct hook *h = hooks->items[i].util;
>> +		if (h->kind == HOOK_TRADITIONAL ||
>> +		    !h->u.configured.disabled) {
>
> Is the first condition required? I would expect that `disabled` would
> always be false for traditional hooks.

Yes, it is required because !h->u.configured.disabled only applies to
non-traditional (configured) hooks.

The "traditional" member of the union doesn't even have a disabled
field, when h->kind == HOOK_TRADITIONAL => always exists = 1;

Basically we're checking two different types here, if they exist and
short-circuiting in the traditional hook case.

Hope that explanation makes sense.

I'll see if I can make this condition clearer in v2.


>> +			exists = 1;
>> +			break;
>> +		}
>> +	}
>>  	string_list_clear_func(hooks, hook_free);
>>  	free(hooks);
>>  	return exists;
>> diff --git a/hook.h b/hook.h
>> index 0d711ed21a..0432df963f 100644
>> --- a/hook.h
>> +++ b/hook.h
>> @@ -31,6 +31,7 @@ struct hook {
>>  			const char *friendly_name;
>>  			const char *command;
>>  			enum config_scope scope;
>> +			int disabled;
>>  		} configured;
>>  	} u;
>>  
>
> Same question here regarding the type of the struct member.

Yes, it can be a bool. Will do in v2.

Re: [PATCH 10/10] hook: show disabled hooks in "git hook list"

[Patrick Steinhardt]

On Wed, Mar 11, 2026 at 02:24:14PM +0200, Adrian Ratiu wrote:
> On Wed, 11 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> > On Mon, Mar 09, 2026 at 02:54:16AM +0200, Adrian Ratiu wrote:
> >> diff --git a/builtin/hook.c b/builtin/hook.c
> >> index c806640361..ff446948fa 100644
> >> --- a/builtin/hook.c
> >> +++ b/builtin/hook.c
> >> @@ -72,16 +72,20 @@ static int list(int argc, const char **argv, const char *prefix,
> >>  		case HOOK_TRADITIONAL:
> >>  			printf("%s%c", _("hook from hookdir"), line_terminator);
> >>  			break;
> >> -		case HOOK_CONFIGURED:
> >> -			if (show_scope)
> >> -				printf("%s (%s)%c",
> >> -				       h->u.configured.friendly_name,
> >> -				       config_scope_name(h->u.configured.scope),
> >> +		case HOOK_CONFIGURED: {
> >> +			const char *name = h->u.configured.friendly_name;
> >> +			const char *scope = show_scope ?
> >> +				config_scope_name(h->u.configured.scope) : NULL;
> >> +			if (scope)
> >> +				printf("%s (%s%s)%c", name, scope,
> >> +				       h->u.configured.disabled ? ", disabled" : "",
> >>  				       line_terminator);
> >> +			else if (h->u.configured.disabled)
> >> +				printf("%s (disabled)%c", name, line_terminator);
> >>  			else
> >> -				printf("%s%c", h->u.configured.friendly_name,
> >> -				       line_terminator);
> >> +				printf("%s%c", name, line_terminator);
> >>  			break;
> >> +		}
> >>  		default:
> >>  			BUG("unknown hook kind");
> >>  		}
> >
> > Hm. This starts to feel less and less like an interface that can easily
> > be parsed by a machine, even with "-z". I guess this partly comes from
> > our insistence to reinvent the wheel in Git instead of just using
> > something like JSON :/
> 
> Yes, I agree, a structured output format like JSON would be ideal in
> this case.
> 
> Please see my previous patch suggestion of mirroring the existing git
> config --show-scope by using tab separated prefixes. Maybe we could do
> that here as well.

Yeah, I think doing it similarly makes sense.

Patrick

Re: [PATCH 00/10] config-hook cleanups and two small 'git hook list' features

[Junio C Hamano]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> As promised I've spun-off v3 of the config series [1] into its own standalone
> patch series after v2 landed in next.
>
> This is mostly minor cleanups and refactorings + two minor feature additions
> to `git hook list`, which resulted from the previous review discussions:
>
> 1. The ability to show the config scope (--show-scope).
> 2. The ability to show which hooks are disabled.

OK.

> This is based on next because the config hooks support is only in next.

Not advisable, as doing so would take your topic hostage of _all_
other topics in 'next', and it will _never_ happen for all of them,
including the merge commit that merged them into 'next', to be
merged to 'master'.

After learning from the output of

    $ git log --first-parent --oneline master..'seen^{/^### match next}' |
      grep ar/

that ar/config-hooks and ar/run-command-hook-take-2 are the two
topic that may be relevant to the config-hook topic in 'next', and
knowing that ar/config-hooks fully contains the other topic, I
instead did the following to prepare a base:

    $ git checkout -b ar/config-hook-cleanups master
    $ git merge ar/config-hooks

and then applied these 10 patches.  That way, ar/config-hooks can
graduate in due course, and then this topic can follow, without
waiting for other random things in 'next'.

> I have pushed the branch to Github [2] and provided a clean CI run [3].
>
> Big thank-you's to all who contributed to this up to now,
> Adrian

Thanks.  Queued.

Re: [PATCH 00/10] config-hook cleanups and two small 'git hook list' features

[Adrian Ratiu]

On Mon, 09 Mar 2026, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
>
>> As promised I've spun-off v3 of the config series [1] into its own standalone
>> patch series after v2 landed in next.
>>
>> This is mostly minor cleanups and refactorings + two minor feature additions
>> to `git hook list`, which resulted from the previous review discussions:
>>
>> 1. The ability to show the config scope (--show-scope).
>> 2. The ability to show which hooks are disabled.
>
> OK.
>
>> This is based on next because the config hooks support is only in next.
>
> Not advisable, as doing so would take your topic hostage of _all_
> other topics in 'next', and it will _never_ happen for all of them,
> including the merge commit that merged them into 'next', to be
> merged to 'master'.
>
> After learning from the output of
>
>     $ git log --first-parent --oneline master..'seen^{/^### match next}' |
>       grep ar/
>
> that ar/config-hooks and ar/run-command-hook-take-2 are the two
> topic that may be relevant to the config-hook topic in 'next', and
> knowing that ar/config-hooks fully contains the other topic, I
> instead did the following to prepare a base:
>
>     $ git checkout -b ar/config-hook-cleanups master
>     $ git merge ar/config-hooks
>
> and then applied these 10 patches.  That way, ar/config-hooks can
> graduate in due course, and then this topic can follow, without
> waiting for other random things in 'next'.

I'll do something similar in my future series and stop basing patches on
top of next (I know it's the second time you're telling me to stop doing
this). Creating such a merge-base manually just didn't cross my mind. :)

Thanks for the tip.

Re: [PATCH 00/10] config-hook cleanups and two small 'git hook list' features

[Junio C Hamano]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> Hello everyone,
>
> As promised I've spun-off v3 of the config series [1] into its own standalone
> patch series after v2 landed in next.
>
> This is mostly minor cleanups and refactorings + two minor feature additions
> to `git hook list`, which resulted from the previous review discussions:
>
> 1. The ability to show the config scope (--show-scope).
> 2. The ability to show which hooks are disabled.

This is a very pleasant series to read. Thank you for spinning these
cleanups and new features off into their own series. It makes the
evolution of the hook-config work much easier to follow.

The overall progression from general cleanups to the more involved
cache refactoring and finally the new features is logical and well-
executed.

I may have a few comments on the later patches, but the early parts
look already very promising.

[PATCH 1/10] to [PATCH 4/10]
These look solid and correctly address the style and naming nits
raised in previous rounds. Moving unsorted_string_list_remove() to
string-list.[ch] is a good call as it's a generally useful utility.

[PATCH 5/10] hook: replace hook_list_clear() -> string_list_clear_func()
Appreciative of this change; using the standard string_list API
makes the code more idiomatic. Stashing the data_free pointer in
struct hook is a clean way to handle the internal callback data.

Thanks.

[PATCH v2 00/10] config-hook cleanups and two small 'git hook list' features

[Adrian Ratiu]

Hello everyone,

v2 addresses all the feedback received in v1, many thanks to everyone
who contributed.

This series is just minor cleanups / refactorings + two minor feature additions
to `git hook list`, which resulted from the previous series review discussions:

1. The ability to show the config scope (--show-scope).
2. The ability to show which hooks are disabled.

This is now based on the master branch.

I have pushed the branch to Github [1] and provided a clean CI run [2] with
the exception of a known breakage for some MacOS builders (REG_ENHANCED).

Thanks again,
Adrian

1: https://github.com/10ne1/git/tree/dev/aratiu/config-cleanups-v2
2: https://github.com/10ne1/git/actions/runs/23340298770

Changes in v2:
* Cleanly rebased on master, no conflicts (Adrian)
* Fix first patch build break by updating call-sites in same commit (Szeder)
* Drop UNUSED from function declaration in the header file (Eric)
* Drop the new struct hook_config_cache because it's redundant (Junio)
* git hook list now prints in tab separated output format similar
  to git config --show-scope to improve machine parseability (Junio, Patrick)
* Fix small style issues, comments, type, commit messages. (Eric, Patrick)

Range-diff v1 -> v2:
 1:  6f4ac0ddf8 !  1:  aeefa72f33 hook: move unsorted_string_list_remove() to string-list.[ch]
    @@ Commit message
         Move the convenience wrapper from hook to string-list since
         it's a more suitable place. Add a doc comment to the header.
     
    +    Also add a free_util arg to make the function more generic
    +    and make the API similar to other functions in string-list.h.
    +    Update the existing call-sites.
    +
         Suggested-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
     
    @@ hook.c: static void list_hooks_add_default(struct repository *r, const char *hoo
      /*
       * Callback struct to collect all hook.* keys in a single config pass.
       * commands: friendly-name to command map.
    +@@ hook.c: static int hook_config_lookup_all(const char *key, const char *value,
    + 			struct strmap_entry *e;
    + 
    + 			strmap_for_each_entry(&data->event_hooks, &iter, e)
    +-				unsorted_string_list_remove(e->value, hook_name);
    ++				unsorted_string_list_remove(e->value, hook_name, 0);
    + 		} else {
    + 			struct string_list *hooks =
    + 				strmap_get(&data->event_hooks, value);
    +@@ hook.c: static int hook_config_lookup_all(const char *key, const char *value,
    + 			}
    + 
    + 			/* Re-insert if necessary to preserve last-seen order. */
    +-			unsorted_string_list_remove(hooks, hook_name);
    ++			unsorted_string_list_remove(hooks, hook_name, 0);
    + 			string_list_append(hooks, hook_name);
    + 		}
    + 	} else if (!strcmp(subkey, "command")) {
    +@@ hook.c: static int hook_config_lookup_all(const char *key, const char *value,
    + 			break;
    + 		case 1: /* enabled: undo a prior disabled entry */
    + 			unsorted_string_list_remove(&data->disabled_hooks,
    +-						    hook_name);
    ++						    hook_name, 0);
    + 			break;
    + 		default:
    + 			break; /* ignore unrecognised values */
     
      ## string-list.c ##
     @@ string-list.c: void unsorted_string_list_delete_item(struct string_list *list, int i, int free_
 2:  df876b8bc5 !  2:  90e821bdfa hook: fix minor style issues
    @@ Metadata
      ## Commit message ##
         hook: fix minor style issues
     
    -    Fix some minor style nits pointed by Patrick and Junio:
    +    Fix some minor style nits pointed by Patrick, Junio and Eric:
           * Use CALLOC_ARRAY instead of xcalloc.
           * Init struct members during declaration.
           * Simplify if condition boolean logic.
           * Missing curly braces in if/else stmts.
           * Unnecessary header includes.
    -      * Capitalization in error/warn messages.
    +      * Capitalization and full-stop in error/warn messages.
    +      * Curly brace on separate line when defining struct.
           * Comment spelling: free'd -> freed.
     
         These contain no logic changes, the code behaves the same as before.
     
    +    Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
         Suggested-by: Junio C Hamano <gitster@pobox.com>
         Suggested-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
    @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix,
      	 */
      	if (argc != 1)
     -		usage_msg_opt(_("You must specify a hook event name to list."),
    -+		usage_msg_opt(_("you must specify a hook event name to list."),
    ++		usage_msg_opt(_("you must specify a hook event name to list"),
      			      builtin_hook_list_usage, list_options);
      
      	hookname = argv[0];
    @@ hook.c: int run_hooks_opt(struct repository *r, const char *hook_name,
      
      	if (options->invoked_hook)
     
    + ## hook.h ##
    +@@ hook.h: struct hook {
    + typedef void (*cb_data_free_fn)(void *data);
    + typedef void *(*cb_data_alloc_fn)(void *init_ctx);
    + 
    +-struct run_hooks_opt
    +-{
    ++struct run_hooks_opt {
    + 	/* Environment vars to be set for each hook */
    + 	struct strvec env;
    + 
    +
      ## refs.c ##
     @@ refs.c: static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
      
 3:  7cf6e32087 !  3:  dee1dd49a4 hook: rename cb_data_free/alloc -> hook_data_free/alloc
    @@ hook.h: struct hook {
     +typedef void (*hook_data_free_fn)(void *data);
     +typedef void *(*hook_data_alloc_fn)(void *init_ctx);
      
    - struct run_hooks_opt
    - {
    -@@ hook.h: struct run_hooks_opt
    + struct run_hooks_opt {
    + 	/* Environment vars to be set for each hook */
    +@@ hook.h: struct run_hooks_opt {
      	 *
      	 * The `feed_pipe_ctx` pointer can be used to pass initialization data.
      	 */
 4:  06a9f3bfcb =  4:  86f61204b3 hook: detect & emit two more bugs
 5:  47190d7a22 !  5:  30542de351 hook: replace hook_list_clear() -> string_list_clear_func()
    @@ hook.c: static void list_hooks_add_default(struct repository *r, const char *hoo
      
      	h->kind = HOOK_TRADITIONAL;
      	h->u.traditional.path = xstrdup(hook_path);
    -@@ hook.c: static int hook_config_lookup_all(const char *key, const char *value,
    - 			struct strmap_entry *e;
    - 
    - 			strmap_for_each_entry(&data->event_hooks, &iter, e)
    --				unsorted_string_list_remove(e->value, hook_name);
    -+				unsorted_string_list_remove(e->value, hook_name, 0);
    - 		} else {
    - 			struct string_list *hooks =
    - 				strmap_get(&data->event_hooks, value);
    -@@ hook.c: static int hook_config_lookup_all(const char *key, const char *value,
    - 			}
    - 
    - 			/* Re-insert if necessary to preserve last-seen order. */
    --			unsorted_string_list_remove(hooks, hook_name);
    -+			unsorted_string_list_remove(hooks, hook_name, 0);
    - 			string_list_append(hooks, hook_name);
    - 		}
    - 	} else if (!strcmp(subkey, "command")) {
    -@@ hook.c: static int hook_config_lookup_all(const char *key, const char *value,
    - 			break;
    - 		case 1: /* enabled: undo a prior disabled entry */
    - 			unsorted_string_list_remove(&data->disabled_hooks,
    --						    hook_name);
    -+						    hook_name, 0);
    - 			break;
    - 		default:
    - 			break; /* ignore unrecognised values */
     @@ hook.c: static void list_hooks_add_configured(struct repository *r,
      		struct hook *hook;
      		CALLOC_ARRAY(hook, 1);
    @@ hook.h: struct hook {
     +	hook_data_free_fn data_free;
     +};
      
    - struct run_hooks_opt
    - {
    + struct run_hooks_opt {
    + 	/* Environment vars to be set for each hook */
     @@ hook.h: struct string_list *list_hooks(struct repository *r, const char *hookname,
      			       struct run_hooks_opt *options);
      
    @@ hook.h: struct string_list *list_hooks(struct repository *r, const char *hooknam
     + * Suitable for use as a string_list_clear_func_t callback.
       */
     -void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
    -+void hook_free(void *p, const char *str UNUSED);
    ++void hook_free(void *p, const char *str);
      
      /**
       * Frees the hook configuration cache stored in `struct repository`.
 6:  469b8a7192 =  6:  4e1374b84e hook: make consistent use of friendly-name in docs
 7:  dff30a5c06 =  7:  075f8202aa t1800: add test to verify hook execution ordering
 8:  e53a000982 <  -:  ---------- hook: refactor hook_config_cache from strmap to named struct
 -:  ---------- >  8:  8f948bbbe7 hook: introduce hook_config_cache_entry for per-hook data
 9:  7d7753b48a !  9:  dbf81604ed hook: show config scope in git hook list
    @@ Commit message
     
         Without the flag the output is unchanged.
     
    +    The scope is printed as a tab-separated prefix (like "git config --show-scope"),
    +    making it unambiguously machine-parseable even when the friendly name
    +    contains spaces.
    +
         Example usage:
         $ git hook list --show-scope pre-commit
    -    linter (global)
    -    no-leaks (local)
    +    global  linter
    +    local   no-leaks
         hook from hookdir
     
         Traditional hooks from the hookdir are unaffected by --show-scope since
    @@ Documentation/git-hook.adoc: OPTIONS
      	Terminate "list" output lines with NUL instead of newlines.
      
     +--show-scope::
    -+	For "list"; print the config scope (e.g. `local`, `global`, `system`)
    -+	in parentheses after the friendly name of each configured hook, to show
    -+	where it was defined. Traditional hooks from the hookdir are unaffected.
    ++	For "list"; prefix each configured hook's friendly name with a
    ++	tab-separated config scope (e.g. `local`, `global`, `system`),
    ++	mirroring the output style of `git config --show-scope`. Traditional
    ++	hooks from the hookdir are unaffected.
     +
      WRAPPERS
      --------
    @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix,
      		case HOOK_CONFIGURED:
     -			printf("%s%c", h->u.configured.friendly_name, line_terminator);
     +			if (show_scope)
    -+				printf("%s (%s)%c",
    -+				       h->u.configured.friendly_name,
    ++				printf("%s\t%s%c",
     +				       config_scope_name(h->u.configured.scope),
    ++				       h->u.configured.friendly_name,
     +				       line_terminator);
     +			else
     +				printf("%s%c", h->u.configured.friendly_name,
    @@ hook.c: static int hook_config_lookup_all(const char *key, const char *value,
      		}
      	} else if (!strcmp(subkey, "command")) {
      		/* Store command overwriting the old value */
    -@@ hook.c: static void build_hook_config_map(struct repository *r,
    +@@ hook.c: static void build_hook_config_map(struct repository *r, struct strmap *cache)
      
      		for (size_t i = 0; i < hook_names->nr; i++) {
      			const char *hname = hook_names->items[i].string;
    @@ hook.c: static void build_hook_config_map(struct repository *r,
      			struct hook_config_cache_entry *entry;
      			char *command;
      
    -@@ hook.c: static void build_hook_config_map(struct repository *r,
    +@@ hook.c: static void build_hook_config_map(struct repository *r, struct strmap *cache)
      			/* util stores a cache entry; owned by the cache. */
      			CALLOC_ARRAY(entry, 1);
      			entry->command = xstrdup(command);
    @@ t/t1800-hook.sh: test_expect_success 'configured hooks run before hookdir hook'
     +	test_config hook.local-hook.event test-hook --add &&
     +
     +	cat >expected <<-\EOF &&
    -+	global-hook (global)
    -+	local-hook (local)
    ++	global	global-hook
    ++	local	local-hook
     +	EOF
     +	git hook list --show-scope test-hook >actual &&
     +	test_cmp expected actual &&
     +
     +	# without --show-scope the scope must not appear
     +	git hook list test-hook >actual &&
    -+	test_grep ! "(global)" actual &&
    -+	test_grep ! "(local)" actual
    ++	test_grep ! "^global	" actual &&
    ++	test_grep ! "^local	" actual
     +'
     +
      test_expect_success 'git hook run a hook with a bad shebang' '
10:  dabb17aad6 ! 10:  2a67244b20 hook: show disabled hooks in "git hook list"
    @@ Commit message
         invisible to "git hook list". Keep them in the cache with a new
         "disabled" flag which is propagated to the respective struct hook.
     
    -    "git hook list" now shows disabled hooks annotated with "(disabled)"
    -    in the config order. With --show-scope, it looks like:
    +    "git hook list" now shows disabled hooks as tab-separated columns,
    +    with the status as a prefix before the name (like scope with
    +    --show-scope). With --show-scope it looks like:
     
         $ git hook list --show-scope pre-commit
    -    linter (global)
    -    no-leaks (local, disabled)
    +    global  linter
    +    local   disabled        no-leaks
         hook from hookdir
     
         A disabled hook without a command issues a warning instead of the
    @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix,
      			break;
     -		case HOOK_CONFIGURED:
     -			if (show_scope)
    --				printf("%s (%s)%c",
    --				       h->u.configured.friendly_name,
    +-				printf("%s\t%s%c",
     -				       config_scope_name(h->u.configured.scope),
    +-				       h->u.configured.friendly_name,
    +-				       line_terminator);
     +		case HOOK_CONFIGURED: {
     +			const char *name = h->u.configured.friendly_name;
     +			const char *scope = show_scope ?
     +				config_scope_name(h->u.configured.scope) : NULL;
     +			if (scope)
    -+				printf("%s (%s%s)%c", name, scope,
    -+				       h->u.configured.disabled ? ", disabled" : "",
    - 				       line_terminator);
    -+			else if (h->u.configured.disabled)
    -+				printf("%s (disabled)%c", name, line_terminator);
    ++				printf("%s\t%s%s%c", scope,
    ++				       h->u.configured.disabled ? "disabled\t" : "",
    ++				       name, line_terminator);
      			else
     -				printf("%s%c", h->u.configured.friendly_name,
     -				       line_terminator);
    -+				printf("%s%c", name, line_terminator);
    ++				printf("%s%s%c",
    ++				       h->u.configured.disabled ? "disabled\t" : "",
    ++				       name, line_terminator);
      			break;
     +		}
      		default:
    @@ hook.c: static void list_hooks_add_default(struct repository *r, const char *hoo
      struct hook_config_cache_entry {
      	char *command;
      	enum config_scope scope;
    -+	int disabled;
    ++	unsigned int disabled:1;
      };
      
      /*
     @@ hook.c: static int hook_config_lookup_all(const char *key, const char *value,
       * every item's string is the hook's friendly-name and its util pointer is
    -  * a hook_config_cache_entry. All strings are owned by the map.
    +  * the corresponding command string. Both strings are owned by the map.
       *
     - * Disabled hooks and hooks missing a command are already filtered out at
     - * parse time, so callers can iterate the list directly.
     + * Disabled hooks are kept in the cache with entry->disabled set, so that
    -+ * "git hook list" can display them. Hooks missing a command are filtered
    -+ * out at build time; if a disabled hook has no command it is silently
    -+ * skipped rather than triggering a fatal error.
    ++ * "git hook list" can display them. A non-disabled hook missing a command
    ++ * is fatal; a disabled hook missing a command emits a warning and is kept
    ++ * in the cache with entry->command = NULL.
       */
    - void hook_cache_clear(struct hook_config_cache *cache)
    + void hook_cache_clear(struct strmap *cache)
      {
    -@@ hook.c: static void build_hook_config_map(struct repository *r,
    +@@ hook.c: static void build_hook_config_map(struct repository *r, struct strmap *cache)
      			struct hook_config_cache_entry *entry;
      			char *command;
      
    @@ hook.c: static void build_hook_config_map(struct repository *r,
      			/* util stores a cache entry; owned by the cache. */
      			CALLOC_ARRAY(entry, 1);
     -			entry->command = xstrdup(command);
    -+			entry->command = command ? xstrdup(command) : NULL;
    ++			entry->command = xstrdup_or_null(command);
      			entry->scope = scope;
     +			entry->disabled = is_disabled;
      			string_list_append(hooks, hname)->util = entry;
    @@ hook.h: struct hook {
      			const char *friendly_name;
      			const char *command;
      			enum config_scope scope;
    -+			int disabled;
    ++			unsigned int disabled:1;
      		} configured;
      	} u;
      
    @@ t/t1800-hook.sh: test_expect_success 'disabled hook does not appear in git hook
     -	test_grep "active" actual &&
     -	test_grep ! "inactive" actual
     +	test_grep "^active$" actual &&
    -+	test_grep "^inactive (disabled)$" actual
    ++	test_grep "^disabled	inactive$" actual
     +'
     +
     +test_expect_success 'disabled hook shows scope with --show-scope' '
    @@ t/t1800-hook.sh: test_expect_success 'disabled hook does not appear in git hook
     +	test_config hook.myhook.enabled false &&
     +
     +	git hook list --show-scope pre-commit >actual &&
    -+	test_grep "myhook (local, disabled)" actual
    ++	test_grep "^local	disabled	myhook$" actual
     +'
     +
     +test_expect_success 'disabled configured hook is not reported as existing by hook_exists' '

Adrian Ratiu (10):
  hook: move unsorted_string_list_remove() to string-list.[ch]
  hook: fix minor style issues
  hook: rename cb_data_free/alloc -> hook_data_free/alloc
  hook: detect & emit two more bugs
  hook: replace hook_list_clear() -> string_list_clear_func()
  hook: make consistent use of friendly-name in docs
  t1800: add test to verify hook execution ordering
  hook: introduce hook_config_cache_entry for per-hook data
  hook: show config scope in git hook list
  hook: show disabled hooks in "git hook list"

 Documentation/config/hook.adoc |  30 +++---
 Documentation/git-hook.adoc    |  16 ++-
 builtin/hook.c                 |  28 +++--
 builtin/receive-pack.c         |  11 +-
 hook.c                         | 187 +++++++++++++++++++++------------
 hook.h                         |  32 ++++--
 refs.c                         |   3 +-
 string-list.c                  |   9 ++
 string-list.h                  |   8 ++
 t/t1800-hook.sh                |  83 ++++++++++++++-
 transport.c                    |   3 +-
 11 files changed, 295 insertions(+), 115 deletions(-)

-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v2 01/10] hook: move unsorted_string_list_remove() to string-list.[ch]

[Adrian Ratiu]

Move the convenience wrapper from hook to string-list since
it's a more suitable place. Add a doc comment to the header.

Also add a free_util arg to make the function more generic
and make the API similar to other functions in string-list.h.
Update the existing call-sites.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c        | 14 +++-----------
 string-list.c |  9 +++++++++
 string-list.h |  8 ++++++++
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/hook.c b/hook.c
index 2c8252b2c4..67cc9a66df 100644
--- a/hook.c
+++ b/hook.c
@@ -110,14 +110,6 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	string_list_append(hook_list, hook_path)->util = h;
 }
 
-static void unsorted_string_list_remove(struct string_list *list,
-					const char *str)
-{
-	struct string_list_item *item = unsorted_string_list_lookup(list, str);
-	if (item)
-		unsorted_string_list_delete_item(list, item - list->items, 0);
-}
-
 /*
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
@@ -156,7 +148,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			struct strmap_entry *e;
 
 			strmap_for_each_entry(&data->event_hooks, &iter, e)
-				unsorted_string_list_remove(e->value, hook_name);
+				unsorted_string_list_remove(e->value, hook_name, 0);
 		} else {
 			struct string_list *hooks =
 				strmap_get(&data->event_hooks, value);
@@ -168,7 +160,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			}
 
 			/* Re-insert if necessary to preserve last-seen order. */
-			unsorted_string_list_remove(hooks, hook_name);
+			unsorted_string_list_remove(hooks, hook_name, 0);
 			string_list_append(hooks, hook_name);
 		}
 	} else if (!strcmp(subkey, "command")) {
@@ -186,7 +178,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			break;
 		case 1: /* enabled: undo a prior disabled entry */
 			unsorted_string_list_remove(&data->disabled_hooks,
-						    hook_name);
+						    hook_name, 0);
 			break;
 		default:
 			break; /* ignore unrecognised values */
diff --git a/string-list.c b/string-list.c
index fffa2ad4b6..d260b873c8 100644
--- a/string-list.c
+++ b/string-list.c
@@ -281,6 +281,15 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_
 	list->nr--;
 }
 
+void unsorted_string_list_remove(struct string_list *list, const char *str,
+				 int free_util)
+{
+	struct string_list_item *item = unsorted_string_list_lookup(list, str);
+	if (item)
+		unsorted_string_list_delete_item(list, item - list->items,
+						 free_util);
+}
+
 /*
  * append a substring [p..end] to list; return number of things it
  * appended to the list.
diff --git a/string-list.h b/string-list.h
index 3ad862a187..b86ee7c099 100644
--- a/string-list.h
+++ b/string-list.h
@@ -265,6 +265,14 @@ struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
  */
 void unsorted_string_list_delete_item(struct string_list *list, int i, int free_util);
 
+/**
+ * Remove the first item matching `str` from an unsorted string_list.
+ * No-op if `str` is not found. If `free_util` is non-zero, the `util`
+ * pointer of the removed item is freed before deletion.
+ */
+void unsorted_string_list_remove(struct string_list *list, const char *str,
+				 int free_util);
+
 /**
  * Split string into substrings on characters in `delim` and append the
  * substrings to `list`.  The input string is not modified.
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v2 02/10] hook: fix minor style issues

[Adrian Ratiu]

Fix some minor style nits pointed by Patrick, Junio and Eric:
  * Use CALLOC_ARRAY instead of xcalloc.
  * Init struct members during declaration.
  * Simplify if condition boolean logic.
  * Missing curly braces in if/else stmts.
  * Unnecessary header includes.
  * Capitalization and full-stop in error/warn messages.
  * Curly brace on separate line when defining struct.
  * Comment spelling: free'd -> freed.

These contain no logic changes, the code behaves the same as before.

Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c         |  6 ++----
 builtin/receive-pack.c | 11 +++++++----
 hook.c                 | 25 +++++++++++++------------
 hook.h                 |  3 +--
 refs.c                 |  3 ++-
 t/t1800-hook.sh        |  2 +-
 transport.c            |  3 ++-
 7 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index 83020dfb4f..e641614b84 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -5,8 +5,6 @@
 #include "gettext.h"
 #include "hook.h"
 #include "parse-options.h"
-#include "strvec.h"
-#include "abspath.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
 	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
@@ -51,7 +49,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	 * 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."),
+		usage_msg_opt(_("you must specify a hook event name to list"),
 			      builtin_hook_list_usage, list_options);
 
 	hookname = argv[0];
@@ -59,7 +57,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	head = list_hooks(repo, hookname, NULL);
 
 	if (!head->nr) {
-		warning(_("No hooks found for event '%s'"), hookname);
+		warning(_("no hooks found for event '%s'"), hookname);
 		ret = 1; /* no hooks found */
 		goto cleanup;
 	}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e34edff406..991d6ca7d5 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -904,7 +904,8 @@ static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_
 static void *receive_hook_feed_state_alloc(void *feed_pipe_ctx)
 {
 	struct receive_hook_feed_state *init_state = feed_pipe_ctx;
-	struct receive_hook_feed_state *data = xcalloc(1, sizeof(*data));
+	struct receive_hook_feed_state *data;
+	CALLOC_ARRAY(data, 1);
 	data->report = init_state->report;
 	data->cmd = init_state->cmd;
 	data->skip_broken = init_state->skip_broken;
@@ -928,7 +929,11 @@ static int run_receive_hook(struct command *commands,
 {
 	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
 	struct command *iter = commands;
-	struct receive_hook_feed_state feed_init_state = { 0 };
+	struct receive_hook_feed_state feed_init_state = {
+		.cmd = commands,
+		.skip_broken = skip_broken,
+		.buf = STRBUF_INIT,
+	};
 	struct async sideband_async;
 	int sideband_async_started = 0;
 	int saved_stderr = -1;
@@ -961,8 +966,6 @@ static int run_receive_hook(struct command *commands,
 	prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started);
 
 	/* set up stdin callback */
-	feed_init_state.cmd = commands;
-	feed_init_state.skip_broken = skip_broken;
 	opt.feed_pipe_ctx = &feed_init_state;
 	opt.feed_pipe = feed_receive_hook_cb;
 	opt.feed_pipe_cb_data_alloc = receive_hook_feed_state_alloc;
diff --git a/hook.c b/hook.c
index 67cc9a66df..349db729f6 100644
--- a/hook.c
+++ b/hook.c
@@ -57,9 +57,9 @@ static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
 	if (!h)
 		return;
 
-	if (h->kind == HOOK_TRADITIONAL)
+	if (h->kind == HOOK_TRADITIONAL) {
 		free((void *)h->u.traditional.path);
-	else if (h->kind == HOOK_CONFIGURED) {
+	} else if (h->kind == HOOK_CONFIGURED) {
 		free((void *)h->u.configured.friendly_name);
 		free((void *)h->u.configured.command);
 	}
@@ -91,7 +91,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	if (!hook_path)
 		return;
 
-	h = xcalloc(1, sizeof(struct hook));
+	CALLOC_ARRAY(h, 1);
 
 	/*
 	 * If the hook is to run in a specific dir, a relative path can
@@ -154,7 +154,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 				strmap_get(&data->event_hooks, value);
 
 			if (!hooks) {
-				hooks = xcalloc(1, sizeof(*hooks));
+				CALLOC_ARRAY(hooks, 1);
 				string_list_init_dup(hooks);
 				strmap_put(&data->event_hooks, value, hooks);
 			}
@@ -227,7 +227,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 	/* Construct the cache from parsed configs. */
 	strmap_for_each_entry(&cb_data.event_hooks, &iter, e) {
 		struct string_list *hook_names = e->value;
-		struct string_list *hooks = xcalloc(1, sizeof(*hooks));
+		struct string_list *hooks;
+		CALLOC_ARRAY(hooks, 1);
 
 		string_list_init_dup(hooks);
 
@@ -281,7 +282,7 @@ static struct strmap *get_hook_config_cache(struct repository *r)
 		 * it just once on the first call.
 		 */
 		if (!r->hook_config_cache) {
-			r->hook_config_cache = xcalloc(1, sizeof(*cache));
+			CALLOC_ARRAY(r->hook_config_cache, 1);
 			strmap_init(r->hook_config_cache);
 			build_hook_config_map(r, r->hook_config_cache);
 		}
@@ -289,9 +290,9 @@ static struct strmap *get_hook_config_cache(struct repository *r)
 	} else {
 		/*
 		 * Out-of-repo calls (no gitdir) allocate and return a temporary
-		 * map cache which gets free'd immediately by the caller.
+		 * cache which gets freed immediately by the caller.
 		 */
-		cache = xcalloc(1, sizeof(*cache));
+		CALLOC_ARRAY(cache, 1);
 		strmap_init(cache);
 		build_hook_config_map(r, cache);
 	}
@@ -311,7 +312,8 @@ static void list_hooks_add_configured(struct repository *r,
 	for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) {
 		const char *friendly_name = configured_hooks->items[i].string;
 		const char *command = configured_hooks->items[i].util;
-		struct hook *hook = xcalloc(1, sizeof(struct hook));
+		struct hook *hook;
+		CALLOC_ARRAY(hook, 1);
 
 		if (options && options->feed_pipe_cb_data_alloc)
 			hook->feed_pipe_cb_data =
@@ -343,7 +345,7 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 	if (!hookname)
 		BUG("null hookname was provided to hook_list()!");
 
-	hook_head = xmalloc(sizeof(struct string_list));
+	CALLOC_ARRAY(hook_head, 1);
 	string_list_init_dup(hook_head);
 
 	/* Add hooks from the config, e.g. hook.myhook.event = pre-commit */
@@ -493,8 +495,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
 	 * Ensure cb_data copy and free functions are either provided together,
 	 * or neither one is provided.
 	 */
-	if ((options->feed_pipe_cb_data_alloc && !options->feed_pipe_cb_data_free) ||
-	    (!options->feed_pipe_cb_data_alloc && options->feed_pipe_cb_data_free))
+	if (!options->feed_pipe_cb_data_alloc != !options->feed_pipe_cb_data_free)
 		BUG("feed_pipe_cb_data_alloc and feed_pipe_cb_data_free must be set together");
 
 	if (options->invoked_hook)
diff --git a/hook.h b/hook.h
index e949f5d488..40823ebde7 100644
--- a/hook.h
+++ b/hook.h
@@ -46,8 +46,7 @@ struct hook {
 typedef void (*cb_data_free_fn)(void *data);
 typedef void *(*cb_data_alloc_fn)(void *init_ctx);
 
-struct run_hooks_opt
-{
+struct run_hooks_opt {
 	/* Environment vars to be set for each hook */
 	struct strvec env;
 
diff --git a/refs.c b/refs.c
index 6fb8f9d10c..33c7961802 100644
--- a/refs.c
+++ b/refs.c
@@ -2591,7 +2591,8 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
 
 static void *transaction_feed_cb_data_alloc(void *feed_pipe_ctx UNUSED)
 {
-	struct transaction_feed_cb_data *data = xmalloc(sizeof(*data));
+	struct transaction_feed_cb_data *data;
+	CALLOC_ARRAY(data, 1);
 	strbuf_init(&data->buf, 0);
 	data->index = 0;
 	return data;
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index b1583e9ef9..952bf97b86 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -34,7 +34,7 @@ test_expect_success 'git hook usage' '
 
 test_expect_success 'git hook list: nonexistent hook' '
 	cat >stderr.expect <<-\EOF &&
-	warning: No hooks found for event '\''test-hook'\''
+	warning: no hooks found for event '\''test-hook'\''
 	EOF
 	test_expect_code 1 git hook list test-hook 2>stderr.actual &&
 	test_cmp stderr.expect stderr.actual
diff --git a/transport.c b/transport.c
index 107f4fa5dc..56a4015389 100644
--- a/transport.c
+++ b/transport.c
@@ -1360,7 +1360,8 @@ static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void
 
 static void *pre_push_hook_data_alloc(void *feed_pipe_ctx)
 {
-	struct feed_pre_push_hook_data *data = xmalloc(sizeof(*data));
+	struct feed_pre_push_hook_data *data;
+	CALLOC_ARRAY(data, 1);
 	strbuf_init(&data->buf, 0);
 	data->refs = (struct ref *)feed_pipe_ctx;
 	return data;
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH v2 02/10] hook: fix minor style issues

[Patrick Steinhardt]

On Fri, Mar 20, 2026 at 01:52:03PM +0200, Adrian Ratiu wrote:
> Fix some minor style nits pointed by Patrick, Junio and Eric:

Tiny nit, not worth rerolling over: "pointed out by"

> diff --git a/builtin/hook.c b/builtin/hook.c
> index 83020dfb4f..e641614b84 100644
> --- a/builtin/hook.c
> +++ b/builtin/hook.c
> @@ -5,8 +5,6 @@
>  #include "gettext.h"
>  #include "hook.h"
>  #include "parse-options.h"
> -#include "strvec.h"
> -#include "abspath.h"

Another thing we could address while at it is to sort the headers
(except "builtin.h" of course). Feel free to ignore though.

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index e34edff406..991d6ca7d5 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -904,7 +904,8 @@ static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_
>  static void *receive_hook_feed_state_alloc(void *feed_pipe_ctx)
>  {
>  	struct receive_hook_feed_state *init_state = feed_pipe_ctx;
> -	struct receive_hook_feed_state *data = xcalloc(1, sizeof(*data));
> +	struct receive_hook_feed_state *data;
> +	CALLOC_ARRAY(data, 1);

I think it might help the reader to have an empty line between variables
and logic.

> @@ -928,7 +929,11 @@ static int run_receive_hook(struct command *commands,
>  {
>  	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
>  	struct command *iter = commands;
> -	struct receive_hook_feed_state feed_init_state = { 0 };
> +	struct receive_hook_feed_state feed_init_state = {
> +		.cmd = commands,
> +		.skip_broken = skip_broken,
> +		.buf = STRBUF_INIT,
> +	};

Interesting. The buffer here isn't only a style fix, but an actual bug
fix, isn't it?

> diff --git a/hook.c b/hook.c
> index 67cc9a66df..349db729f6 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -227,7 +227,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
>  	/* Construct the cache from parsed configs. */
>  	strmap_for_each_entry(&cb_data.event_hooks, &iter, e) {
>  		struct string_list *hook_names = e->value;
> -		struct string_list *hooks = xcalloc(1, sizeof(*hooks));
> +		struct string_list *hooks;
> +		CALLOC_ARRAY(hooks, 1);
>  
>  		string_list_init_dup(hooks);
>  

Same nit here: I'd move the empty line to come before `CALLOC_ARRAY()`.

> @@ -311,7 +312,8 @@ static void list_hooks_add_configured(struct repository *r,
>  	for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) {
>  		const char *friendly_name = configured_hooks->items[i].string;
>  		const char *command = configured_hooks->items[i].util;
> -		struct hook *hook = xcalloc(1, sizeof(struct hook));
> +		struct hook *hook;
> +		CALLOC_ARRAY(hook, 1);
>  
>  		if (options && options->feed_pipe_cb_data_alloc)
>  			hook->feed_pipe_cb_data =

And here.

None of my nits are really important, so please feel free to address or
ignore them as you like.

Patrick

Re: [PATCH v2 02/10] hook: fix minor style issues

[Adrian Ratiu]

On Tue, 24 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Fri, Mar 20, 2026 at 01:52:03PM +0200, Adrian Ratiu wrote:
>> diff --git a/builtin/hook.c b/builtin/hook.c
>> index 83020dfb4f..e641614b84 100644
>> --- a/builtin/hook.c
>> +++ b/builtin/hook.c
>> @@ -5,8 +5,6 @@
>>  #include "gettext.h"
>>  #include "hook.h"
>>  #include "parse-options.h"
>> -#include "strvec.h"
>> -#include "abspath.h"
>pp
> Another thing we could address while at it is to sort the headers
> (except "builtin.h" of course). Feel free to ignore though.
>

I'll fix all the nits you pointed out in v3, no worries. :)

<snip>

>> @@ -928,7 +929,11 @@ static int run_receive_hook(struct command *commands,
>>  {
>>  	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
>>  	struct command *iter = commands;
>> -	struct receive_hook_feed_state feed_init_state = { 0 };
>> +	struct receive_hook_feed_state feed_init_state = {
>> +		.cmd = commands,
>> +		.skip_broken = skip_broken,
>> +		.buf = STRBUF_INIT,
>> +	};
>
> Interesting. The buffer here isn't only a style fix, but an actual bug
> fix, isn't it?

In theory yes, it's a bug. I'll fix it in a separate commit.

In practice there is no difference because this is passed to
receive_hook_feed_state_alloc() which creates "copies" for each hook and
properly initializes each copy with strbuf_init(&data->buf, 0);

(The differnce is between .buf being NULL vs pointing to a static array
strbuf_slopbuf with one element containing NULL )

I'll fix it anyway and explain in a separate commit to avoid confusion.

Thanks for spotting this!

[PATCH v2 03/10] hook: rename cb_data_free/alloc -> hook_data_free/alloc

[Adrian Ratiu]

Rename the hook callback function types to use the hook prefix.

This is a style fix with no logic changes.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c |  4 ++--
 hook.h | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hook.c b/hook.c
index 349db729f6..afa8db21a0 100644
--- a/hook.c
+++ b/hook.c
@@ -52,7 +52,7 @@ const char *find_hook(struct repository *r, const char *name)
 	return path.buf;
 }
 
-static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
+static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
 {
 	if (!h)
 		return;
@@ -70,7 +70,7 @@ static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
 	free(h);
 }
 
-void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free)
+void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free)
 {
 	struct string_list_item *item;
 
diff --git a/hook.h b/hook.h
index 40823ebde7..94649218a1 100644
--- a/hook.h
+++ b/hook.h
@@ -43,8 +43,8 @@ struct hook {
 	void *feed_pipe_cb_data;
 };
 
-typedef void (*cb_data_free_fn)(void *data);
-typedef void *(*cb_data_alloc_fn)(void *init_ctx);
+typedef void (*hook_data_free_fn)(void *data);
+typedef void *(*hook_data_alloc_fn)(void *init_ctx);
 
 struct run_hooks_opt {
 	/* Environment vars to be set for each hook */
@@ -131,14 +131,14 @@ struct run_hooks_opt {
 	 *
 	 * The `feed_pipe_ctx` pointer can be used to pass initialization data.
 	 */
-	cb_data_alloc_fn feed_pipe_cb_data_alloc;
+	hook_data_alloc_fn feed_pipe_cb_data_alloc;
 
 	/**
 	 * Called to free the memory initialized by `feed_pipe_cb_data_alloc`.
 	 *
 	 * Must always be provided when `feed_pipe_cb_data_alloc` is provided.
 	 */
-	cb_data_free_fn feed_pipe_cb_data_free;
+	hook_data_free_fn feed_pipe_cb_data_free;
 };
 
 #define RUN_HOOKS_OPT_INIT { \
@@ -188,7 +188,7 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
  * Frees the memory allocated for the hook list, including the `struct hook`
  * items and their internal state.
  */
-void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free);
+void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
 
 /**
  * Frees the hook configuration cache stored in `struct repository`.
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v2 04/10] hook: detect & emit two more bugs

[Adrian Ratiu]

Trigger a bug when an unknown hook type is encountered while
setting up hook execution.

Also issue a bug if a configured hook is enabled without a cmd.

Mostly useful for defensive coding.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hook.c b/hook.c
index afa8db21a0..6dfaa7e9b1 100644
--- a/hook.c
+++ b/hook.c
@@ -408,7 +408,11 @@ static int pick_next_hook(struct child_process *cp,
 	} else if (h->kind == HOOK_CONFIGURED) {
 		/* to enable oneliners, let config-specified hooks run in shell. */
 		cp->use_shell = true;
+		if (!h->u.configured.command)
+			BUG("non-disabled HOOK_CONFIGURED hook has no command");
 		strvec_push(&cp->args, h->u.configured.command);
+	} else {
+		BUG("unknown hook kind");
 	}
 
 	if (!cp->args.nr)
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v2 05/10] hook: replace hook_list_clear() -> string_list_clear_func()

[Adrian Ratiu]

Replace the custom function with string_list_clear_func() which
is a more common pattern for clearing a string_list.

To be able to do this, rework hook_clear() into hook_free(), so
it can be passed to string_list_clear_func().

A slight complication is the need to keep a copy of the internal
cb data free() pointer, however I think it's worth it since the
API becomes cleaner, e.g. no more calls with NULL function args
like hook_list_clear(hooks, NULL).

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c |  2 +-
 hook.c         | 44 ++++++++++++++++++++++++++------------------
 hook.h         | 20 ++++++++++++++------
 3 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index e641614b84..54b737990b 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -78,7 +78,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	}
 
 cleanup:
-	hook_list_clear(head, NULL);
+	string_list_clear_func(head, hook_free);
 	free(head);
 	return ret;
 }
diff --git a/hook.c b/hook.c
index 6dfaa7e9b1..f6bb1999ae 100644
--- a/hook.c
+++ b/hook.c
@@ -52,8 +52,14 @@ const char *find_hook(struct repository *r, const char *name)
 	return path.buf;
 }
 
-static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
+/*
+ * Frees a struct hook stored as the util pointer of a string_list_item.
+ * Suitable for use as a string_list_clear_func_t callback.
+ */
+void hook_free(void *p, const char *str UNUSED)
 {
+	struct hook *h = p;
+
 	if (!h)
 		return;
 
@@ -64,22 +70,12 @@ static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
 		free((void *)h->u.configured.command);
 	}
 
-	if (cb_data_free)
-		cb_data_free(h->feed_pipe_cb_data);
+	if (h->data_free && h->feed_pipe_cb_data)
+		h->data_free(h->feed_pipe_cb_data);
 
 	free(h);
 }
 
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free)
-{
-	struct string_list_item *item;
-
-	for_each_string_list_item(item, hooks)
-		hook_clear(item->util, cb_data_free);
-
-	string_list_clear(hooks, 0);
-}
-
 /* Helper to detect and add default "traditional" hooks from the hookdir. */
 static void list_hooks_add_default(struct repository *r, const char *hookname,
 				   struct string_list *hook_list,
@@ -100,9 +96,15 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	if (options && options->dir)
 		hook_path = absolute_path(hook_path);
 
-	/* Setup per-hook internal state cb data */
-	if (options && options->feed_pipe_cb_data_alloc)
+	/*
+	 * Setup per-hook internal state callback data.
+	 * When provided, the alloc/free callbacks are always provided
+	 * together, so use them to alloc/free the internal hook state.
+	 */
+	if (options && options->feed_pipe_cb_data_alloc) {
 		h->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx);
+		h->data_free = options->feed_pipe_cb_data_free;
+	}
 
 	h->kind = HOOK_TRADITIONAL;
 	h->u.traditional.path = xstrdup(hook_path);
@@ -315,10 +317,16 @@ static void list_hooks_add_configured(struct repository *r,
 		struct hook *hook;
 		CALLOC_ARRAY(hook, 1);
 
-		if (options && options->feed_pipe_cb_data_alloc)
+		/*
+		 * When provided, the alloc/free callbacks are always provided
+		 * together, so use them to alloc/free the internal hook state.
+		 */
+		if (options && options->feed_pipe_cb_data_alloc) {
 			hook->feed_pipe_cb_data =
 				options->feed_pipe_cb_data_alloc(
 					options->feed_pipe_ctx);
+			hook->data_free = options->feed_pipe_cb_data_free;
+		}
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
@@ -361,7 +369,7 @@ int hook_exists(struct repository *r, const char *name)
 {
 	struct string_list *hooks = list_hooks(r, name, NULL);
 	int exists = hooks->nr > 0;
-	hook_list_clear(hooks, NULL);
+	string_list_clear_func(hooks, hook_free);
 	free(hooks);
 	return exists;
 }
@@ -515,7 +523,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
 	run_processes_parallel(&opts);
 	ret = cb_data.rc;
 cleanup:
-	hook_list_clear(cb_data.hook_command_list, options->feed_pipe_cb_data_free);
+	string_list_clear_func(cb_data.hook_command_list, hook_free);
 	free(cb_data.hook_command_list);
 	run_hooks_opt_clear(options);
 	return ret;
diff --git a/hook.h b/hook.h
index 94649218a1..74f4701176 100644
--- a/hook.h
+++ b/hook.h
@@ -7,6 +7,9 @@
 
 struct repository;
 
+typedef void (*hook_data_free_fn)(void *data);
+typedef void *(*hook_data_alloc_fn)(void *init_ctx);
+
 /**
  * Represents a hook command to be run.
  * Hooks can be:
@@ -41,10 +44,15 @@ struct hook {
 	 * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it.
 	 */
 	void *feed_pipe_cb_data;
-};
 
-typedef void (*hook_data_free_fn)(void *data);
-typedef void *(*hook_data_alloc_fn)(void *init_ctx);
+	/**
+	 * Callback to free `feed_pipe_cb_data`.
+	 *
+	 * It is called automatically and points to the `feed_pipe_cb_data_free`
+	 * provided via the `run_hook_opt` parameter.
+	 */
+	hook_data_free_fn data_free;
+};
 
 struct run_hooks_opt {
 	/* Environment vars to be set for each hook */
@@ -185,10 +193,10 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 			       struct run_hooks_opt *options);
 
 /**
- * Frees the memory allocated for the hook list, including the `struct hook`
- * items and their internal state.
+ * Frees a struct hook stored as the util pointer of a string_list_item.
+ * Suitable for use as a string_list_clear_func_t callback.
  */
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
+void hook_free(void *p, const char *str);
 
 /**
  * Frees the hook configuration cache stored in `struct repository`.
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH v2 05/10] hook: replace hook_list_clear() -> string_list_clear_func()

[Patrick Steinhardt]

On Fri, Mar 20, 2026 at 01:52:06PM +0200, Adrian Ratiu wrote:
> diff --git a/hook.c b/hook.c
> index 6dfaa7e9b1..f6bb1999ae 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -52,8 +52,14 @@ const char *find_hook(struct repository *r, const char *name)
>  	return path.buf;
>  }
>  
> -static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
> +/*
> + * Frees a struct hook stored as the util pointer of a string_list_item.
> + * Suitable for use as a string_list_clear_func_t callback.
> + */

This comment should probably live in the header. I also wonder whether
this wrapper isn't a bit too specific to freeing hooks with a string
list. Maybe it would be preferable to expose a "proper" `hook_free()`
function that only takes a hook, and then provide a small wrapper
function for freeing in the string list?

If so it feels like we're going a bit full circle though. Maybe the
original code wasn't all that bad in the first place?

Patrick

Re: [PATCH v2 05/10] hook: replace hook_list_clear() -> string_list_clear_func()

[Adrian Ratiu]

On Tue, 24 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Fri, Mar 20, 2026 at 01:52:06PM +0200, Adrian Ratiu wrote:
>> diff --git a/hook.c b/hook.c
>> index 6dfaa7e9b1..f6bb1999ae 100644
>> --- a/hook.c
>> +++ b/hook.c
>> @@ -52,8 +52,14 @@ const char *find_hook(struct repository *r, const char *name)
>>  	return path.buf;
>>  }
>>  
>> -static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
>> +/*
>> + * Frees a struct hook stored as the util pointer of a string_list_item.
>> + * Suitable for use as a string_list_clear_func_t callback.
>> + */
>
> This comment should probably live in the header. I also wonder whether
> this wrapper isn't a bit too specific to freeing hooks with a string
> list. Maybe it would be preferable to expose a "proper" `hook_free()`
> function that only takes a hook, and then provide a small wrapper
> function for freeing in the string list?
>
> If so it feels like we're going a bit full circle though. Maybe the
> original code wasn't all that bad in the first place?

I prefer this new design (suggested by you), because:

1. We use the generic string_list_clear_func() API.

2. Each struct hook owns its data_free callback, which means that callers
don't need to keep track of internal state (e.g. when to pass NULL to
skip cleanup).

3. The hook API itself is cleaner for hook.[ch] users, because it's
always string_list_clear_func(head, hook_free); regardless of context.

So if it's ok with you, let's use your new design. :)

I'll move the comment to the header (it's already there, I just forgot
to remove the duplicated comment in hook.c above the function
definition).

Re: [PATCH v2 05/10] hook: replace hook_list_clear() -> string_list_clear_func()

[Patrick Steinhardt]

On Wed, Mar 25, 2026 at 12:33:21AM +0200, Adrian Ratiu wrote:
> On Tue, 24 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> > On Fri, Mar 20, 2026 at 01:52:06PM +0200, Adrian Ratiu wrote:
> >> diff --git a/hook.c b/hook.c
> >> index 6dfaa7e9b1..f6bb1999ae 100644
> >> --- a/hook.c
> >> +++ b/hook.c
> >> @@ -52,8 +52,14 @@ const char *find_hook(struct repository *r, const char *name)
> >>  	return path.buf;
> >>  }
> >>  
> >> -static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
> >> +/*
> >> + * Frees a struct hook stored as the util pointer of a string_list_item.
> >> + * Suitable for use as a string_list_clear_func_t callback.
> >> + */
> >
> > This comment should probably live in the header. I also wonder whether
> > this wrapper isn't a bit too specific to freeing hooks with a string
> > list. Maybe it would be preferable to expose a "proper" `hook_free()`
> > function that only takes a hook, and then provide a small wrapper
> > function for freeing in the string list?
> >
> > If so it feels like we're going a bit full circle though. Maybe the
> > original code wasn't all that bad in the first place?
> 
> I prefer this new design (suggested by you), because:
> 
> 1. We use the generic string_list_clear_func() API.
> 
> 2. Each struct hook owns its data_free callback, which means that callers
> don't need to keep track of internal state (e.g. when to pass NULL to
> skip cleanup).
> 
> 3. The hook API itself is cleaner for hook.[ch] users, because it's
> always string_list_clear_func(head, hook_free); regardless of context.
> 
> So if it's ok with you, let's use your new design. :)
> 
> I'll move the comment to the header (it's already there, I just forgot
> to remove the duplicated comment in hook.c above the function
> definition).

Fine with me, thanks!

Patrick

[PATCH v2 06/10] hook: make consistent use of friendly-name in docs

[Adrian Ratiu]

Both `name` and `friendly-name` is being used. Standardize on
`friendly-name` for consistency since name is rather generic,
even when used in the hooks namespace.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 Documentation/config/hook.adoc | 30 +++++++++++++++---------------
 Documentation/git-hook.adoc    |  6 +++---
 hook.c                         |  2 +-
 hook.h                         |  2 +-
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc
index 64e845a260..9e78f26439 100644
--- a/Documentation/config/hook.adoc
+++ b/Documentation/config/hook.adoc
@@ -1,23 +1,23 @@
-hook.<name>.command::
-	The command to execute for `hook.<name>`. `<name>` is a unique
-	"friendly" name that identifies this hook. (The hook events that
-	trigger the command are configured with `hook.<name>.event`.) The
-	value can be an executable path or a shell oneliner. If more than
-	one value is specified for the same `<name>`, only the last value
-	parsed is used. See linkgit:git-hook[1].
+hook.<friendly-name>.command::
+	The command to execute for `hook.<friendly-name>`. `<friendly-name>`
+	is a unique name that identifies this hook. The hook events that
+	trigger the command are configured with `hook.<friendly-name>.event`.
+	The value can be an executable path or a shell oneliner. If more than
+	one value is specified for the same `<friendly-name>`, only the last
+	value parsed is used. See linkgit:git-hook[1].
 
-hook.<name>.event::
-	The hook events that trigger `hook.<name>`. The value is the name
-	of a hook event, like "pre-commit" or "update". (See
+hook.<friendly-name>.event::
+	The hook events that trigger `hook.<friendly-name>`. The value is the
+	name of a hook event, like "pre-commit" or "update". (See
 	linkgit:githooks[5] for a complete list of hook events.) On the
-	specified event, the associated `hook.<name>.command` is executed.
-	This is a multi-valued key. To run `hook.<name>` on multiple
+	specified event, the associated `hook.<friendly-name>.command` is executed.
+	This is a multi-valued key. To run `hook.<friendly-name>` on multiple
 	events, specify the key more than once. An empty value resets
 	the list of events, clearing any previously defined events for
-	`hook.<name>`. See linkgit:git-hook[1].
+	`hook.<friendly-name>`. See linkgit:git-hook[1].
 
-hook.<name>.enabled::
-	Whether the hook `hook.<name>` is enabled. Defaults to `true`.
+hook.<friendly-name>.enabled::
+	Whether the hook `hook.<friendly-name>` is enabled. Defaults to `true`.
 	Set to `false` to disable the hook without removing its
 	configuration. This is particularly useful when a hook is defined
 	in a system or global config file and needs to be disabled for a
diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index 12d2701b52..966388660a 100644
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -44,7 +44,7 @@ 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
+`hook.<friendly-name>.event` configs during the configuration parse (see
 linkgit:git-config[1]). Although multiple `hook.linter.event` configs can be
 added, only one `hook.linter.command` event is valid - Git uses "last-one-wins"
 to determine which command to run.
@@ -76,10 +76,10 @@ 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,
+For a full list of hook events which you can set your `hook.<friendly-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
+Git will ignore any `hook.<friendly-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.
 
diff --git a/hook.c b/hook.c
index f6bb1999ae..7f89ae9cc2 100644
--- a/hook.c
+++ b/hook.c
@@ -116,7 +116,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
  * event_hooks: event-name to list of friendly-names map.
- * disabled_hooks: set of friendly-names with hook.name.enabled = false.
+ * disabled_hooks: set of friendly-names with hook.<friendly-name>.enabled = false.
  */
 struct hook_all_config_cb {
 	struct strmap commands;
diff --git a/hook.h b/hook.h
index 74f4701176..ad022821c1 100644
--- a/hook.h
+++ b/hook.h
@@ -14,7 +14,7 @@ typedef void *(*hook_data_alloc_fn)(void *init_ctx);
  * Represents a hook command to be run.
  * Hooks can be:
  * 1. "traditional" (found in the hooks directory)
- * 2. "configured" (defined in Git's configuration via hook.<name>.event).
+ * 2. "configured" (defined in Git's configuration via hook.<friendly-name>.event).
  * The 'kind' field determines which part of the union 'u' is valid.
  */
 struct hook {
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v2 07/10] t1800: add test to verify hook execution ordering

[Adrian Ratiu]

There is a documented expectation that configured hooks are
run before the hook from the hookdir. Add a test for it.

While at it, I noticed that `git hook list -h` runs twice
in the `git hook usage` test, so remove one invocation.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 t/t1800-hook.sh | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 952bf97b86..7eee84fc39 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -25,7 +25,6 @@ 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 &&
 	test_expect_code 129 git hook list &&
 	test_expect_code 129 git hook list -h &&
@@ -381,6 +380,34 @@ test_expect_success 'globally disabled hook can be re-enabled locally' '
 	test_cmp expected actual
 '
 
+test_expect_success 'configured hooks run before hookdir hook' '
+	setup_hookdir &&
+	test_config hook.first.event "pre-commit" &&
+	test_config hook.first.command "echo first" &&
+	test_config hook.second.event "pre-commit" &&
+	test_config hook.second.command "echo second" &&
+
+	cat >expected <<-\EOF &&
+	first
+	second
+	hook from hookdir
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual &&
+
+	# "Legacy Hook" is the output of the hookdir pre-commit script
+	# written by setup_hookdir() above.
+	cat >expected <<-\EOF &&
+	first
+	second
+	"Legacy Hook"
+	EOF
+
+	git hook run pre-commit 2>actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'git hook run a hook with a bad shebang' '
 	test_when_finished "rm -rf bad-hooks" &&
 	mkdir bad-hooks &&
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v2 08/10] hook: introduce hook_config_cache_entry for per-hook data

[Adrian Ratiu]

Replace the bare `char *command` util pointer stored in each string_list
item with a heap-allocated `struct hook_config_cache_entry` that carries
that command string.

This is just a refactoring with no behavior changes, to give the cache
entry room to grow, so it can carry the additional hook metadata we'll
be adding in the following commits.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/hook.c b/hook.c
index 7f89ae9cc2..49f1ebe17a 100644
--- a/hook.c
+++ b/hook.c
@@ -112,6 +112,15 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	string_list_append(hook_list, hook_path)->util = h;
 }
 
+/*
+ * Cache entry stored as the .util pointer of string_list items inside the
+ * hook config cache. For now carries only the command for the hook. Next
+ * commits will add more data.
+ */
+struct hook_config_cache_entry {
+	char *command;
+};
+
 /*
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
@@ -206,7 +215,12 @@ void hook_cache_clear(struct strmap *cache)
 
 	strmap_for_each_entry(cache, &iter, e) {
 		struct string_list *hooks = e->value;
-		string_list_clear(hooks, 1); /* free util (command) pointers */
+		for (size_t i = 0; i < hooks->nr; i++) {
+			struct hook_config_cache_entry *entry = hooks->items[i].util;
+			free(entry->command);
+			free(entry);
+		}
+		string_list_clear(hooks, 0);
 		free(hooks);
 	}
 	strmap_clear(cache, 0);
@@ -236,6 +250,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 
 		for (size_t i = 0; i < hook_names->nr; i++) {
 			const char *hname = hook_names->items[i].string;
+			struct hook_config_cache_entry *entry;
 			char *command;
 
 			/* filter out disabled hooks */
@@ -249,9 +264,10 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 				      "'hook.%s.event' must be removed;"
 				      " aborting."), hname, hname);
 
-			/* util stores the command; owned by the cache. */
-			string_list_append(hooks, hname)->util =
-				xstrdup(command);
+			/* util stores a cache entry; owned by the cache. */
+			CALLOC_ARRAY(entry, 1);
+			entry->command = xstrdup(command);
+			string_list_append(hooks, hname)->util = entry;
 		}
 
 		strmap_put(cache, e->key, hooks);
@@ -313,7 +329,7 @@ static void list_hooks_add_configured(struct repository *r,
 	/* Iterate through configured hooks and initialize internal states */
 	for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) {
 		const char *friendly_name = configured_hooks->items[i].string;
-		const char *command = configured_hooks->items[i].util;
+		struct hook_config_cache_entry *entry = configured_hooks->items[i].util;
 		struct hook *hook;
 		CALLOC_ARRAY(hook, 1);
 
@@ -330,7 +346,7 @@ static void list_hooks_add_configured(struct repository *r,
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
-		hook->u.configured.command = xstrdup(command);
+		hook->u.configured.command = xstrdup(entry->command);
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v2 09/10] hook: show config scope in git hook list

[Adrian Ratiu]

Users running "git hook list" can see which hooks are configured but
have no way to tell at which config scope (local, global, system...)
each hook was defined.

Store the scope from ctx->kvi->scope in the single-pass config callback,
then carry it through the cache to the hook structs, so we can expose it
to users via the "git hook list --show-scope" flag, which mirrors the
existing git config --show-scope convention.

Without the flag the output is unchanged.

The scope is printed as a tab-separated prefix (like "git config --show-scope"),
making it unambiguously machine-parseable even when the friendly name
contains spaces.

Example usage:
$ git hook list --show-scope pre-commit
global	linter
local	no-leaks
hook from hookdir

Traditional hooks from the hookdir are unaffected by --show-scope since
the config scope concept does not apply to them.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 Documentation/git-hook.adoc | 10 ++++++++--
 builtin/hook.c              | 14 ++++++++++++--
 hook.c                      | 24 ++++++++++++++++++++----
 hook.h                      |  2 ++
 t/t1800-hook.sh             | 19 +++++++++++++++++++
 5 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index 966388660a..e7d399ae57 100644
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
-'git hook' list [-z] <hook-name>
+'git hook' list [-z] [--show-scope] <hook-name>
 
 DESCRIPTION
 -----------
@@ -113,7 +113,7 @@ Any positional arguments to the hook should be passed after a
 mandatory `--` (or `--end-of-options`, see linkgit:gitcli[7]). See
 linkgit:githooks[5] for arguments hooks might expect (if any).
 
-list [-z]::
+list [-z] [--show-scope]::
 	Print a list of hooks which will be run on `<hook-name>` event. If no
 	hooks are configured for that event, print a warning and return 1.
 	Use `-z` to terminate output lines with NUL instead of newlines.
@@ -134,6 +134,12 @@ OPTIONS
 -z::
 	Terminate "list" output lines with NUL instead of newlines.
 
+--show-scope::
+	For "list"; prefix each configured hook's friendly name with a
+	tab-separated config scope (e.g. `local`, `global`, `system`),
+	mirroring the output style of `git config --show-scope`. Traditional
+	hooks from the hookdir are unaffected.
+
 WRAPPERS
 --------
 
diff --git a/builtin/hook.c b/builtin/hook.c
index 54b737990b..4cc65a0dc5 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -9,7 +9,7 @@
 #define BUILTIN_HOOK_RUN_USAGE \
 	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
 #define BUILTIN_HOOK_LIST_USAGE \
-	N_("git hook list [-z] <hook-name>")
+	N_("git hook list [-z] [--show-scope] <hook-name>")
 
 static const char * const builtin_hook_usage[] = {
 	BUILTIN_HOOK_RUN_USAGE,
@@ -33,11 +33,14 @@ static int list(int argc, const char **argv, const char *prefix,
 	struct string_list_item *item;
 	const char *hookname = NULL;
 	int line_terminator = '\n';
+	int show_scope = 0;
 	int ret = 0;
 
 	struct option list_options[] = {
 		OPT_SET_INT('z', NULL, &line_terminator,
 			    N_("use NUL as line terminator"), '\0'),
+		OPT_BOOL(0, "show-scope", &show_scope,
+			 N_("show the config scope that defined each hook")),
 		OPT_END(),
 	};
 
@@ -70,7 +73,14 @@ static int list(int argc, const char **argv, const char *prefix,
 			printf("%s%c", _("hook from hookdir"), line_terminator);
 			break;
 		case HOOK_CONFIGURED:
-			printf("%s%c", h->u.configured.friendly_name, line_terminator);
+			if (show_scope)
+				printf("%s\t%s%c",
+				       config_scope_name(h->u.configured.scope),
+				       h->u.configured.friendly_name,
+				       line_terminator);
+			else
+				printf("%s%c", h->u.configured.friendly_name,
+				       line_terminator);
 			break;
 		default:
 			BUG("unknown hook kind");
diff --git a/hook.c b/hook.c
index 49f1ebe17a..aa08c38c27 100644
--- a/hook.c
+++ b/hook.c
@@ -114,11 +114,11 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 
 /*
  * Cache entry stored as the .util pointer of string_list items inside the
- * hook config cache. For now carries only the command for the hook. Next
- * commits will add more data.
+ * hook config cache.
  */
 struct hook_config_cache_entry {
 	char *command;
+	enum config_scope scope;
 };
 
 /*
@@ -135,7 +135,7 @@ struct hook_all_config_cb {
 
 /* repo_config() callback that collects all hook.* configuration in one pass. */
 static int hook_config_lookup_all(const char *key, const char *value,
-				  const struct config_context *ctx UNUSED,
+				  const struct config_context *ctx,
 				  void *cb_data)
 {
 	struct hook_all_config_cb *data = cb_data;
@@ -172,7 +172,19 @@ static int hook_config_lookup_all(const char *key, const char *value,
 
 			/* Re-insert if necessary to preserve last-seen order. */
 			unsorted_string_list_remove(hooks, hook_name, 0);
-			string_list_append(hooks, hook_name);
+
+			if (!ctx->kvi)
+				BUG("hook config callback called without key-value info");
+
+			/*
+			 * Stash the config scope in the util pointer for
+			 * later retrieval in build_hook_config_map(). This
+			 * intermediate struct is transient and never leaves
+			 * that function, so we pack the enum value into the
+			 * pointer rather than heap-allocating a wrapper.
+			 */
+			string_list_append(hooks, hook_name)->util =
+				(void *)(uintptr_t)ctx->kvi->scope;
 		}
 	} else if (!strcmp(subkey, "command")) {
 		/* Store command overwriting the old value */
@@ -250,6 +262,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 
 		for (size_t i = 0; i < hook_names->nr; i++) {
 			const char *hname = hook_names->items[i].string;
+			enum config_scope scope =
+				(enum config_scope)(uintptr_t)hook_names->items[i].util;
 			struct hook_config_cache_entry *entry;
 			char *command;
 
@@ -267,6 +281,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 			/* util stores a cache entry; owned by the cache. */
 			CALLOC_ARRAY(entry, 1);
 			entry->command = xstrdup(command);
+			entry->scope = scope;
 			string_list_append(hooks, hname)->util = entry;
 		}
 
@@ -347,6 +362,7 @@ static void list_hooks_add_configured(struct repository *r,
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
 		hook->u.configured.command = xstrdup(entry->command);
+		hook->u.configured.scope = entry->scope;
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
diff --git a/hook.h b/hook.h
index ad022821c1..92e9faf9bb 100644
--- a/hook.h
+++ b/hook.h
@@ -1,5 +1,6 @@
 #ifndef HOOK_H
 #define HOOK_H
+#include "config.h"
 #include "strvec.h"
 #include "run-command.h"
 #include "string-list.h"
@@ -29,6 +30,7 @@ struct hook {
 		struct {
 			const char *friendly_name;
 			const char *command;
+			enum config_scope scope;
 		} configured;
 	} u;
 
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 7eee84fc39..22cca15fda 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -408,6 +408,25 @@ test_expect_success 'configured hooks run before hookdir hook' '
 	test_cmp expected actual
 '
 
+test_expect_success 'git hook list --show-scope shows config scope' '
+	test_config_global hook.global-hook.command "echo global" &&
+	test_config_global hook.global-hook.event test-hook --add &&
+	test_config hook.local-hook.command "echo local" &&
+	test_config hook.local-hook.event test-hook --add &&
+
+	cat >expected <<-\EOF &&
+	global	global-hook
+	local	local-hook
+	EOF
+	git hook list --show-scope test-hook >actual &&
+	test_cmp expected actual &&
+
+	# without --show-scope the scope must not appear
+	git hook list test-hook >actual &&
+	test_grep ! "^global	" actual &&
+	test_grep ! "^local	" actual
+'
+
 test_expect_success 'git hook run a hook with a bad shebang' '
 	test_when_finished "rm -rf bad-hooks" &&
 	mkdir bad-hooks &&
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH v2 09/10] hook: show config scope in git hook list

[Patrick Steinhardt]

On Fri, Mar 20, 2026 at 01:52:10PM +0200, Adrian Ratiu wrote:
> diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
> index 966388660a..e7d399ae57 100644
> --- a/Documentation/git-hook.adoc
> +++ b/Documentation/git-hook.adoc
> @@ -9,7 +9,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
> -'git hook' list [-z] <hook-name>
> +'git hook' list [-z] [--show-scope] <hook-name>
>  
>  DESCRIPTION
>  -----------

Taking a random patch that relates to the git-hook(1) command. I was
wondering whether we want to introduce another change here that will
cause git-hook(1) to bail out when given an unknown hook name.

I know that we explicitly want to allow having custom hook events, but I
would argue that 99% of all invocations will use any of Git's own hook
events. And given that it's really easy to misspell the "prereceive"
hook (which really is "pre-receive") I think it would be nice if we told
the user that it's an unknown hook instead of silently doing nothing.

To cover the original use case we could then add something like
"--allow-unknown-hook-name" to make the caller explicitly accept non-Git
hook events.

> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
> index 7eee84fc39..22cca15fda 100755
> --- a/t/t1800-hook.sh
> +++ b/t/t1800-hook.sh
> @@ -408,6 +408,25 @@ test_expect_success 'configured hooks run before hookdir hook' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'git hook list --show-scope shows config scope' '
> +	test_config_global hook.global-hook.command "echo global" &&
> +	test_config_global hook.global-hook.event test-hook --add &&
> +	test_config hook.local-hook.command "echo local" &&
> +	test_config hook.local-hook.event test-hook --add &&
> +
> +	cat >expected <<-\EOF &&
> +	global	global-hook
> +	local	local-hook
> +	EOF
> +	git hook list --show-scope test-hook >actual &&
> +	test_cmp expected actual &&
> +
> +	# without --show-scope the scope must not appear
> +	git hook list test-hook >actual &&
> +	test_grep ! "^global	" actual &&
> +	test_grep ! "^local	" actual
> +'

Do we also want to add a hook discovered via ".git/hooks" to show how it
interacts with the new flag?

Patrick

Re: [PATCH v2 09/10] hook: show config scope in git hook list

[Adrian Ratiu]

On Tue, 24 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Fri, Mar 20, 2026 at 01:52:10PM +0200, Adrian Ratiu wrote:
>> diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
>> index 966388660a..e7d399ae57 100644
>> --- a/Documentation/git-hook.adoc
>> +++ b/Documentation/git-hook.adoc
>> @@ -9,7 +9,7 @@ SYNOPSIS
>>  --------
>>  [verse]
>>  'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
>> -'git hook' list [-z] <hook-name>
>> +'git hook' list [-z] [--show-scope] <hook-name>
>>  
>>  DESCRIPTION
>>  -----------
>
> Taking a random patch that relates to the git-hook(1) command. I was
> wondering whether we want to introduce another change here that will
> cause git-hook(1) to bail out when given an unknown hook name.
>
> I know that we explicitly want to allow having custom hook events, but I
> would argue that 99% of all invocations will use any of Git's own hook
> events. And given that it's really easy to misspell the "prereceive"
> hook (which really is "pre-receive") I think it would be nice if we told
> the user that it's an unknown hook instead of silently doing nothing.
>
> To cover the original use case we could then add something like
> "--allow-unknown-hook-name" to make the caller explicitly accept non-Git
> hook events.

I think this makes sense.

I just wrote and tested the implementation and it works nicely.

I'll include it in v3.

>> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
>> index 7eee84fc39..22cca15fda 100755
>> --- a/t/t1800-hook.sh
>> +++ b/t/t1800-hook.sh
>> @@ -408,6 +408,25 @@ test_expect_success 'configured hooks run before hookdir hook' '
>>  	test_cmp expected actual
>>  '
>>  
>> +test_expect_success 'git hook list --show-scope shows config scope' '
>> +	test_config_global hook.global-hook.command "echo global" &&
>> +	test_config_global hook.global-hook.event test-hook --add &&
>> +	test_config hook.local-hook.command "echo local" &&
>> +	test_config hook.local-hook.event test-hook --add &&
>> +
>> +	cat >expected <<-\EOF &&
>> +	global	global-hook
>> +	local	local-hook
>> +	EOF
>> +	git hook list --show-scope test-hook >actual &&
>> +	test_cmp expected actual &&
>> +
>> +	# without --show-scope the scope must not appear
>> +	git hook list test-hook >actual &&
>> +	test_grep ! "^global	" actual &&
>> +	test_grep ! "^local	" actual
>> +'
>
> Do we also want to add a hook discovered via ".git/hooks" to show how it
> interacts with the new flag?

Yes, good catch, I'll add it in v3.

[PATCH v2 10/10] hook: show disabled hooks in "git hook list"

[Adrian Ratiu]

Disabled hooks were filtered out of the cache entirely, making them
invisible to "git hook list". Keep them in the cache with a new
"disabled" flag which is propagated to the respective struct hook.

"git hook list" now shows disabled hooks as tab-separated columns,
with the status as a prefix before the name (like scope with
--show-scope). With --show-scope it looks like:

$ git hook list --show-scope pre-commit
global	linter
local	disabled	no-leaks
hook from hookdir

A disabled hook without a command issues a warning instead of the
fatal "hook.X.command must be configured" error. We could also throw
an error, however it seemd a bit excessive to me in this case.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c  | 20 ++++++++++--------
 hook.c          | 54 +++++++++++++++++++++++++++++++++----------------
 hook.h          |  1 +
 t/t1800-hook.sh | 33 +++++++++++++++++++++++++++---
 4 files changed, 80 insertions(+), 28 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index 4cc65a0dc5..f671e7f91a 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -72,16 +72,20 @@ static int list(int argc, const char **argv, const char *prefix,
 		case HOOK_TRADITIONAL:
 			printf("%s%c", _("hook from hookdir"), line_terminator);
 			break;
-		case HOOK_CONFIGURED:
-			if (show_scope)
-				printf("%s\t%s%c",
-				       config_scope_name(h->u.configured.scope),
-				       h->u.configured.friendly_name,
-				       line_terminator);
+		case HOOK_CONFIGURED: {
+			const char *name = h->u.configured.friendly_name;
+			const char *scope = show_scope ?
+				config_scope_name(h->u.configured.scope) : NULL;
+			if (scope)
+				printf("%s\t%s%s%c", scope,
+				       h->u.configured.disabled ? "disabled\t" : "",
+				       name, line_terminator);
 			else
-				printf("%s%c", h->u.configured.friendly_name,
-				       line_terminator);
+				printf("%s%s%c",
+				       h->u.configured.disabled ? "disabled\t" : "",
+				       name, line_terminator);
 			break;
+		}
 		default:
 			BUG("unknown hook kind");
 		}
diff --git a/hook.c b/hook.c
index aa08c38c27..0e09b9a2bb 100644
--- a/hook.c
+++ b/hook.c
@@ -119,6 +119,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 struct hook_config_cache_entry {
 	char *command;
 	enum config_scope scope;
+	unsigned int disabled:1;
 };
 
 /*
@@ -217,8 +218,10 @@ static int hook_config_lookup_all(const char *key, const char *value,
  * every item's string is the hook's friendly-name and its util pointer is
  * the corresponding command string. Both strings are owned by the map.
  *
- * Disabled hooks and hooks missing a command are already filtered out at
- * parse time, so callers can iterate the list directly.
+ * Disabled hooks are kept in the cache with entry->disabled set, so that
+ * "git hook list" can display them. A non-disabled hook missing a command
+ * is fatal; a disabled hook missing a command emits a warning and is kept
+ * in the cache with entry->command = NULL.
  */
 void hook_cache_clear(struct strmap *cache)
 {
@@ -267,21 +270,26 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 			struct hook_config_cache_entry *entry;
 			char *command;
 
-			/* filter out disabled hooks */
-			if (unsorted_string_list_lookup(&cb_data.disabled_hooks,
-							hname))
-				continue;
+			int is_disabled =
+				!!unsorted_string_list_lookup(
+					&cb_data.disabled_hooks, hname);
 
 			command = strmap_get(&cb_data.commands, hname);
-			if (!command)
-				die(_("'hook.%s.command' must be configured or "
-				      "'hook.%s.event' must be removed;"
-				      " aborting."), hname, hname);
+			if (!command) {
+				if (is_disabled)
+					warning(_("disabled hook '%s' has no "
+						  "command configured"), hname);
+				else
+					die(_("'hook.%s.command' must be configured or "
+					      "'hook.%s.event' must be removed;"
+					      " aborting."), hname, hname);
+			}
 
 			/* util stores a cache entry; owned by the cache. */
 			CALLOC_ARRAY(entry, 1);
-			entry->command = xstrdup(command);
+			entry->command = xstrdup_or_null(command);
 			entry->scope = scope;
+			entry->disabled = is_disabled;
 			string_list_append(hooks, hname)->util = entry;
 		}
 
@@ -361,8 +369,10 @@ static void list_hooks_add_configured(struct repository *r,
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
-		hook->u.configured.command = xstrdup(entry->command);
+		hook->u.configured.command =
+			entry->command ? xstrdup(entry->command) : NULL;
 		hook->u.configured.scope = entry->scope;
+		hook->u.configured.disabled = entry->disabled;
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
@@ -400,7 +410,16 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 int hook_exists(struct repository *r, const char *name)
 {
 	struct string_list *hooks = list_hooks(r, name, NULL);
-	int exists = hooks->nr > 0;
+	int exists = 0;
+
+	for (size_t i = 0; i < hooks->nr; i++) {
+		struct hook *h = hooks->items[i].util;
+		if (h->kind == HOOK_TRADITIONAL ||
+		    !h->u.configured.disabled) {
+			exists = 1;
+			break;
+		}
+	}
 	string_list_clear_func(hooks, hook_free);
 	free(hooks);
 	return exists;
@@ -415,10 +434,11 @@ static int pick_next_hook(struct child_process *cp,
 	struct string_list *hook_list = hook_cb->hook_command_list;
 	struct hook *h;
 
-	if (hook_cb->hook_to_run_index >= hook_list->nr)
-		return 0;
-
-	h = hook_list->items[hook_cb->hook_to_run_index++].util;
+	do {
+		if (hook_cb->hook_to_run_index >= hook_list->nr)
+			return 0;
+		h = hook_list->items[hook_cb->hook_to_run_index++].util;
+	} while (h->kind == HOOK_CONFIGURED && h->u.configured.disabled);
 
 	cp->no_stdin = 1;
 	strvec_pushv(&cp->env, hook_cb->options->env.v);
diff --git a/hook.h b/hook.h
index 92e9faf9bb..7c8c3d471e 100644
--- a/hook.h
+++ b/hook.h
@@ -31,6 +31,7 @@ struct hook {
 			const char *friendly_name;
 			const char *command;
 			enum config_scope scope;
+			unsigned int disabled:1;
 		} configured;
 	} u;
 
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 22cca15fda..6f6fe88bea 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -357,7 +357,15 @@ test_expect_success 'disabled hook is not run' '
 	test_must_be_empty actual
 '
 
-test_expect_success 'disabled hook does not appear in git hook list' '
+test_expect_success 'disabled hook with no command warns' '
+	test_config hook.nocommand.event "pre-commit" &&
+	test_config hook.nocommand.enabled false &&
+
+	git hook list pre-commit 2>actual &&
+	test_grep "disabled hook.*nocommand.*no command configured" actual
+'
+
+test_expect_success 'disabled hook appears as disabled in git hook list' '
 	test_config hook.active.event "pre-commit" &&
 	test_config hook.active.command "echo active" &&
 	test_config hook.inactive.event "pre-commit" &&
@@ -365,8 +373,27 @@ test_expect_success 'disabled hook does not appear in git hook list' '
 	test_config hook.inactive.enabled false &&
 
 	git hook list pre-commit >actual &&
-	test_grep "active" actual &&
-	test_grep ! "inactive" actual
+	test_grep "^active$" actual &&
+	test_grep "^disabled	inactive$" actual
+'
+
+test_expect_success 'disabled hook shows scope with --show-scope' '
+	test_config hook.myhook.event "pre-commit" &&
+	test_config hook.myhook.command "echo hi" &&
+	test_config hook.myhook.enabled false &&
+
+	git hook list --show-scope pre-commit >actual &&
+	test_grep "^local	disabled	myhook$" actual
+'
+
+test_expect_success 'disabled configured hook is not reported as existing by hook_exists' '
+	test_when_finished "rm -f git-bugreport-hook-exists-test.txt" &&
+	test_config hook.linter.event "pre-commit" &&
+	test_config hook.linter.command "echo lint" &&
+	test_config hook.linter.enabled false &&
+
+	git bugreport -s hook-exists-test &&
+	test_grep ! "pre-commit" git-bugreport-hook-exists-test.txt
 '
 
 test_expect_success 'globally disabled hook can be re-enabled locally' '
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH v2 10/10] hook: show disabled hooks in "git hook list"

[Patrick Steinhardt]

On Fri, Mar 20, 2026 at 01:52:11PM +0200, Adrian Ratiu wrote:
> diff --git a/hook.c b/hook.c
> index aa08c38c27..0e09b9a2bb 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -119,6 +119,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
>  struct hook_config_cache_entry {
>  	char *command;
>  	enum config_scope scope;
> +	unsigned int disabled:1;
>  };
>  
>  /*

Curious, this is now a single-bit int. I still would have expected a
proper bool here :)

> diff --git a/hook.h b/hook.h
> index 92e9faf9bb..7c8c3d471e 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -31,6 +31,7 @@ struct hook {
>  			const char *friendly_name;
>  			const char *command;
>  			enum config_scope scope;
> +			unsigned int disabled:1;
>  		} configured;
>  	} u;

Same here, I would expect a proper bool.

Patrick

Re: [PATCH v2 10/10] hook: show disabled hooks in "git hook list"

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Mar 20, 2026 at 01:52:11PM +0200, Adrian Ratiu wrote:
>> diff --git a/hook.c b/hook.c
>> index aa08c38c27..0e09b9a2bb 100644
>> --- a/hook.c
>> +++ b/hook.c
>> @@ -119,6 +119,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
>>  struct hook_config_cache_entry {
>>  	char *command;
>>  	enum config_scope scope;
>> +	unsigned int disabled:1;
>>  };
>>  
>>  /*
>
> Curious, this is now a single-bit int. I still would have expected a
> proper bool here :)
>
>> diff --git a/hook.h b/hook.h
>> index 92e9faf9bb..7c8c3d471e 100644
>> --- a/hook.h
>> +++ b/hook.h
>> @@ -31,6 +31,7 @@ struct hook {
>>  			const char *friendly_name;
>>  			const char *command;
>>  			enum config_scope scope;
>> +			unsigned int disabled:1;
>>  		} configured;
>>  	} u;
>
> Same here, I would expect a proper bool.
>
> Patrick

I do not know what an improper bool is, though ;-).

Re: [PATCH v2 10/10] hook: show disabled hooks in "git hook list"

[Adrian Ratiu]

On Tue, 24 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Fri, Mar 20, 2026 at 01:52:11PM +0200, Adrian Ratiu wrote:
>> diff --git a/hook.c b/hook.c
>> index aa08c38c27..0e09b9a2bb 100644
>> --- a/hook.c
>> +++ b/hook.c
>> @@ -119,6 +119,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
>>  struct hook_config_cache_entry {
>>  	char *command;
>>  	enum config_scope scope;
>> +	unsigned int disabled:1;
>>  };
>>  
>>  /*
>
> Curious, this is now a single-bit int. I still would have expected a
> proper bool here :)

Yes, I did it this way because the parallel series also adds another
1-bit int after this one. :) 

I'll make both these a proper bool as you requested in the other series
as well. I don't have any preference and am ok with bool as well. :)

Re: [PATCH v2 00/10] config-hook cleanups and two small 'git hook list' features

[Junio C Hamano]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> Hello everyone,
>
> v2 addresses all the feedback received in v1, many thanks to everyone
> who contributed.
>
> This series is just minor cleanups / refactorings + two minor feature additions
> to `git hook list`, which resulted from the previous series review discussions:
>
> 1. The ability to show the config scope (--show-scope).
> 2. The ability to show which hooks are disabled.
>
> This is now based on the master branch.
>
> I have pushed the branch to Github [1] and provided a clean CI run [2] with
> the exception of a known breakage for some MacOS builders (REG_ENHANCED).
>
> Thanks again,
> Adrian
>
> 1: https://github.com/10ne1/git/tree/dev/aratiu/config-cleanups-v2
> 2: https://github.com/10ne1/git/actions/runs/23340298770
>
> Changes in v2:
> * Cleanly rebased on master, no conflicts (Adrian)
> * Fix first patch build break by updating call-sites in same commit (Szeder)
> * Drop UNUSED from function declaration in the header file (Eric)
> * Drop the new struct hook_config_cache because it's redundant (Junio)
> * git hook list now prints in tab separated output format similar
>   to git config --show-scope to improve machine parseability (Junio, Patrick)
> * Fix small style issues, comments, type, commit messages. (Eric, Patrick)

These came just before the weekend for many people, so let's hold to
see if we hear further comments for a few days and then merge it
down to 'next'.

Thanks, all.

Re: [PATCH v2 00/10] config-hook cleanups and two small 'git hook list' features

[Patrick Steinhardt]

On Mon, Mar 23, 2026 at 09:11:13AM -0700, Junio C Hamano wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
> 
> > Hello everyone,
> >
> > v2 addresses all the feedback received in v1, many thanks to everyone
> > who contributed.
> >
> > This series is just minor cleanups / refactorings + two minor feature additions
> > to `git hook list`, which resulted from the previous series review discussions:
> >
> > 1. The ability to show the config scope (--show-scope).
> > 2. The ability to show which hooks are disabled.
> >
> > This is now based on the master branch.
> >
> > I have pushed the branch to Github [1] and provided a clean CI run [2] with
> > the exception of a known breakage for some MacOS builders (REG_ENHANCED).
> >
> > Thanks again,
> > Adrian
> >
> > 1: https://github.com/10ne1/git/tree/dev/aratiu/config-cleanups-v2
> > 2: https://github.com/10ne1/git/actions/runs/23340298770
> >
> > Changes in v2:
> > * Cleanly rebased on master, no conflicts (Adrian)
> > * Fix first patch build break by updating call-sites in same commit (Szeder)
> > * Drop UNUSED from function declaration in the header file (Eric)
> > * Drop the new struct hook_config_cache because it's redundant (Junio)
> > * git hook list now prints in tab separated output format similar
> >   to git config --show-scope to improve machine parseability (Junio, Patrick)
> > * Fix small style issues, comments, type, commit messages. (Eric, Patrick)
> 
> These came just before the weekend for many people, so let's hold to
> see if we hear further comments for a few days and then merge it
> down to 'next'.

Sorry, I've been a bit behind on the mailing list recently. I've got a
few further comments that might warrant a v3, but I think we're overall
close.

Thanks!

Patrick

Re: [PATCH v2 00/10] config-hook cleanups and two small 'git hook list' features

[Adrian Ratiu]

On Tue, 24 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Mon, Mar 23, 2026 at 09:11:13AM -0700, Junio C Hamano wrote:
>> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
>> 
>> > Hello everyone,
>> >
>> > v2 addresses all the feedback received in v1, many thanks to everyone
>> > who contributed.
>> >
>> > This series is just minor cleanups / refactorings + two minor feature additions
>> > to `git hook list`, which resulted from the previous series review discussions:
>> >
>> > 1. The ability to show the config scope (--show-scope).
>> > 2. The ability to show which hooks are disabled.
>> >
>> > This is now based on the master branch.
>> >
>> > I have pushed the branch to Github [1] and provided a clean CI run [2] with
>> > the exception of a known breakage for some MacOS builders (REG_ENHANCED).
>> >
>> > Thanks again,
>> > Adrian
>> >
>> > 1: https://github.com/10ne1/git/tree/dev/aratiu/config-cleanups-v2
>> > 2: https://github.com/10ne1/git/actions/runs/23340298770
>> >
>> > Changes in v2:
>> > * Cleanly rebased on master, no conflicts (Adrian)
>> > * Fix first patch build break by updating call-sites in same commit (Szeder)
>> > * Drop UNUSED from function declaration in the header file (Eric)
>> > * Drop the new struct hook_config_cache because it's redundant (Junio)
>> > * git hook list now prints in tab separated output format similar
>> >   to git config --show-scope to improve machine parseability (Junio, Patrick)
>> > * Fix small style issues, comments, type, commit messages. (Eric, Patrick)
>> 
>> These came just before the weekend for many people, so let's hold to
>> see if we hear further comments for a few days and then merge it
>> down to 'next'.
>
> Sorry, I've been a bit behind on the mailing list recently. I've got a
> few further comments that might warrant a v3, but I think we're overall
> close.

No worries, I appreciate all your reviewes and feedback!

I'll give it 1-2 more days in case other people have more feedback then
send a v3 which addresses everything you pointed out.

[PATCH v3 00/12] config-hook cleanups and three small git-hook features

[Adrian Ratiu]

Hello everyone,

v3 addresses all the feedback and requests received in v2, many thanks to all
who contributed.

Let's please stop adding features since this is getting rather big again. :)
New features can be added in subsequent patches.

This series is mostly for minor cleanups, bug fixes and refactorings + three
minor feature additions to git-hook, which resulted from review discussions:

1. The ability to show the config scope (--show-scope).
2. The ability to show which hooks are disabled.
3. The ability reject unknown hook names with "--allow-unknown-hook-name" as
   an escape hatch.

The series is based on the master branch.

Branch pushed GitHub: [1]
Successful CI run: [2]

Thanks again,
Adrian

1: https://github.com/10ne1/git/tree/dev/aratiu/config-cleanups-v3
2: https://github.com/10ne1/git/actions/runs/23540818495

Changes in v3:
* New commit: properly initialize strbuf in receive-pack.c (Patrick)
* New commit: add a check which prevents unknown hooks with git-hook(1) (Patrick)
* Removed duplicated function doc comment between .h and .c files (Patrick)
* Extended `git hook list` test to also include a hook from the hookdir (Patrick)
* Converted unsigned int disabled:1 to proper bool (Patrick)
* Minor commit rewording, header sorting, blank line fixes (Patrick)

Range-diff v2 -> v3:
 1:  aeefa72f33 =  1:  db8b7b0552 hook: move unsorted_string_list_remove() to string-list.[ch]
 -:  ---------- >  2:  02854ecc8b builtin/receive-pack: properly init receive_hook strbuf
 2:  90e821bdfa !  3:  14dcedcd9b hook: fix minor style issues
    @@ Metadata
      ## Commit message ##
         hook: fix minor style issues
     
    -    Fix some minor style nits pointed by Patrick, Junio and Eric:
    +    Fix some minor style nits pointed out by Patrick, Junio and Eric:
           * Use CALLOC_ARRAY instead of xcalloc.
           * Init struct members during declaration.
           * Simplify if condition boolean logic.
    @@ Commit message
           * Capitalization and full-stop in error/warn messages.
           * Curly brace on separate line when defining struct.
           * Comment spelling: free'd -> freed.
    +      * Sort the included headers.
    +      * Blank line fixes to improve readability.
     
         These contain no logic changes, the code behaves the same as before.
     
    @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix,
      	}
     
      ## builtin/receive-pack.c ##
    +@@
    + 
    + #include "builtin.h"
    + #include "abspath.h"
    +-
    ++#include "commit.h"
    ++#include "commit-reach.h"
    + #include "config.h"
    ++#include "connect.h"
    ++#include "connected.h"
    + #include "environment.h"
    ++#include "exec-cmd.h"
    ++#include "fsck.h"
    + #include "gettext.h"
    ++#include "gpg-interface.h"
    + #include "hex.h"
    +-#include "lockfile.h"
    +-#include "pack.h"
    +-#include "refs.h"
    +-#include "pkt-line.h"
    +-#include "sideband.h"
    +-#include "run-command.h"
    + #include "hook.h"
    +-#include "exec-cmd.h"
    +-#include "commit.h"
    ++#include "lockfile.h"
    + #include "object.h"
    +-#include "remote.h"
    +-#include "connect.h"
    +-#include "string-list.h"
    +-#include "oid-array.h"
    +-#include "connected.h"
    +-#include "strvec.h"
    +-#include "version.h"
    +-#include "gpg-interface.h"
    +-#include "sigchain.h"
    +-#include "fsck.h"
    +-#include "tmp-objdir.h"
    +-#include "oidset.h"
    +-#include "packfile.h"
    + #include "object-file.h"
    + #include "object-name.h"
    + #include "odb.h"
    ++#include "oid-array.h"
    ++#include "oidset.h"
    ++#include "pack.h"
    ++#include "packfile.h"
    ++#include "parse-options.h"
    ++#include "pkt-line.h"
    + #include "protocol.h"
    +-#include "commit-reach.h"
    ++#include "refs.h"
    ++#include "remote.h"
    ++#include "run-command.h"
    + #include "server-info.h"
    ++#include "setup.h"
    ++#include "shallow.h"
    ++#include "sideband.h"
    ++#include "sigchain.h"
    ++#include "string-list.h"
    ++#include "strvec.h"
    ++#include "tmp-objdir.h"
    + #include "trace.h"
    + #include "trace2.h"
    ++#include "version.h"
    + #include "worktree.h"
    +-#include "shallow.h"
    +-#include "setup.h"
    +-#include "parse-options.h"
    + 
    + static const char * const receive_pack_usage[] = {
    + 	N_("git receive-pack <git-dir>"),
     @@ builtin/receive-pack.c: static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_
      static void *receive_hook_feed_state_alloc(void *feed_pipe_ctx)
      {
      	struct receive_hook_feed_state *init_state = feed_pipe_ctx;
     -	struct receive_hook_feed_state *data = xcalloc(1, sizeof(*data));
     +	struct receive_hook_feed_state *data;
    ++
     +	CALLOC_ARRAY(data, 1);
      	data->report = init_state->report;
      	data->cmd = init_state->cmd;
      	data->skip_broken = init_state->skip_broken;
    + 	strbuf_init(&data->buf, 0);
    ++
    + 	return data;
    + }
    + 
     @@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands,
      {
      	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
    @@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands,
      	/* set up stdin callback */
     -	feed_init_state.cmd = commands;
     -	feed_init_state.skip_broken = skip_broken;
    +-	strbuf_init(&feed_init_state.buf, 0);
      	opt.feed_pipe_ctx = &feed_init_state;
      	opt.feed_pipe = feed_receive_hook_cb;
      	opt.feed_pipe_cb_data_alloc = receive_hook_feed_state_alloc;
     
      ## hook.c ##
    +@@
    + #include "git-compat-util.h"
    + #include "abspath.h"
    + #include "advice.h"
    ++#include "config.h"
    ++#include "environment.h"
    + #include "gettext.h"
    + #include "hook.h"
    +-#include "path.h"
    + #include "parse.h"
    ++#include "path.h"
    + #include "run-command.h"
    +-#include "config.h"
    ++#include "setup.h"
    + #include "strbuf.h"
    + #include "strmap.h"
    +-#include "environment.h"
    +-#include "setup.h"
    + 
    + const char *find_hook(struct repository *r, const char *name)
    + {
     @@ hook.c: static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
      	if (!h)
      		return;
    @@ hook.c: static void build_hook_config_map(struct repository *r, struct strmap *c
      		struct string_list *hook_names = e->value;
     -		struct string_list *hooks = xcalloc(1, sizeof(*hooks));
     +		struct string_list *hooks;
    -+		CALLOC_ARRAY(hooks, 1);
      
    ++		CALLOC_ARRAY(hooks, 1);
      		string_list_init_dup(hooks);
      
    + 		for (size_t i = 0; i < hook_names->nr; i++) {
     @@ hook.c: static struct strmap *get_hook_config_cache(struct repository *r)
      		 * it just once on the first call.
      		 */
    @@ hook.c: static void list_hooks_add_configured(struct repository *r,
      		const char *command = configured_hooks->items[i].util;
     -		struct hook *hook = xcalloc(1, sizeof(struct hook));
     +		struct hook *hook;
    ++
     +		CALLOC_ARRAY(hook, 1);
      
      		if (options && options->feed_pipe_cb_data_alloc)
    @@ hook.c: int run_hooks_opt(struct repository *r, const char *hook_name,
      	if (options->invoked_hook)
     
      ## hook.h ##
    +@@
    + #ifndef HOOK_H
    + #define HOOK_H
    +-#include "strvec.h"
    + #include "run-command.h"
    + #include "string-list.h"
    + #include "strmap.h"
    ++#include "strvec.h"
    + 
    + struct repository;
    + 
     @@ hook.h: struct hook {
      typedef void (*cb_data_free_fn)(void *data);
      typedef void *(*cb_data_alloc_fn)(void *init_ctx);
 3:  dee1dd49a4 =  4:  bd49f58486 hook: rename cb_data_free/alloc -> hook_data_free/alloc
 4:  86f61204b3 =  5:  9502a98b09 hook: detect & emit two more bugs
 5:  30542de351 !  6:  b623e682c7 hook: replace hook_list_clear() -> string_list_clear_func()
    @@ Commit message
         API becomes cleaner, e.g. no more calls with NULL function args
         like hook_list_clear(hooks, NULL).
     
    +    In other words, the callers don't need to keep track of hook
    +    internal state to determine when cleanup is necessary or not
    +    (pass NULL) because each `struct hook` now owns its data_free
    +    callback.
    +
         Suggested-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
     
    @@ hook.c: const char *find_hook(struct repository *r, const char *name)
      }
      
     -static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
    -+/*
    -+ * Frees a struct hook stored as the util pointer of a string_list_item.
    -+ * Suitable for use as a string_list_clear_func_t callback.
    -+ */
     +void hook_free(void *p, const char *str UNUSED)
      {
     +	struct hook *h = p;
    @@ hook.c: static void list_hooks_add_default(struct repository *r, const char *hoo
      	h->kind = HOOK_TRADITIONAL;
      	h->u.traditional.path = xstrdup(hook_path);
     @@ hook.c: static void list_hooks_add_configured(struct repository *r,
    - 		struct hook *hook;
    + 
      		CALLOC_ARRAY(hook, 1);
      
     -		if (options && options->feed_pipe_cb_data_alloc)
 6:  4e1374b84e =  7:  315b62b24e hook: make consistent use of friendly-name in docs
 7:  075f8202aa =  8:  830cea298b t1800: add test to verify hook execution ordering
 8:  8f948bbbe7 !  9:  4f4a720cb8 hook: introduce hook_config_cache_entry for per-hook data
    @@ hook.c: static void list_hooks_add_configured(struct repository *r,
     -		const char *command = configured_hooks->items[i].util;
     +		struct hook_config_cache_entry *entry = configured_hooks->items[i].util;
      		struct hook *hook;
    - 		CALLOC_ARRAY(hook, 1);
      
    + 		CALLOC_ARRAY(hook, 1);
     @@ hook.c: static void list_hooks_add_configured(struct repository *r,
      
      		hook->kind = HOOK_CONFIGURED;
 9:  dbf81604ed ! 10:  164e3df981 hook: show config scope in git hook list
    @@ hook.h
      #ifndef HOOK_H
      #define HOOK_H
     +#include "config.h"
    - #include "strvec.h"
      #include "run-command.h"
      #include "string-list.h"
    + #include "strmap.h"
     @@ hook.h: struct hook {
      		struct {
      			const char *friendly_name;
    @@ t/t1800-hook.sh: test_expect_success 'configured hooks run before hookdir hook'
      '
      
     +test_expect_success 'git hook list --show-scope shows config scope' '
    ++	setup_hookdir &&
     +	test_config_global hook.global-hook.command "echo global" &&
    -+	test_config_global hook.global-hook.event test-hook --add &&
    ++	test_config_global hook.global-hook.event pre-commit --add &&
     +	test_config hook.local-hook.command "echo local" &&
    -+	test_config hook.local-hook.event test-hook --add &&
    ++	test_config hook.local-hook.event pre-commit --add &&
     +
     +	cat >expected <<-\EOF &&
     +	global	global-hook
     +	local	local-hook
    ++	hook from hookdir
     +	EOF
    -+	git hook list --show-scope test-hook >actual &&
    ++	git hook list --show-scope pre-commit >actual &&
     +	test_cmp expected actual &&
     +
     +	# without --show-scope the scope must not appear
    -+	git hook list test-hook >actual &&
    ++	git hook list pre-commit >actual &&
     +	test_grep ! "^global	" actual &&
     +	test_grep ! "^local	" actual
     +'
10:  2a67244b20 ! 11:  ab9cd3ec68 hook: show disabled hooks in "git hook list"
    @@ hook.c: static void list_hooks_add_default(struct repository *r, const char *hoo
      struct hook_config_cache_entry {
      	char *command;
      	enum config_scope scope;
    -+	unsigned int disabled:1;
    ++	bool disabled;
      };
      
      /*
    @@ hook.c: static void build_hook_config_map(struct repository *r, struct strmap *c
     -			if (unsorted_string_list_lookup(&cb_data.disabled_hooks,
     -							hname))
     -				continue;
    -+			int is_disabled =
    ++			bool is_disabled =
     +				!!unsorted_string_list_lookup(
     +					&cb_data.disabled_hooks, hname);
      
    @@ hook.h: struct hook {
      			const char *friendly_name;
      			const char *command;
      			enum config_scope scope;
    -+			unsigned int disabled:1;
    ++			bool disabled;
      		} configured;
      	} u;
      
 -:  ---------- > 12:  dce86488bc hook: reject unknown hook names in git-hook(1)

Adrian Ratiu (12):
  hook: move unsorted_string_list_remove() to string-list.[ch]
  builtin/receive-pack: properly init receive_hook strbuf
  hook: fix minor style issues
  hook: rename cb_data_free/alloc -> hook_data_free/alloc
  hook: detect & emit two more bugs
  hook: replace hook_list_clear() -> string_list_clear_func()
  hook: make consistent use of friendly-name in docs
  t1800: add test to verify hook execution ordering
  hook: introduce hook_config_cache_entry for per-hook data
  hook: show config scope in git hook list
  hook: show disabled hooks in "git hook list"
  hook: reject unknown hook names in git-hook(1)

 Documentation/config/hook.adoc |  30 +++---
 Documentation/git-hook.adoc    |  27 +++--
 Makefile                       |   1 +
 builtin/hook.c                 |  61 +++++++++--
 builtin/receive-pack.c         |  64 +++++------
 hook.c                         | 192 +++++++++++++++++++++------------
 hook.h                         |  34 +++---
 refs.c                         |   3 +-
 string-list.c                  |   9 ++
 string-list.h                  |   8 ++
 t/t1800-hook.sh                | 167 ++++++++++++++++++++++------
 transport.c                    |   3 +-
 12 files changed, 424 insertions(+), 175 deletions(-)

-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 01/12] hook: move unsorted_string_list_remove() to string-list.[ch]

[Adrian Ratiu]

Move the convenience wrapper from hook to string-list since
it's a more suitable place. Add a doc comment to the header.

Also add a free_util arg to make the function more generic
and make the API similar to other functions in string-list.h.
Update the existing call-sites.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c        | 14 +++-----------
 string-list.c |  9 +++++++++
 string-list.h |  8 ++++++++
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/hook.c b/hook.c
index 2c8252b2c4..67cc9a66df 100644
--- a/hook.c
+++ b/hook.c
@@ -110,14 +110,6 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	string_list_append(hook_list, hook_path)->util = h;
 }
 
-static void unsorted_string_list_remove(struct string_list *list,
-					const char *str)
-{
-	struct string_list_item *item = unsorted_string_list_lookup(list, str);
-	if (item)
-		unsorted_string_list_delete_item(list, item - list->items, 0);
-}
-
 /*
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
@@ -156,7 +148,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			struct strmap_entry *e;
 
 			strmap_for_each_entry(&data->event_hooks, &iter, e)
-				unsorted_string_list_remove(e->value, hook_name);
+				unsorted_string_list_remove(e->value, hook_name, 0);
 		} else {
 			struct string_list *hooks =
 				strmap_get(&data->event_hooks, value);
@@ -168,7 +160,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			}
 
 			/* Re-insert if necessary to preserve last-seen order. */
-			unsorted_string_list_remove(hooks, hook_name);
+			unsorted_string_list_remove(hooks, hook_name, 0);
 			string_list_append(hooks, hook_name);
 		}
 	} else if (!strcmp(subkey, "command")) {
@@ -186,7 +178,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 			break;
 		case 1: /* enabled: undo a prior disabled entry */
 			unsorted_string_list_remove(&data->disabled_hooks,
-						    hook_name);
+						    hook_name, 0);
 			break;
 		default:
 			break; /* ignore unrecognised values */
diff --git a/string-list.c b/string-list.c
index fffa2ad4b6..d260b873c8 100644
--- a/string-list.c
+++ b/string-list.c
@@ -281,6 +281,15 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_
 	list->nr--;
 }
 
+void unsorted_string_list_remove(struct string_list *list, const char *str,
+				 int free_util)
+{
+	struct string_list_item *item = unsorted_string_list_lookup(list, str);
+	if (item)
+		unsorted_string_list_delete_item(list, item - list->items,
+						 free_util);
+}
+
 /*
  * append a substring [p..end] to list; return number of things it
  * appended to the list.
diff --git a/string-list.h b/string-list.h
index 3ad862a187..b86ee7c099 100644
--- a/string-list.h
+++ b/string-list.h
@@ -265,6 +265,14 @@ struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
  */
 void unsorted_string_list_delete_item(struct string_list *list, int i, int free_util);
 
+/**
+ * Remove the first item matching `str` from an unsorted string_list.
+ * No-op if `str` is not found. If `free_util` is non-zero, the `util`
+ * pointer of the removed item is freed before deletion.
+ */
+void unsorted_string_list_remove(struct string_list *list, const char *str,
+				 int free_util);
+
 /**
  * Split string into substrings on characters in `delim` and append the
  * substrings to `list`.  The input string is not modified.
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 02/12] builtin/receive-pack: properly init receive_hook strbuf

[Adrian Ratiu]

The run_receive_hook() stack-allocated `struct receive_hook_feed_state`
is a template with initial values for child states allocated on the heap
for each hook process, by calling receive_hook_feed_state_alloc() when
spinning up each hook child.

All these values are already initialized to zero, however I forgot to
properly initialize the strbuf, which I left NULL.

This is more of a code cleanup because in practice it has no effect,
the states used by the children are always initialized, however it's
good to fix in case someone ends up accidentally dereferencing the NULL
pointer in the future.

Reported-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/receive-pack.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e34edff406..a1ffe4570f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -963,6 +963,7 @@ static int run_receive_hook(struct command *commands,
 	/* set up stdin callback */
 	feed_init_state.cmd = commands;
 	feed_init_state.skip_broken = skip_broken;
+	strbuf_init(&feed_init_state.buf, 0);
 	opt.feed_pipe_ctx = &feed_init_state;
 	opt.feed_pipe = feed_receive_hook_cb;
 	opt.feed_pipe_cb_data_alloc = receive_hook_feed_state_alloc;
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 03/12] hook: fix minor style issues

[Adrian Ratiu]

Fix some minor style nits pointed out by Patrick, Junio and Eric:
  * Use CALLOC_ARRAY instead of xcalloc.
  * Init struct members during declaration.
  * Simplify if condition boolean logic.
  * Missing curly braces in if/else stmts.
  * Unnecessary header includes.
  * Capitalization and full-stop in error/warn messages.
  * Curly brace on separate line when defining struct.
  * Comment spelling: free'd -> freed.
  * Sort the included headers.
  * Blank line fixes to improve readability.

These contain no logic changes, the code behaves the same as before.

Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c         |  6 ++--
 builtin/receive-pack.c | 65 ++++++++++++++++++++++--------------------
 hook.c                 | 34 +++++++++++-----------
 hook.h                 |  5 ++--
 refs.c                 |  3 +-
 t/t1800-hook.sh        |  2 +-
 transport.c            |  3 +-
 7 files changed, 61 insertions(+), 57 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index 83020dfb4f..e641614b84 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -5,8 +5,6 @@
 #include "gettext.h"
 #include "hook.h"
 #include "parse-options.h"
-#include "strvec.h"
-#include "abspath.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
 	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
@@ -51,7 +49,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	 * 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."),
+		usage_msg_opt(_("you must specify a hook event name to list"),
 			      builtin_hook_list_usage, list_options);
 
 	hookname = argv[0];
@@ -59,7 +57,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	head = list_hooks(repo, hookname, NULL);
 
 	if (!head->nr) {
-		warning(_("No hooks found for event '%s'"), hookname);
+		warning(_("no hooks found for event '%s'"), hookname);
 		ret = 1; /* no hooks found */
 		goto cleanup;
 	}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a1ffe4570f..cb3656a034 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -3,46 +3,45 @@
 
 #include "builtin.h"
 #include "abspath.h"
-
+#include "commit.h"
+#include "commit-reach.h"
 #include "config.h"
+#include "connect.h"
+#include "connected.h"
 #include "environment.h"
+#include "exec-cmd.h"
+#include "fsck.h"
 #include "gettext.h"
+#include "gpg-interface.h"
 #include "hex.h"
-#include "lockfile.h"
-#include "pack.h"
-#include "refs.h"
-#include "pkt-line.h"
-#include "sideband.h"
-#include "run-command.h"
 #include "hook.h"
-#include "exec-cmd.h"
-#include "commit.h"
+#include "lockfile.h"
 #include "object.h"
-#include "remote.h"
-#include "connect.h"
-#include "string-list.h"
-#include "oid-array.h"
-#include "connected.h"
-#include "strvec.h"
-#include "version.h"
-#include "gpg-interface.h"
-#include "sigchain.h"
-#include "fsck.h"
-#include "tmp-objdir.h"
-#include "oidset.h"
-#include "packfile.h"
 #include "object-file.h"
 #include "object-name.h"
 #include "odb.h"
+#include "oid-array.h"
+#include "oidset.h"
+#include "pack.h"
+#include "packfile.h"
+#include "parse-options.h"
+#include "pkt-line.h"
 #include "protocol.h"
-#include "commit-reach.h"
+#include "refs.h"
+#include "remote.h"
+#include "run-command.h"
 #include "server-info.h"
+#include "setup.h"
+#include "shallow.h"
+#include "sideband.h"
+#include "sigchain.h"
+#include "string-list.h"
+#include "strvec.h"
+#include "tmp-objdir.h"
 #include "trace.h"
 #include "trace2.h"
+#include "version.h"
 #include "worktree.h"
-#include "shallow.h"
-#include "setup.h"
-#include "parse-options.h"
 
 static const char * const receive_pack_usage[] = {
 	N_("git receive-pack <git-dir>"),
@@ -904,11 +903,14 @@ static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_
 static void *receive_hook_feed_state_alloc(void *feed_pipe_ctx)
 {
 	struct receive_hook_feed_state *init_state = feed_pipe_ctx;
-	struct receive_hook_feed_state *data = xcalloc(1, sizeof(*data));
+	struct receive_hook_feed_state *data;
+
+	CALLOC_ARRAY(data, 1);
 	data->report = init_state->report;
 	data->cmd = init_state->cmd;
 	data->skip_broken = init_state->skip_broken;
 	strbuf_init(&data->buf, 0);
+
 	return data;
 }
 
@@ -928,7 +930,11 @@ static int run_receive_hook(struct command *commands,
 {
 	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
 	struct command *iter = commands;
-	struct receive_hook_feed_state feed_init_state = { 0 };
+	struct receive_hook_feed_state feed_init_state = {
+		.cmd = commands,
+		.skip_broken = skip_broken,
+		.buf = STRBUF_INIT,
+	};
 	struct async sideband_async;
 	int sideband_async_started = 0;
 	int saved_stderr = -1;
@@ -961,9 +967,6 @@ static int run_receive_hook(struct command *commands,
 	prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started);
 
 	/* set up stdin callback */
-	feed_init_state.cmd = commands;
-	feed_init_state.skip_broken = skip_broken;
-	strbuf_init(&feed_init_state.buf, 0);
 	opt.feed_pipe_ctx = &feed_init_state;
 	opt.feed_pipe = feed_receive_hook_cb;
 	opt.feed_pipe_cb_data_alloc = receive_hook_feed_state_alloc;
diff --git a/hook.c b/hook.c
index 67cc9a66df..935237fc1d 100644
--- a/hook.c
+++ b/hook.c
@@ -1,16 +1,16 @@
 #include "git-compat-util.h"
 #include "abspath.h"
 #include "advice.h"
+#include "config.h"
+#include "environment.h"
 #include "gettext.h"
 #include "hook.h"
-#include "path.h"
 #include "parse.h"
+#include "path.h"
 #include "run-command.h"
-#include "config.h"
+#include "setup.h"
 #include "strbuf.h"
 #include "strmap.h"
-#include "environment.h"
-#include "setup.h"
 
 const char *find_hook(struct repository *r, const char *name)
 {
@@ -57,9 +57,9 @@ static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
 	if (!h)
 		return;
 
-	if (h->kind == HOOK_TRADITIONAL)
+	if (h->kind == HOOK_TRADITIONAL) {
 		free((void *)h->u.traditional.path);
-	else if (h->kind == HOOK_CONFIGURED) {
+	} else if (h->kind == HOOK_CONFIGURED) {
 		free((void *)h->u.configured.friendly_name);
 		free((void *)h->u.configured.command);
 	}
@@ -91,7 +91,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	if (!hook_path)
 		return;
 
-	h = xcalloc(1, sizeof(struct hook));
+	CALLOC_ARRAY(h, 1);
 
 	/*
 	 * If the hook is to run in a specific dir, a relative path can
@@ -154,7 +154,7 @@ static int hook_config_lookup_all(const char *key, const char *value,
 				strmap_get(&data->event_hooks, value);
 
 			if (!hooks) {
-				hooks = xcalloc(1, sizeof(*hooks));
+				CALLOC_ARRAY(hooks, 1);
 				string_list_init_dup(hooks);
 				strmap_put(&data->event_hooks, value, hooks);
 			}
@@ -227,8 +227,9 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 	/* Construct the cache from parsed configs. */
 	strmap_for_each_entry(&cb_data.event_hooks, &iter, e) {
 		struct string_list *hook_names = e->value;
-		struct string_list *hooks = xcalloc(1, sizeof(*hooks));
+		struct string_list *hooks;
 
+		CALLOC_ARRAY(hooks, 1);
 		string_list_init_dup(hooks);
 
 		for (size_t i = 0; i < hook_names->nr; i++) {
@@ -281,7 +282,7 @@ static struct strmap *get_hook_config_cache(struct repository *r)
 		 * it just once on the first call.
 		 */
 		if (!r->hook_config_cache) {
-			r->hook_config_cache = xcalloc(1, sizeof(*cache));
+			CALLOC_ARRAY(r->hook_config_cache, 1);
 			strmap_init(r->hook_config_cache);
 			build_hook_config_map(r, r->hook_config_cache);
 		}
@@ -289,9 +290,9 @@ static struct strmap *get_hook_config_cache(struct repository *r)
 	} else {
 		/*
 		 * Out-of-repo calls (no gitdir) allocate and return a temporary
-		 * map cache which gets free'd immediately by the caller.
+		 * cache which gets freed immediately by the caller.
 		 */
-		cache = xcalloc(1, sizeof(*cache));
+		CALLOC_ARRAY(cache, 1);
 		strmap_init(cache);
 		build_hook_config_map(r, cache);
 	}
@@ -311,7 +312,9 @@ static void list_hooks_add_configured(struct repository *r,
 	for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) {
 		const char *friendly_name = configured_hooks->items[i].string;
 		const char *command = configured_hooks->items[i].util;
-		struct hook *hook = xcalloc(1, sizeof(struct hook));
+		struct hook *hook;
+
+		CALLOC_ARRAY(hook, 1);
 
 		if (options && options->feed_pipe_cb_data_alloc)
 			hook->feed_pipe_cb_data =
@@ -343,7 +346,7 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 	if (!hookname)
 		BUG("null hookname was provided to hook_list()!");
 
-	hook_head = xmalloc(sizeof(struct string_list));
+	CALLOC_ARRAY(hook_head, 1);
 	string_list_init_dup(hook_head);
 
 	/* Add hooks from the config, e.g. hook.myhook.event = pre-commit */
@@ -493,8 +496,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
 	 * Ensure cb_data copy and free functions are either provided together,
 	 * or neither one is provided.
 	 */
-	if ((options->feed_pipe_cb_data_alloc && !options->feed_pipe_cb_data_free) ||
-	    (!options->feed_pipe_cb_data_alloc && options->feed_pipe_cb_data_free))
+	if (!options->feed_pipe_cb_data_alloc != !options->feed_pipe_cb_data_free)
 		BUG("feed_pipe_cb_data_alloc and feed_pipe_cb_data_free must be set together");
 
 	if (options->invoked_hook)
diff --git a/hook.h b/hook.h
index e949f5d488..1c447cbb6b 100644
--- a/hook.h
+++ b/hook.h
@@ -1,9 +1,9 @@
 #ifndef HOOK_H
 #define HOOK_H
-#include "strvec.h"
 #include "run-command.h"
 #include "string-list.h"
 #include "strmap.h"
+#include "strvec.h"
 
 struct repository;
 
@@ -46,8 +46,7 @@ struct hook {
 typedef void (*cb_data_free_fn)(void *data);
 typedef void *(*cb_data_alloc_fn)(void *init_ctx);
 
-struct run_hooks_opt
-{
+struct run_hooks_opt {
 	/* Environment vars to be set for each hook */
 	struct strvec env;
 
diff --git a/refs.c b/refs.c
index 6fb8f9d10c..33c7961802 100644
--- a/refs.c
+++ b/refs.c
@@ -2591,7 +2591,8 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
 
 static void *transaction_feed_cb_data_alloc(void *feed_pipe_ctx UNUSED)
 {
-	struct transaction_feed_cb_data *data = xmalloc(sizeof(*data));
+	struct transaction_feed_cb_data *data;
+	CALLOC_ARRAY(data, 1);
 	strbuf_init(&data->buf, 0);
 	data->index = 0;
 	return data;
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index b1583e9ef9..952bf97b86 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -34,7 +34,7 @@ test_expect_success 'git hook usage' '
 
 test_expect_success 'git hook list: nonexistent hook' '
 	cat >stderr.expect <<-\EOF &&
-	warning: No hooks found for event '\''test-hook'\''
+	warning: no hooks found for event '\''test-hook'\''
 	EOF
 	test_expect_code 1 git hook list test-hook 2>stderr.actual &&
 	test_cmp stderr.expect stderr.actual
diff --git a/transport.c b/transport.c
index cb1befba8c..e53936d87b 100644
--- a/transport.c
+++ b/transport.c
@@ -1360,7 +1360,8 @@ static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void
 
 static void *pre_push_hook_data_alloc(void *feed_pipe_ctx)
 {
-	struct feed_pre_push_hook_data *data = xmalloc(sizeof(*data));
+	struct feed_pre_push_hook_data *data;
+	CALLOC_ARRAY(data, 1);
 	strbuf_init(&data->buf, 0);
 	data->refs = (struct ref *)feed_pipe_ctx;
 	return data;
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 04/12] hook: rename cb_data_free/alloc -> hook_data_free/alloc

[Adrian Ratiu]

Rename the hook callback function types to use the hook prefix.

This is a style fix with no logic changes.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c |  4 ++--
 hook.h | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hook.c b/hook.c
index 935237fc1d..4a0db5cfeb 100644
--- a/hook.c
+++ b/hook.c
@@ -52,7 +52,7 @@ const char *find_hook(struct repository *r, const char *name)
 	return path.buf;
 }
 
-static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
+static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
 {
 	if (!h)
 		return;
@@ -70,7 +70,7 @@ static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free)
 	free(h);
 }
 
-void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free)
+void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free)
 {
 	struct string_list_item *item;
 
diff --git a/hook.h b/hook.h
index 1c447cbb6b..965794a5b8 100644
--- a/hook.h
+++ b/hook.h
@@ -43,8 +43,8 @@ struct hook {
 	void *feed_pipe_cb_data;
 };
 
-typedef void (*cb_data_free_fn)(void *data);
-typedef void *(*cb_data_alloc_fn)(void *init_ctx);
+typedef void (*hook_data_free_fn)(void *data);
+typedef void *(*hook_data_alloc_fn)(void *init_ctx);
 
 struct run_hooks_opt {
 	/* Environment vars to be set for each hook */
@@ -131,14 +131,14 @@ struct run_hooks_opt {
 	 *
 	 * The `feed_pipe_ctx` pointer can be used to pass initialization data.
 	 */
-	cb_data_alloc_fn feed_pipe_cb_data_alloc;
+	hook_data_alloc_fn feed_pipe_cb_data_alloc;
 
 	/**
 	 * Called to free the memory initialized by `feed_pipe_cb_data_alloc`.
 	 *
 	 * Must always be provided when `feed_pipe_cb_data_alloc` is provided.
 	 */
-	cb_data_free_fn feed_pipe_cb_data_free;
+	hook_data_free_fn feed_pipe_cb_data_free;
 };
 
 #define RUN_HOOKS_OPT_INIT { \
@@ -188,7 +188,7 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
  * Frees the memory allocated for the hook list, including the `struct hook`
  * items and their internal state.
  */
-void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free);
+void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
 
 /**
  * Frees the hook configuration cache stored in `struct repository`.
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 05/12] hook: detect & emit two more bugs

[Adrian Ratiu]

Trigger a bug when an unknown hook type is encountered while
setting up hook execution.

Also issue a bug if a configured hook is enabled without a cmd.

Mostly useful for defensive coding.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hook.c b/hook.c
index 4a0db5cfeb..b0226ed716 100644
--- a/hook.c
+++ b/hook.c
@@ -409,7 +409,11 @@ static int pick_next_hook(struct child_process *cp,
 	} else if (h->kind == HOOK_CONFIGURED) {
 		/* to enable oneliners, let config-specified hooks run in shell. */
 		cp->use_shell = true;
+		if (!h->u.configured.command)
+			BUG("non-disabled HOOK_CONFIGURED hook has no command");
 		strvec_push(&cp->args, h->u.configured.command);
+	} else {
+		BUG("unknown hook kind");
 	}
 
 	if (!cp->args.nr)
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 06/12] hook: replace hook_list_clear() -> string_list_clear_func()

[Adrian Ratiu]

Replace the custom function with string_list_clear_func() which
is a more common pattern for clearing a string_list.

To be able to do this, rework hook_clear() into hook_free(), so
it can be passed to string_list_clear_func().

A slight complication is the need to keep a copy of the internal
cb data free() pointer, however I think it's worth it since the
API becomes cleaner, e.g. no more calls with NULL function args
like hook_list_clear(hooks, NULL).

In other words, the callers don't need to keep track of hook
internal state to determine when cleanup is necessary or not
(pass NULL) because each `struct hook` now owns its data_free
callback.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c |  2 +-
 hook.c         | 40 ++++++++++++++++++++++------------------
 hook.h         | 20 ++++++++++++++------
 3 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index e641614b84..54b737990b 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -78,7 +78,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	}
 
 cleanup:
-	hook_list_clear(head, NULL);
+	string_list_clear_func(head, hook_free);
 	free(head);
 	return ret;
 }
diff --git a/hook.c b/hook.c
index b0226ed716..021110f216 100644
--- a/hook.c
+++ b/hook.c
@@ -52,8 +52,10 @@ const char *find_hook(struct repository *r, const char *name)
 	return path.buf;
 }
 
-static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
+void hook_free(void *p, const char *str UNUSED)
 {
+	struct hook *h = p;
+
 	if (!h)
 		return;
 
@@ -64,22 +66,12 @@ static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
 		free((void *)h->u.configured.command);
 	}
 
-	if (cb_data_free)
-		cb_data_free(h->feed_pipe_cb_data);
+	if (h->data_free && h->feed_pipe_cb_data)
+		h->data_free(h->feed_pipe_cb_data);
 
 	free(h);
 }
 
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free)
-{
-	struct string_list_item *item;
-
-	for_each_string_list_item(item, hooks)
-		hook_clear(item->util, cb_data_free);
-
-	string_list_clear(hooks, 0);
-}
-
 /* Helper to detect and add default "traditional" hooks from the hookdir. */
 static void list_hooks_add_default(struct repository *r, const char *hookname,
 				   struct string_list *hook_list,
@@ -100,9 +92,15 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	if (options && options->dir)
 		hook_path = absolute_path(hook_path);
 
-	/* Setup per-hook internal state cb data */
-	if (options && options->feed_pipe_cb_data_alloc)
+	/*
+	 * Setup per-hook internal state callback data.
+	 * When provided, the alloc/free callbacks are always provided
+	 * together, so use them to alloc/free the internal hook state.
+	 */
+	if (options && options->feed_pipe_cb_data_alloc) {
 		h->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx);
+		h->data_free = options->feed_pipe_cb_data_free;
+	}
 
 	h->kind = HOOK_TRADITIONAL;
 	h->u.traditional.path = xstrdup(hook_path);
@@ -316,10 +314,16 @@ static void list_hooks_add_configured(struct repository *r,
 
 		CALLOC_ARRAY(hook, 1);
 
-		if (options && options->feed_pipe_cb_data_alloc)
+		/*
+		 * When provided, the alloc/free callbacks are always provided
+		 * together, so use them to alloc/free the internal hook state.
+		 */
+		if (options && options->feed_pipe_cb_data_alloc) {
 			hook->feed_pipe_cb_data =
 				options->feed_pipe_cb_data_alloc(
 					options->feed_pipe_ctx);
+			hook->data_free = options->feed_pipe_cb_data_free;
+		}
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
@@ -362,7 +366,7 @@ int hook_exists(struct repository *r, const char *name)
 {
 	struct string_list *hooks = list_hooks(r, name, NULL);
 	int exists = hooks->nr > 0;
-	hook_list_clear(hooks, NULL);
+	string_list_clear_func(hooks, hook_free);
 	free(hooks);
 	return exists;
 }
@@ -516,7 +520,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
 	run_processes_parallel(&opts);
 	ret = cb_data.rc;
 cleanup:
-	hook_list_clear(cb_data.hook_command_list, options->feed_pipe_cb_data_free);
+	string_list_clear_func(cb_data.hook_command_list, hook_free);
 	free(cb_data.hook_command_list);
 	run_hooks_opt_clear(options);
 	return ret;
diff --git a/hook.h b/hook.h
index 965794a5b8..a56ac20ccf 100644
--- a/hook.h
+++ b/hook.h
@@ -7,6 +7,9 @@
 
 struct repository;
 
+typedef void (*hook_data_free_fn)(void *data);
+typedef void *(*hook_data_alloc_fn)(void *init_ctx);
+
 /**
  * Represents a hook command to be run.
  * Hooks can be:
@@ -41,10 +44,15 @@ struct hook {
 	 * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it.
 	 */
 	void *feed_pipe_cb_data;
-};
 
-typedef void (*hook_data_free_fn)(void *data);
-typedef void *(*hook_data_alloc_fn)(void *init_ctx);
+	/**
+	 * Callback to free `feed_pipe_cb_data`.
+	 *
+	 * It is called automatically and points to the `feed_pipe_cb_data_free`
+	 * provided via the `run_hook_opt` parameter.
+	 */
+	hook_data_free_fn data_free;
+};
 
 struct run_hooks_opt {
 	/* Environment vars to be set for each hook */
@@ -185,10 +193,10 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 			       struct run_hooks_opt *options);
 
 /**
- * Frees the memory allocated for the hook list, including the `struct hook`
- * items and their internal state.
+ * Frees a struct hook stored as the util pointer of a string_list_item.
+ * Suitable for use as a string_list_clear_func_t callback.
  */
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
+void hook_free(void *p, const char *str);
 
 /**
  * Frees the hook configuration cache stored in `struct repository`.
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 07/12] hook: make consistent use of friendly-name in docs

[Adrian Ratiu]

Both `name` and `friendly-name` is being used. Standardize on
`friendly-name` for consistency since name is rather generic,
even when used in the hooks namespace.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 Documentation/config/hook.adoc | 30 +++++++++++++++---------------
 Documentation/git-hook.adoc    |  6 +++---
 hook.c                         |  2 +-
 hook.h                         |  2 +-
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc
index 64e845a260..9e78f26439 100644
--- a/Documentation/config/hook.adoc
+++ b/Documentation/config/hook.adoc
@@ -1,23 +1,23 @@
-hook.<name>.command::
-	The command to execute for `hook.<name>`. `<name>` is a unique
-	"friendly" name that identifies this hook. (The hook events that
-	trigger the command are configured with `hook.<name>.event`.) The
-	value can be an executable path or a shell oneliner. If more than
-	one value is specified for the same `<name>`, only the last value
-	parsed is used. See linkgit:git-hook[1].
+hook.<friendly-name>.command::
+	The command to execute for `hook.<friendly-name>`. `<friendly-name>`
+	is a unique name that identifies this hook. The hook events that
+	trigger the command are configured with `hook.<friendly-name>.event`.
+	The value can be an executable path or a shell oneliner. If more than
+	one value is specified for the same `<friendly-name>`, only the last
+	value parsed is used. See linkgit:git-hook[1].
 
-hook.<name>.event::
-	The hook events that trigger `hook.<name>`. The value is the name
-	of a hook event, like "pre-commit" or "update". (See
+hook.<friendly-name>.event::
+	The hook events that trigger `hook.<friendly-name>`. The value is the
+	name of a hook event, like "pre-commit" or "update". (See
 	linkgit:githooks[5] for a complete list of hook events.) On the
-	specified event, the associated `hook.<name>.command` is executed.
-	This is a multi-valued key. To run `hook.<name>` on multiple
+	specified event, the associated `hook.<friendly-name>.command` is executed.
+	This is a multi-valued key. To run `hook.<friendly-name>` on multiple
 	events, specify the key more than once. An empty value resets
 	the list of events, clearing any previously defined events for
-	`hook.<name>`. See linkgit:git-hook[1].
+	`hook.<friendly-name>`. See linkgit:git-hook[1].
 
-hook.<name>.enabled::
-	Whether the hook `hook.<name>` is enabled. Defaults to `true`.
+hook.<friendly-name>.enabled::
+	Whether the hook `hook.<friendly-name>` is enabled. Defaults to `true`.
 	Set to `false` to disable the hook without removing its
 	configuration. This is particularly useful when a hook is defined
 	in a system or global config file and needs to be disabled for a
diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index 12d2701b52..966388660a 100644
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -44,7 +44,7 @@ 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
+`hook.<friendly-name>.event` configs during the configuration parse (see
 linkgit:git-config[1]). Although multiple `hook.linter.event` configs can be
 added, only one `hook.linter.command` event is valid - Git uses "last-one-wins"
 to determine which command to run.
@@ -76,10 +76,10 @@ 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,
+For a full list of hook events which you can set your `hook.<friendly-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
+Git will ignore any `hook.<friendly-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.
 
diff --git a/hook.c b/hook.c
index 021110f216..dc0c3de667 100644
--- a/hook.c
+++ b/hook.c
@@ -112,7 +112,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
  * event_hooks: event-name to list of friendly-names map.
- * disabled_hooks: set of friendly-names with hook.name.enabled = false.
+ * disabled_hooks: set of friendly-names with hook.<friendly-name>.enabled = false.
  */
 struct hook_all_config_cb {
 	struct strmap commands;
diff --git a/hook.h b/hook.h
index a56ac20ccf..d2cf59e649 100644
--- a/hook.h
+++ b/hook.h
@@ -14,7 +14,7 @@ typedef void *(*hook_data_alloc_fn)(void *init_ctx);
  * Represents a hook command to be run.
  * Hooks can be:
  * 1. "traditional" (found in the hooks directory)
- * 2. "configured" (defined in Git's configuration via hook.<name>.event).
+ * 2. "configured" (defined in Git's configuration via hook.<friendly-name>.event).
  * The 'kind' field determines which part of the union 'u' is valid.
  */
 struct hook {
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 08/12] t1800: add test to verify hook execution ordering

[Adrian Ratiu]

There is a documented expectation that configured hooks are
run before the hook from the hookdir. Add a test for it.

While at it, I noticed that `git hook list -h` runs twice
in the `git hook usage` test, so remove one invocation.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 t/t1800-hook.sh | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 952bf97b86..7eee84fc39 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -25,7 +25,6 @@ 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 &&
 	test_expect_code 129 git hook list &&
 	test_expect_code 129 git hook list -h &&
@@ -381,6 +380,34 @@ test_expect_success 'globally disabled hook can be re-enabled locally' '
 	test_cmp expected actual
 '
 
+test_expect_success 'configured hooks run before hookdir hook' '
+	setup_hookdir &&
+	test_config hook.first.event "pre-commit" &&
+	test_config hook.first.command "echo first" &&
+	test_config hook.second.event "pre-commit" &&
+	test_config hook.second.command "echo second" &&
+
+	cat >expected <<-\EOF &&
+	first
+	second
+	hook from hookdir
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual &&
+
+	# "Legacy Hook" is the output of the hookdir pre-commit script
+	# written by setup_hookdir() above.
+	cat >expected <<-\EOF &&
+	first
+	second
+	"Legacy Hook"
+	EOF
+
+	git hook run pre-commit 2>actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'git hook run a hook with a bad shebang' '
 	test_when_finished "rm -rf bad-hooks" &&
 	mkdir bad-hooks &&
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 09/12] hook: introduce hook_config_cache_entry for per-hook data

[Adrian Ratiu]

Replace the bare `char *command` util pointer stored in each string_list
item with a heap-allocated `struct hook_config_cache_entry` that carries
that command string.

This is just a refactoring with no behavior changes, to give the cache
entry room to grow, so it can carry the additional hook metadata we'll
be adding in the following commits.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 hook.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/hook.c b/hook.c
index dc0c3de667..54f99f4989 100644
--- a/hook.c
+++ b/hook.c
@@ -108,6 +108,15 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 	string_list_append(hook_list, hook_path)->util = h;
 }
 
+/*
+ * Cache entry stored as the .util pointer of string_list items inside the
+ * hook config cache. For now carries only the command for the hook. Next
+ * commits will add more data.
+ */
+struct hook_config_cache_entry {
+	char *command;
+};
+
 /*
  * Callback struct to collect all hook.* keys in a single config pass.
  * commands: friendly-name to command map.
@@ -202,7 +211,12 @@ void hook_cache_clear(struct strmap *cache)
 
 	strmap_for_each_entry(cache, &iter, e) {
 		struct string_list *hooks = e->value;
-		string_list_clear(hooks, 1); /* free util (command) pointers */
+		for (size_t i = 0; i < hooks->nr; i++) {
+			struct hook_config_cache_entry *entry = hooks->items[i].util;
+			free(entry->command);
+			free(entry);
+		}
+		string_list_clear(hooks, 0);
 		free(hooks);
 	}
 	strmap_clear(cache, 0);
@@ -232,6 +246,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 
 		for (size_t i = 0; i < hook_names->nr; i++) {
 			const char *hname = hook_names->items[i].string;
+			struct hook_config_cache_entry *entry;
 			char *command;
 
 			/* filter out disabled hooks */
@@ -245,9 +260,10 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 				      "'hook.%s.event' must be removed;"
 				      " aborting."), hname, hname);
 
-			/* util stores the command; owned by the cache. */
-			string_list_append(hooks, hname)->util =
-				xstrdup(command);
+			/* util stores a cache entry; owned by the cache. */
+			CALLOC_ARRAY(entry, 1);
+			entry->command = xstrdup(command);
+			string_list_append(hooks, hname)->util = entry;
 		}
 
 		strmap_put(cache, e->key, hooks);
@@ -309,7 +325,7 @@ static void list_hooks_add_configured(struct repository *r,
 	/* Iterate through configured hooks and initialize internal states */
 	for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) {
 		const char *friendly_name = configured_hooks->items[i].string;
-		const char *command = configured_hooks->items[i].util;
+		struct hook_config_cache_entry *entry = configured_hooks->items[i].util;
 		struct hook *hook;
 
 		CALLOC_ARRAY(hook, 1);
@@ -327,7 +343,7 @@ static void list_hooks_add_configured(struct repository *r,
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
-		hook->u.configured.command = xstrdup(command);
+		hook->u.configured.command = xstrdup(entry->command);
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 10/12] hook: show config scope in git hook list

[Adrian Ratiu]

Users running "git hook list" can see which hooks are configured but
have no way to tell at which config scope (local, global, system...)
each hook was defined.

Store the scope from ctx->kvi->scope in the single-pass config callback,
then carry it through the cache to the hook structs, so we can expose it
to users via the "git hook list --show-scope" flag, which mirrors the
existing git config --show-scope convention.

Without the flag the output is unchanged.

The scope is printed as a tab-separated prefix (like "git config --show-scope"),
making it unambiguously machine-parseable even when the friendly name
contains spaces.

Example usage:
$ git hook list --show-scope pre-commit
global	linter
local	no-leaks
hook from hookdir

Traditional hooks from the hookdir are unaffected by --show-scope since
the config scope concept does not apply to them.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 Documentation/git-hook.adoc | 10 ++++++++--
 builtin/hook.c              | 14 ++++++++++++--
 hook.c                      | 24 ++++++++++++++++++++----
 hook.h                      |  2 ++
 t/t1800-hook.sh             | 21 +++++++++++++++++++++
 5 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index 966388660a..e7d399ae57 100644
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
-'git hook' list [-z] <hook-name>
+'git hook' list [-z] [--show-scope] <hook-name>
 
 DESCRIPTION
 -----------
@@ -113,7 +113,7 @@ Any positional arguments to the hook should be passed after a
 mandatory `--` (or `--end-of-options`, see linkgit:gitcli[7]). See
 linkgit:githooks[5] for arguments hooks might expect (if any).
 
-list [-z]::
+list [-z] [--show-scope]::
 	Print a list of hooks which will be run on `<hook-name>` event. If no
 	hooks are configured for that event, print a warning and return 1.
 	Use `-z` to terminate output lines with NUL instead of newlines.
@@ -134,6 +134,12 @@ OPTIONS
 -z::
 	Terminate "list" output lines with NUL instead of newlines.
 
+--show-scope::
+	For "list"; prefix each configured hook's friendly name with a
+	tab-separated config scope (e.g. `local`, `global`, `system`),
+	mirroring the output style of `git config --show-scope`. Traditional
+	hooks from the hookdir are unaffected.
+
 WRAPPERS
 --------
 
diff --git a/builtin/hook.c b/builtin/hook.c
index 54b737990b..4cc65a0dc5 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -9,7 +9,7 @@
 #define BUILTIN_HOOK_RUN_USAGE \
 	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
 #define BUILTIN_HOOK_LIST_USAGE \
-	N_("git hook list [-z] <hook-name>")
+	N_("git hook list [-z] [--show-scope] <hook-name>")
 
 static const char * const builtin_hook_usage[] = {
 	BUILTIN_HOOK_RUN_USAGE,
@@ -33,11 +33,14 @@ static int list(int argc, const char **argv, const char *prefix,
 	struct string_list_item *item;
 	const char *hookname = NULL;
 	int line_terminator = '\n';
+	int show_scope = 0;
 	int ret = 0;
 
 	struct option list_options[] = {
 		OPT_SET_INT('z', NULL, &line_terminator,
 			    N_("use NUL as line terminator"), '\0'),
+		OPT_BOOL(0, "show-scope", &show_scope,
+			 N_("show the config scope that defined each hook")),
 		OPT_END(),
 	};
 
@@ -70,7 +73,14 @@ static int list(int argc, const char **argv, const char *prefix,
 			printf("%s%c", _("hook from hookdir"), line_terminator);
 			break;
 		case HOOK_CONFIGURED:
-			printf("%s%c", h->u.configured.friendly_name, line_terminator);
+			if (show_scope)
+				printf("%s\t%s%c",
+				       config_scope_name(h->u.configured.scope),
+				       h->u.configured.friendly_name,
+				       line_terminator);
+			else
+				printf("%s%c", h->u.configured.friendly_name,
+				       line_terminator);
 			break;
 		default:
 			BUG("unknown hook kind");
diff --git a/hook.c b/hook.c
index 54f99f4989..74f5a1df35 100644
--- a/hook.c
+++ b/hook.c
@@ -110,11 +110,11 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 
 /*
  * Cache entry stored as the .util pointer of string_list items inside the
- * hook config cache. For now carries only the command for the hook. Next
- * commits will add more data.
+ * hook config cache.
  */
 struct hook_config_cache_entry {
 	char *command;
+	enum config_scope scope;
 };
 
 /*
@@ -131,7 +131,7 @@ struct hook_all_config_cb {
 
 /* repo_config() callback that collects all hook.* configuration in one pass. */
 static int hook_config_lookup_all(const char *key, const char *value,
-				  const struct config_context *ctx UNUSED,
+				  const struct config_context *ctx,
 				  void *cb_data)
 {
 	struct hook_all_config_cb *data = cb_data;
@@ -168,7 +168,19 @@ static int hook_config_lookup_all(const char *key, const char *value,
 
 			/* Re-insert if necessary to preserve last-seen order. */
 			unsorted_string_list_remove(hooks, hook_name, 0);
-			string_list_append(hooks, hook_name);
+
+			if (!ctx->kvi)
+				BUG("hook config callback called without key-value info");
+
+			/*
+			 * Stash the config scope in the util pointer for
+			 * later retrieval in build_hook_config_map(). This
+			 * intermediate struct is transient and never leaves
+			 * that function, so we pack the enum value into the
+			 * pointer rather than heap-allocating a wrapper.
+			 */
+			string_list_append(hooks, hook_name)->util =
+				(void *)(uintptr_t)ctx->kvi->scope;
 		}
 	} else if (!strcmp(subkey, "command")) {
 		/* Store command overwriting the old value */
@@ -246,6 +258,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 
 		for (size_t i = 0; i < hook_names->nr; i++) {
 			const char *hname = hook_names->items[i].string;
+			enum config_scope scope =
+				(enum config_scope)(uintptr_t)hook_names->items[i].util;
 			struct hook_config_cache_entry *entry;
 			char *command;
 
@@ -263,6 +277,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 			/* util stores a cache entry; owned by the cache. */
 			CALLOC_ARRAY(entry, 1);
 			entry->command = xstrdup(command);
+			entry->scope = scope;
 			string_list_append(hooks, hname)->util = entry;
 		}
 
@@ -344,6 +359,7 @@ static void list_hooks_add_configured(struct repository *r,
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
 		hook->u.configured.command = xstrdup(entry->command);
+		hook->u.configured.scope = entry->scope;
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
diff --git a/hook.h b/hook.h
index d2cf59e649..a0432e8307 100644
--- a/hook.h
+++ b/hook.h
@@ -1,5 +1,6 @@
 #ifndef HOOK_H
 #define HOOK_H
+#include "config.h"
 #include "run-command.h"
 #include "string-list.h"
 #include "strmap.h"
@@ -29,6 +30,7 @@ struct hook {
 		struct {
 			const char *friendly_name;
 			const char *command;
+			enum config_scope scope;
 		} configured;
 	} u;
 
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 7eee84fc39..6fc6603da8 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -408,6 +408,27 @@ test_expect_success 'configured hooks run before hookdir hook' '
 	test_cmp expected actual
 '
 
+test_expect_success 'git hook list --show-scope shows config scope' '
+	setup_hookdir &&
+	test_config_global hook.global-hook.command "echo global" &&
+	test_config_global hook.global-hook.event pre-commit --add &&
+	test_config hook.local-hook.command "echo local" &&
+	test_config hook.local-hook.event pre-commit --add &&
+
+	cat >expected <<-\EOF &&
+	global	global-hook
+	local	local-hook
+	hook from hookdir
+	EOF
+	git hook list --show-scope pre-commit >actual &&
+	test_cmp expected actual &&
+
+	# without --show-scope the scope must not appear
+	git hook list pre-commit >actual &&
+	test_grep ! "^global	" actual &&
+	test_grep ! "^local	" actual
+'
+
 test_expect_success 'git hook run a hook with a bad shebang' '
 	test_when_finished "rm -rf bad-hooks" &&
 	mkdir bad-hooks &&
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 11/12] hook: show disabled hooks in "git hook list"

[Adrian Ratiu]

Disabled hooks were filtered out of the cache entirely, making them
invisible to "git hook list". Keep them in the cache with a new
"disabled" flag which is propagated to the respective struct hook.

"git hook list" now shows disabled hooks as tab-separated columns,
with the status as a prefix before the name (like scope with
--show-scope). With --show-scope it looks like:

$ git hook list --show-scope pre-commit
global	linter
local	disabled	no-leaks
hook from hookdir

A disabled hook without a command issues a warning instead of the
fatal "hook.X.command must be configured" error. We could also throw
an error, however it seemd a bit excessive to me in this case.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 builtin/hook.c  | 20 ++++++++++--------
 hook.c          | 54 +++++++++++++++++++++++++++++++++----------------
 hook.h          |  1 +
 t/t1800-hook.sh | 33 +++++++++++++++++++++++++++---
 4 files changed, 80 insertions(+), 28 deletions(-)

diff --git a/builtin/hook.c b/builtin/hook.c
index 4cc65a0dc5..f671e7f91a 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -72,16 +72,20 @@ static int list(int argc, const char **argv, const char *prefix,
 		case HOOK_TRADITIONAL:
 			printf("%s%c", _("hook from hookdir"), line_terminator);
 			break;
-		case HOOK_CONFIGURED:
-			if (show_scope)
-				printf("%s\t%s%c",
-				       config_scope_name(h->u.configured.scope),
-				       h->u.configured.friendly_name,
-				       line_terminator);
+		case HOOK_CONFIGURED: {
+			const char *name = h->u.configured.friendly_name;
+			const char *scope = show_scope ?
+				config_scope_name(h->u.configured.scope) : NULL;
+			if (scope)
+				printf("%s\t%s%s%c", scope,
+				       h->u.configured.disabled ? "disabled\t" : "",
+				       name, line_terminator);
 			else
-				printf("%s%c", h->u.configured.friendly_name,
-				       line_terminator);
+				printf("%s%s%c",
+				       h->u.configured.disabled ? "disabled\t" : "",
+				       name, line_terminator);
 			break;
+		}
 		default:
 			BUG("unknown hook kind");
 		}
diff --git a/hook.c b/hook.c
index 74f5a1df35..cc23276d27 100644
--- a/hook.c
+++ b/hook.c
@@ -115,6 +115,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 struct hook_config_cache_entry {
 	char *command;
 	enum config_scope scope;
+	bool disabled;
 };
 
 /*
@@ -213,8 +214,10 @@ static int hook_config_lookup_all(const char *key, const char *value,
  * every item's string is the hook's friendly-name and its util pointer is
  * the corresponding command string. Both strings are owned by the map.
  *
- * Disabled hooks and hooks missing a command are already filtered out at
- * parse time, so callers can iterate the list directly.
+ * Disabled hooks are kept in the cache with entry->disabled set, so that
+ * "git hook list" can display them. A non-disabled hook missing a command
+ * is fatal; a disabled hook missing a command emits a warning and is kept
+ * in the cache with entry->command = NULL.
  */
 void hook_cache_clear(struct strmap *cache)
 {
@@ -263,21 +266,26 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
 			struct hook_config_cache_entry *entry;
 			char *command;
 
-			/* filter out disabled hooks */
-			if (unsorted_string_list_lookup(&cb_data.disabled_hooks,
-							hname))
-				continue;
+			bool is_disabled =
+				!!unsorted_string_list_lookup(
+					&cb_data.disabled_hooks, hname);
 
 			command = strmap_get(&cb_data.commands, hname);
-			if (!command)
-				die(_("'hook.%s.command' must be configured or "
-				      "'hook.%s.event' must be removed;"
-				      " aborting."), hname, hname);
+			if (!command) {
+				if (is_disabled)
+					warning(_("disabled hook '%s' has no "
+						  "command configured"), hname);
+				else
+					die(_("'hook.%s.command' must be configured or "
+					      "'hook.%s.event' must be removed;"
+					      " aborting."), hname, hname);
+			}
 
 			/* util stores a cache entry; owned by the cache. */
 			CALLOC_ARRAY(entry, 1);
-			entry->command = xstrdup(command);
+			entry->command = xstrdup_or_null(command);
 			entry->scope = scope;
+			entry->disabled = is_disabled;
 			string_list_append(hooks, hname)->util = entry;
 		}
 
@@ -358,8 +366,10 @@ static void list_hooks_add_configured(struct repository *r,
 
 		hook->kind = HOOK_CONFIGURED;
 		hook->u.configured.friendly_name = xstrdup(friendly_name);
-		hook->u.configured.command = xstrdup(entry->command);
+		hook->u.configured.command =
+			entry->command ? xstrdup(entry->command) : NULL;
 		hook->u.configured.scope = entry->scope;
+		hook->u.configured.disabled = entry->disabled;
 
 		string_list_append(list, friendly_name)->util = hook;
 	}
@@ -397,7 +407,16 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
 int hook_exists(struct repository *r, const char *name)
 {
 	struct string_list *hooks = list_hooks(r, name, NULL);
-	int exists = hooks->nr > 0;
+	int exists = 0;
+
+	for (size_t i = 0; i < hooks->nr; i++) {
+		struct hook *h = hooks->items[i].util;
+		if (h->kind == HOOK_TRADITIONAL ||
+		    !h->u.configured.disabled) {
+			exists = 1;
+			break;
+		}
+	}
 	string_list_clear_func(hooks, hook_free);
 	free(hooks);
 	return exists;
@@ -412,10 +431,11 @@ static int pick_next_hook(struct child_process *cp,
 	struct string_list *hook_list = hook_cb->hook_command_list;
 	struct hook *h;
 
-	if (hook_cb->hook_to_run_index >= hook_list->nr)
-		return 0;
-
-	h = hook_list->items[hook_cb->hook_to_run_index++].util;
+	do {
+		if (hook_cb->hook_to_run_index >= hook_list->nr)
+			return 0;
+		h = hook_list->items[hook_cb->hook_to_run_index++].util;
+	} while (h->kind == HOOK_CONFIGURED && h->u.configured.disabled);
 
 	cp->no_stdin = 1;
 	strvec_pushv(&cp->env, hook_cb->options->env.v);
diff --git a/hook.h b/hook.h
index a0432e8307..5c5628dd1f 100644
--- a/hook.h
+++ b/hook.h
@@ -31,6 +31,7 @@ struct hook {
 			const char *friendly_name;
 			const char *command;
 			enum config_scope scope;
+			bool disabled;
 		} configured;
 	} u;
 
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 6fc6603da8..8c5237449d 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -357,7 +357,15 @@ test_expect_success 'disabled hook is not run' '
 	test_must_be_empty actual
 '
 
-test_expect_success 'disabled hook does not appear in git hook list' '
+test_expect_success 'disabled hook with no command warns' '
+	test_config hook.nocommand.event "pre-commit" &&
+	test_config hook.nocommand.enabled false &&
+
+	git hook list pre-commit 2>actual &&
+	test_grep "disabled hook.*nocommand.*no command configured" actual
+'
+
+test_expect_success 'disabled hook appears as disabled in git hook list' '
 	test_config hook.active.event "pre-commit" &&
 	test_config hook.active.command "echo active" &&
 	test_config hook.inactive.event "pre-commit" &&
@@ -365,8 +373,27 @@ test_expect_success 'disabled hook does not appear in git hook list' '
 	test_config hook.inactive.enabled false &&
 
 	git hook list pre-commit >actual &&
-	test_grep "active" actual &&
-	test_grep ! "inactive" actual
+	test_grep "^active$" actual &&
+	test_grep "^disabled	inactive$" actual
+'
+
+test_expect_success 'disabled hook shows scope with --show-scope' '
+	test_config hook.myhook.event "pre-commit" &&
+	test_config hook.myhook.command "echo hi" &&
+	test_config hook.myhook.enabled false &&
+
+	git hook list --show-scope pre-commit >actual &&
+	test_grep "^local	disabled	myhook$" actual
+'
+
+test_expect_success 'disabled configured hook is not reported as existing by hook_exists' '
+	test_when_finished "rm -f git-bugreport-hook-exists-test.txt" &&
+	test_config hook.linter.event "pre-commit" &&
+	test_config hook.linter.command "echo lint" &&
+	test_config hook.linter.enabled false &&
+
+	git bugreport -s hook-exists-test &&
+	test_grep ! "pre-commit" git-bugreport-hook-exists-test.txt
 '
 
 test_expect_success 'globally disabled hook can be re-enabled locally' '
-- 
2.52.0.732.gb351b5166d.dirty

[PATCH v3 12/12] hook: reject unknown hook names in git-hook(1)

[Adrian Ratiu]

Teach "git hook run" and "git hook list" to reject hook event names
that are not recognized by Git. This helps catch typos such as
"prereceive" when "pre-receive" was intended, since in 99% of the
cases users want known (already-existing) hook names.

The list of known hooks is derived from the generated hook-list.h
(built from Documentation/githooks.adoc). This is why the Makefile
is updated, so builtin/hook.c depends on hook-list.h. In meson the
header is already a dependency for all builtins, no change required.

The "--allow-unknown-hook-name" flag can be used to bypass this check.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 Documentation/git-hook.adoc | 13 ++++--
 Makefile                    |  1 +
 builtin/hook.c              | 35 +++++++++++++++-
 t/t1800-hook.sh             | 82 +++++++++++++++++++++++++------------
 4 files changed, 100 insertions(+), 31 deletions(-)

diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index e7d399ae57..318c637bd8 100644
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -8,8 +8,8 @@ git-hook - Run git hooks
 SYNOPSIS
 --------
 [verse]
-'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
-'git hook' list [-z] [--show-scope] <hook-name>
+'git hook' run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
+'git hook' list [--allow-unknown-hook-name] [-z] [--show-scope] <hook-name>
 
 DESCRIPTION
 -----------
@@ -121,6 +121,13 @@ list [-z] [--show-scope]::
 OPTIONS
 -------
 
+--allow-unknown-hook-name::
+	By default `git hook run` and `git hook list` will bail out when
+	`<hook-name>` is not a hook event known to Git (see linkgit:githooks[5]
+	for the list of known hooks). This is meant to help catch typos
+	such as `prereceive` when `pre-receive` was intended. Pass this
+	flag to allow unknown hook names.
+
 --to-stdin::
 	For "run"; specify a file which will be streamed into the
 	hook's stdin. The hook will receive the entire file from
@@ -159,7 +166,7 @@ Then, in your 'mywrapper' tool, you can invoke any users' configured hooks by
 running:
 
 ----
-git hook run mywrapper-start-tests \
+git hook run --allow-unknown-hook-name mywrapper-start-tests \
   # providing something to stdin
   --stdin some-tempfile-123 \
   # execute hooks in serial
diff --git a/Makefile b/Makefile
index bf2228de9d..6d64431219 100644
--- a/Makefile
+++ b/Makefile
@@ -2673,6 +2673,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 
 help.sp help.s help.o: command-list.h
 builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h
+builtin/hook.sp builtin/hook.s builtin/hook.o: hook-list.h
 
 builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
diff --git a/builtin/hook.c b/builtin/hook.c
index f671e7f91a..c0585587e5 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -4,12 +4,22 @@
 #include "environment.h"
 #include "gettext.h"
 #include "hook.h"
+#include "hook-list.h"
 #include "parse-options.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
-	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
+	N_("git hook run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
 #define BUILTIN_HOOK_LIST_USAGE \
-	N_("git hook list [-z] [--show-scope] <hook-name>")
+	N_("git hook list [--allow-unknown-hook-name] [-z] [--show-scope] <hook-name>")
+
+static int is_known_hook(const char *name)
+{
+	const char **p;
+	for (p = hook_name_list; *p; p++)
+		if (!strcmp(*p, name))
+			return 1;
+	return 0;
+}
 
 static const char * const builtin_hook_usage[] = {
 	BUILTIN_HOOK_RUN_USAGE,
@@ -34,6 +44,7 @@ static int list(int argc, const char **argv, const char *prefix,
 	const char *hookname = NULL;
 	int line_terminator = '\n';
 	int show_scope = 0;
+	int allow_unknown = 0;
 	int ret = 0;
 
 	struct option list_options[] = {
@@ -41,6 +52,8 @@ static int list(int argc, const char **argv, const char *prefix,
 			    N_("use NUL as line terminator"), '\0'),
 		OPT_BOOL(0, "show-scope", &show_scope,
 			 N_("show the config scope that defined each hook")),
+		OPT_BOOL(0, "allow-unknown-hook-name", &allow_unknown,
+			 N_("allow running a hook with a non-native hook name")),
 		OPT_END(),
 	};
 
@@ -57,6 +70,13 @@ static int list(int argc, const char **argv, const char *prefix,
 
 	hookname = argv[0];
 
+	if (!allow_unknown && !is_known_hook(hookname)) {
+		error(_("unknown hook event '%s';\n"
+			"use --allow-unknown-hook-name to allow non-native hook names"),
+		      hookname);
+		return 1;
+	}
+
 	head = list_hooks(repo, hookname, NULL);
 
 	if (!head->nr) {
@@ -103,8 +123,11 @@ static int run(int argc, const char **argv, const char *prefix,
 	int i;
 	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
 	int ignore_missing = 0;
+	int allow_unknown = 0;
 	const char *hook_name;
 	struct option run_options[] = {
+		OPT_BOOL(0, "allow-unknown-hook-name", &allow_unknown,
+			 N_("allow running a hook with a non-native hook name")),
 		OPT_BOOL(0, "ignore-missing", &ignore_missing,
 			 N_("silently ignore missing requested <hook-name>")),
 		OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"),
@@ -136,6 +159,14 @@ static int run(int argc, const char **argv, const char *prefix,
 	repo_config(the_repository, git_default_config, NULL);
 
 	hook_name = argv[0];
+
+	if (!allow_unknown && !is_known_hook(hook_name)) {
+		error(_("unknown hook event '%s';\n"
+			"use --allow-unknown-hook-name to allow non-native hook names"),
+		      hook_name);
+		return 1;
+	}
+
 	if (!ignore_missing)
 		opt.error_if_missing = 1;
 	ret = run_hooks_opt(the_repository, hook_name, &opt);
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 8c5237449d..96749fc06d 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -31,11 +31,41 @@ test_expect_success 'git hook usage' '
 	grep "unknown option" err
 '
 
+test_expect_success 'git hook list: unknown hook name is rejected' '
+	test_must_fail git hook list prereceive 2>err &&
+	test_grep "unknown hook event" err
+'
+
+test_expect_success 'git hook run: unknown hook name is rejected' '
+	test_must_fail git hook run prereceive 2>err &&
+	test_grep "unknown hook event" err
+'
+
+test_expect_success 'git hook list: known hook name is accepted' '
+	test_must_fail git hook list pre-receive 2>err &&
+	test_grep ! "unknown hook event" err
+'
+
+test_expect_success 'git hook run: known hook name is accepted' '
+	git hook run --ignore-missing pre-receive 2>err &&
+	test_grep ! "unknown hook event" err
+'
+
+test_expect_success 'git hook run: --allow-unknown-hook-name overrides rejection' '
+	git hook run --allow-unknown-hook-name --ignore-missing custom-hook 2>err &&
+	test_grep ! "unknown hook event" err
+'
+
+test_expect_success 'git hook list: --allow-unknown-hook-name overrides rejection' '
+	test_must_fail git hook list --allow-unknown-hook-name custom-hook 2>err &&
+	test_grep ! "unknown hook event" err
+'
+
 test_expect_success 'git hook list: nonexistent hook' '
 	cat >stderr.expect <<-\EOF &&
 	warning: no hooks found for event '\''test-hook'\''
 	EOF
-	test_expect_code 1 git hook list test-hook 2>stderr.actual &&
+	test_expect_code 1 git hook list --allow-unknown-hook-name test-hook 2>stderr.actual &&
 	test_cmp stderr.expect stderr.actual
 '
 
@@ -47,7 +77,7 @@ test_expect_success 'git hook list: traditional hook from hookdir' '
 	cat >expect <<-\EOF &&
 	hook from hookdir
 	EOF
-	git hook list test-hook >actual &&
+	git hook list --allow-unknown-hook-name test-hook >actual &&
 	test_cmp expect actual
 '
 
@@ -56,7 +86,7 @@ test_expect_success 'git hook list: configured hook' '
 	test_config hook.myhook.event test-hook --add &&
 
 	echo "myhook" >expect &&
-	git hook list test-hook >actual &&
+	git hook list --allow-unknown-hook-name test-hook >actual &&
 	test_cmp expect actual
 '
 
@@ -68,7 +98,7 @@ test_expect_success 'git hook list: -z shows NUL-terminated output' '
 	test_config hook.myhook.event test-hook --add &&
 
 	printf "myhookQhook from hookdirQ" >expect &&
-	git hook list -z test-hook >actual.raw &&
+	git hook list --allow-unknown-hook-name -z test-hook >actual.raw &&
 	nul_to_q <actual.raw >actual &&
 	test_cmp expect actual
 '
@@ -77,12 +107,12 @@ test_expect_success 'git hook run: nonexistent hook' '
 	cat >stderr.expect <<-\EOF &&
 	error: cannot find a hook named test-hook
 	EOF
-	test_expect_code 1 git hook run test-hook 2>stderr.actual &&
+	test_expect_code 1 git hook run --allow-unknown-hook-name test-hook 2>stderr.actual &&
 	test_cmp stderr.expect stderr.actual
 '
 
 test_expect_success 'git hook run: nonexistent hook with --ignore-missing' '
-	git hook run --ignore-missing does-not-exist 2>stderr.actual &&
+	git hook run --allow-unknown-hook-name --ignore-missing does-not-exist 2>stderr.actual &&
 	test_must_be_empty stderr.actual
 '
 
@@ -94,7 +124,7 @@ test_expect_success 'git hook run: basic' '
 	cat >expect <<-\EOF &&
 	Test hook
 	EOF
-	git hook run test-hook 2>actual &&
+	git hook run --allow-unknown-hook-name test-hook 2>actual &&
 	test_cmp expect actual
 '
 
@@ -108,7 +138,7 @@ test_expect_success 'git hook run: stdout and stderr both write to our stderr' '
 	Will end up on stderr
 	Will end up on stderr
 	EOF
-	git hook run test-hook >stdout.actual 2>stderr.actual &&
+	git hook run --allow-unknown-hook-name test-hook >stdout.actual 2>stderr.actual &&
 	test_cmp stderr.expect stderr.actual &&
 	test_must_be_empty stdout.actual
 '
@@ -120,12 +150,12 @@ do
 		exit $code
 		EOF
 
-		test_expect_code $code git hook run test-hook
+		test_expect_code $code git hook run --allow-unknown-hook-name test-hook
 	'
 done
 
 test_expect_success 'git hook run arg u ments without -- is not allowed' '
-	test_expect_code 129 git hook run test-hook arg u ments
+	test_expect_code 129 git hook run --allow-unknown-hook-name test-hook arg u ments
 '
 
 test_expect_success 'git hook run -- pass arguments' '
@@ -139,7 +169,7 @@ test_expect_success 'git hook run -- pass arguments' '
 	u ments
 	EOF
 
-	git hook run test-hook -- arg "u ments" 2>actual &&
+	git hook run --allow-unknown-hook-name test-hook -- arg "u ments" 2>actual &&
 	test_cmp expect actual
 '
 
@@ -148,12 +178,12 @@ test_expect_success 'git hook run: out-of-repo runs execute global hooks' '
 	test_config_global hook.global-hook.command "echo no repo no problems" --add &&
 
 	echo "global-hook" >expect &&
-	nongit git hook list test-hook >actual &&
+	nongit git hook list --allow-unknown-hook-name test-hook >actual &&
 	test_cmp expect actual &&
 
 	echo "no repo no problems" >expect &&
 
-	nongit git hook run test-hook 2>actual &&
+	nongit git hook run --allow-unknown-hook-name test-hook 2>actual &&
 	test_cmp expect actual
 '
 
@@ -178,11 +208,11 @@ test_expect_success 'git -c core.hooksPath=<PATH> hook run' '
 	# Test various ways of specifying the path. See also
 	# t1350-config-hooks-path.sh
 	>actual &&
-	git hook run test-hook -- ignored 2>>actual &&
-	git -c core.hooksPath=my-hooks hook run test-hook -- one 2>>actual &&
-	git -c core.hooksPath=my-hooks/ hook run test-hook -- two 2>>actual &&
-	git -c core.hooksPath="$PWD/my-hooks" hook run test-hook -- three 2>>actual &&
-	git -c core.hooksPath="$PWD/my-hooks/" hook run test-hook -- four 2>>actual &&
+	git hook run --allow-unknown-hook-name test-hook -- ignored 2>>actual &&
+	git -c core.hooksPath=my-hooks hook run --allow-unknown-hook-name test-hook -- one 2>>actual &&
+	git -c core.hooksPath=my-hooks/ hook run --allow-unknown-hook-name test-hook -- two 2>>actual &&
+	git -c core.hooksPath="$PWD/my-hooks" hook run --allow-unknown-hook-name test-hook -- three 2>>actual &&
+	git -c core.hooksPath="$PWD/my-hooks/" hook run --allow-unknown-hook-name test-hook -- four 2>>actual &&
 	test_cmp expect actual
 '
 
@@ -262,7 +292,7 @@ test_expect_success 'hook can be configured for multiple events' '
 	# '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 &&
+	git hook list --allow-unknown-hook-name test-hook >actual &&
 	grep "ghi" actual
 '
 
@@ -336,15 +366,15 @@ test_expect_success 'stdin to multiple hooks' '
 	b3
 	EOF
 
-	git hook run --to-stdin=input test-hook 2>actual &&
+	git hook run --allow-unknown-hook-name --to-stdin=input test-hook 2>actual &&
 	test_cmp expected actual
 '
 
 test_expect_success 'rejects hooks with no commands configured' '
 	test_config hook.broken.event "test-hook" &&
-	test_must_fail git hook list test-hook 2>actual &&
+	test_must_fail git hook list --allow-unknown-hook-name test-hook 2>actual &&
 	test_grep "hook.broken.command" actual &&
-	test_must_fail git hook run test-hook 2>actual &&
+	test_must_fail git hook run --allow-unknown-hook-name test-hook 2>actual &&
 	test_grep "hook.broken.command" actual
 '
 
@@ -353,7 +383,7 @@ test_expect_success 'disabled hook is not run' '
 	test_config hook.skipped.command "echo \"Should not run\"" &&
 	test_config hook.skipped.enabled false &&
 
-	git hook run --ignore-missing test-hook 2>actual &&
+	git hook run --allow-unknown-hook-name --ignore-missing test-hook 2>actual &&
 	test_must_be_empty actual
 '
 
@@ -403,7 +433,7 @@ test_expect_success 'globally disabled hook can be re-enabled locally' '
 	test_config hook.global-hook.enabled true &&
 
 	echo "global-hook ran" >expected &&
-	git hook run test-hook 2>actual &&
+	git hook run --allow-unknown-hook-name test-hook 2>actual &&
 	test_cmp expected actual
 '
 
@@ -463,7 +493,7 @@ test_expect_success 'git hook run a hook with a bad shebang' '
 
 	test_expect_code 1 git \
 		-c core.hooksPath=bad-hooks \
-		hook run test-hook >out 2>err &&
+		hook run --allow-unknown-hook-name test-hook >out 2>err &&
 	test_must_be_empty out &&
 
 	# TODO: We should emit the same (or at least a more similar)
@@ -487,7 +517,7 @@ test_expect_success 'stdin to hooks' '
 	EOF
 
 	echo hello >input &&
-	git hook run --to-stdin=input test-hook 2>actual &&
+	git hook run --allow-unknown-hook-name --to-stdin=input test-hook 2>actual &&
 	test_cmp expect actual
 '
 
-- 
2.52.0.732.gb351b5166d.dirty

Re: [PATCH v3 00/12] config-hook cleanups and three small git-hook features

[Junio C Hamano]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> Hello everyone,
>
> v3 addresses all the feedback and requests received in v2, many thanks to all
> who contributed.
>
> Let's please stop adding features since this is getting rather big again. :)
> New features can be added in subsequent patches.
>
> This series is mostly for minor cleanups, bug fixes and refactorings + three
> minor feature additions to git-hook, which resulted from review discussions:
>
> 1. The ability to show the config scope (--show-scope).
> 2. The ability to show which hooks are disabled.
> 3. The ability reject unknown hook names with "--allow-unknown-hook-name" as
>    an escape hatch.
>
> The series is based on the master branch.

Replaced the old one, and then rebuilt ar/parallel-hooks on top.
Please sanity-check the latter when I later push out the result of
today's integration.

One thing I noticed a bit annoying was that we have "event_disabled"
boolean in "struct hook", plus a string-list of the same name in
"struct repository", which means "git grep event_disabled" hits
both.  Perhaps the caching "struct string_list event_disabled" can
be renamed to reflect what it is a bit better, like "disabled_events"?

Re: [PATCH v3 00/12] config-hook cleanups and three small git-hook features

[Adrian Ratiu]

On Wed, 25 Mar 2026, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
>
>> Hello everyone,
>>
>> v3 addresses all the feedback and requests received in v2, many thanks to all
>> who contributed.
>>
>> Let's please stop adding features since this is getting rather big again. :)
>> New features can be added in subsequent patches.
>>
>> This series is mostly for minor cleanups, bug fixes and refactorings + three
>> minor feature additions to git-hook, which resulted from review discussions:
>>
>> 1. The ability to show the config scope (--show-scope).
>> 2. The ability to show which hooks are disabled.
>> 3. The ability reject unknown hook names with "--allow-unknown-hook-name" as
>>    an escape hatch.
>>
>> The series is based on the master branch.
>
> Replaced the old one, and then rebuilt ar/parallel-hooks on top.
> Please sanity-check the latter when I later push out the result of
> today's integration.

ar/parallel-hooks is expected to break with this v3.
Sorry I didn't make this clear in the cover letter.

What broke it is the new --allow-unknown-hook-name feature and the
code-review changes (I'd be very surprised if the old ar/parallel-hooks
applied cleanly on top of this, I fixed quite a few conflicts).

Please use v5 of ar/parallel-hooks which I just posted and should apply
cleanly on top of this v3.

> One thing I noticed a bit annoying was that we have "event_disabled"
> boolean in "struct hook", plus a string-list of the same name in
> "struct repository", which means "git grep event_disabled" hits
> both.  Perhaps the caching "struct string_list event_disabled" can
> be renamed to reflect what it is a bit better, like "disabled_events"?

Fixed in parallel-hooks v5.

Thanks!

Re: [PATCH v3 00/12] config-hook cleanups and three small git-hook features

[Patrick Steinhardt]

On Wed, Mar 25, 2026 at 09:54:51PM +0200, Adrian Ratiu wrote:
> Changes in v3:
> * New commit: properly initialize strbuf in receive-pack.c (Patrick)
> * New commit: add a check which prevents unknown hooks with git-hook(1) (Patrick)
> * Removed duplicated function doc comment between .h and .c files (Patrick)
> * Extended `git hook list` test to also include a hook from the hookdir (Patrick)
> * Converted unsigned int disabled:1 to proper bool (Patrick)
> * Minor commit rewording, header sorting, blank line fixes (Patrick)

Thanks, I'm happy with this version!

Patrick

Re: [PATCH v3 00/12] config-hook cleanups and three small git-hook features

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Mar 25, 2026 at 09:54:51PM +0200, Adrian Ratiu wrote:
>> Changes in v3:
>> * New commit: properly initialize strbuf in receive-pack.c (Patrick)
>> * New commit: add a check which prevents unknown hooks with git-hook(1) (Patrick)
>> * Removed duplicated function doc comment between .h and .c files (Patrick)
>> * Extended `git hook list` test to also include a hook from the hookdir (Patrick)
>> * Converted unsigned int disabled:1 to proper bool (Patrick)
>> * Minor commit rewording, header sorting, blank line fixes (Patrick)
>
> Thanks, I'm happy with this version!
>
> Patrick

Thanks, both.