public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Adrian Ratiu <adrian.ratiu@collabora•com>
To: git@vger•kernel.org
Cc: Jeff King <peff@peff•net>,
	Emily Shaffer <emilyshaffer@google•com>,
	Junio C Hamano <gitster@pobox•com>,
	Patrick Steinhardt <ps@pks•im>,
	Josh Steadmon <steadmon@google•com>,
	Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail•com>,
	Adrian Ratiu <adrian.ratiu@collabora•com>
Subject: [PATCH v2 2/8] hook: run a list of hooks to prepare for multihook support
Date: Thu, 19 Feb 2026 00:23:46 +0200	[thread overview]
Message-ID: <20260218222352.55393-3-adrian.ratiu@collabora.com> (raw)
In-Reply-To: <20260218222352.55393-1-adrian.ratiu@collabora.com>

From: Emily Shaffer <emilyshaffer@google•com>

Hooks are limited to run one command (the default from the hookdir) for
each event. This limitation makes it impossible to run multiple commands
via config files, which the next commits will add.

Implement the ability to run a list of hooks in hook.[ch]. For now, the
list contains only one entry representing the "default" hook from the
hookdir, so there is no user-visible change in this commit.

All hook commands still run sequentially like before. A separate patch
series will enable running them in parallel.

Signed-off-by: Emily Shaffer <emilyshaffer@google•com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora•com>
---
 hook.c | 139 ++++++++++++++++++++++++++++++++++++++++++++-------------
 hook.h |  59 ++++++++++++++++++------
 2 files changed, 153 insertions(+), 45 deletions(-)

diff --git a/hook.c b/hook.c
index 83ff658866..c008a7232d 100644
--- a/hook.c
+++ b/hook.c
@@ -47,9 +47,97 @@ 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)
+{
+	if (!h)
+		return;
+
+	if (h->kind == HOOK_TRADITIONAL)
+		free((void *)h->u.traditional.path);
+
+	if (cb_data_free)
+		cb_data_free(h->feed_pipe_cb_data);
+
+	free(h);
+}
+
+static void hook_list_clear(struct string_list *hooks, cb_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,
+				   struct run_hooks_opt *options)
+{
+	const char *hook_path = find_hook(r, hookname);
+	struct hook *h;
+
+	if (!hook_path)
+		return;
+
+	h = xcalloc(1, sizeof(struct hook));
+
+	/*
+	 * If the hook is to run in a specific dir, a relative path can
+	 * become invalid in that dir, so convert to an absolute path.
+	 */
+	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)
+		h->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx);
+
+	h->kind = HOOK_TRADITIONAL;
+	h->u.traditional.path = xstrdup(hook_path);
+
+	string_list_append(hook_list, hook_path)->util = h;
+}
+
+/*
+ * Provides a list of hook commands to run for the 'hookname' event.
+ *
+ * This function consolidates hooks from two sources:
+ * 1. The config-based hooks (not yet implemented).
+ * 2. The "traditional" hook found in the repository hooks directory
+ *    (e.g., .git/hooks/pre-commit).
+ *
+ * The list is ordered by execution priority.
+ *
+ * The caller is responsible for freeing the memory of the returned list
+ * using string_list_clear() and free().
+ */
+static struct string_list *list_hooks(struct repository *r, const char *hookname,
+			       struct run_hooks_opt *options)
+{
+	struct string_list *hook_head;
+
+	if (!hookname)
+		BUG("null hookname was provided to hook_list()!");
+
+	hook_head = xmalloc(sizeof(struct string_list));
+	string_list_init_dup(hook_head);
+
+	/* Add the default "traditional" hooks from hookdir. */
+	list_hooks_add_default(r, hookname, hook_head, options);
+
+	return hook_head;
+}
+
 int hook_exists(struct repository *r, const char *name)
 {
-	return !!find_hook(r, name);
+	struct string_list *hooks = list_hooks(r, name, NULL);
+	int exists = hooks->nr > 0;
+	hook_list_clear(hooks, NULL);
+	free(hooks);
+	return exists;
 }
 
 static int pick_next_hook(struct child_process *cp,
@@ -58,11 +146,14 @@ static int pick_next_hook(struct child_process *cp,
 			  void **pp_task_cb)
 {
 	struct hook_cb_data *hook_cb = pp_cb;
-	const char *hook_path = hook_cb->hook_path;
+	struct string_list *hook_list = hook_cb->hook_command_list;
+	struct hook *h;
 
-	if (!hook_path)
+	if (hook_cb->hook_to_run_index >= hook_list->nr)
 		return 0;
 
+	h = hook_list->items[hook_cb->hook_to_run_index++].util;
+
 	cp->no_stdin = 1;
 	strvec_pushv(&cp->env, hook_cb->options->env.v);
 
@@ -85,21 +176,20 @@ static int pick_next_hook(struct child_process *cp,
 	cp->trace2_hook_name = hook_cb->hook_name;
 	cp->dir = hook_cb->options->dir;
 
-	strvec_push(&cp->args, hook_path);
+	/* Add hook exec paths or commands */
+	if (h->kind == HOOK_TRADITIONAL)
+		strvec_push(&cp->args, h->u.traditional.path);
+
+	if (!cp->args.nr)
+		BUG("hook must have at least one command or exec path");
+
 	strvec_pushv(&cp->args, hook_cb->options->args.v);
 
 	/*
 	 * Provide per-hook internal state via task_cb for easy access, so
 	 * hook callbacks don't have to go through hook_cb->options.
 	 */
-	*pp_task_cb = hook_cb->options->feed_pipe_cb_data;
-
-	/*
-	 * This pick_next_hook() will be called again, we're only
-	 * running one hook, so indicate that no more work will be
-	 * done.
-	 */
-	hook_cb->hook_path = NULL;
+	*pp_task_cb = h->feed_pipe_cb_data;
 
 	return 1;
 }
@@ -133,8 +223,6 @@ static int notify_hook_finished(int result,
 
 static void run_hooks_opt_clear(struct run_hooks_opt *options)
 {
-	if (options->feed_pipe_cb_data_free)
-		options->feed_pipe_cb_data_free(options->feed_pipe_cb_data);
 	strvec_clear(&options->env);
 	strvec_clear(&options->args);
 }
@@ -142,13 +230,11 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options)
 int run_hooks_opt(struct repository *r, const char *hook_name,
 		  struct run_hooks_opt *options)
 {
-	struct strbuf abs_path = STRBUF_INIT;
 	struct hook_cb_data cb_data = {
 		.rc = 0,
 		.hook_name = hook_name,
 		.options = options,
 	};
-	const char *const hook_path = find_hook(r, hook_name);
 	int ret = 0;
 	const struct run_process_parallel_opts opts = {
 		.tr2_category = "hook",
@@ -182,30 +268,21 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
 	    (!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->feed_pipe_cb_data_alloc)
-		options->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx);
-
 	if (options->invoked_hook)
 		*options->invoked_hook = 0;
 
-	if (!hook_path && !options->error_if_missing)
-		goto cleanup;
-
-	if (!hook_path) {
-		ret = error("cannot find a hook named %s", hook_name);
+	cb_data.hook_command_list = list_hooks(r, hook_name, options);
+	if (!cb_data.hook_command_list->nr) {
+		if (options->error_if_missing)
+			ret = error("cannot find a hook named %s", hook_name);
 		goto cleanup;
 	}
 
-	cb_data.hook_path = hook_path;
-	if (options->dir) {
-		strbuf_add_absolute_path(&abs_path, hook_path);
-		cb_data.hook_path = abs_path.buf;
-	}
-
 	run_processes_parallel(&opts);
 	ret = cb_data.rc;
 cleanup:
-	strbuf_release(&abs_path);
+	hook_list_clear(cb_data.hook_command_list, options->feed_pipe_cb_data_free);
+	free(cb_data.hook_command_list);
 	run_hooks_opt_clear(options);
 	return ret;
 }
diff --git a/hook.h b/hook.h
index a6bdc6f90f..3256d2dddb 100644
--- a/hook.h
+++ b/hook.h
@@ -2,9 +2,41 @@
 #define HOOK_H
 #include "strvec.h"
 #include "run-command.h"
+#include "string-list.h"
 
 struct repository;
 
+/**
+ * Represents a hook command to be run.
+ * Hooks can be:
+ * 1. "traditional" (found in the hooks directory)
+ * 2. "configured" (defined in Git's configuration, not yet implemented).
+ * The 'kind' field determines which part of the union 'u' is valid.
+ */
+struct hook {
+	enum {
+		HOOK_TRADITIONAL,
+	} kind;
+	union {
+		struct {
+			const char *path;
+		} traditional;
+	} u;
+
+	/**
+	 * Opaque data pointer used to keep internal state across callback calls.
+	 *
+	 * It can be accessed directly via the third hook callback arg:
+	 * struct ... *state = pp_task_cb;
+	 *
+	 * The caller is responsible for managing the memory for this data by
+	 * providing alloc/free callbacks to `run_hooks_opt`.
+	 *
+	 * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it.
+	 */
+	void *feed_pipe_cb_data;
+};
+
 typedef void (*cb_data_free_fn)(void *data);
 typedef void *(*cb_data_alloc_fn)(void *init_ctx);
 
@@ -85,19 +117,6 @@ struct run_hooks_opt
 	 */
 	void *feed_pipe_ctx;
 
-	/**
-	 * Opaque data pointer used to keep internal state across callback calls.
-	 *
-	 * It can be accessed directly via the third callback arg 'pp_task_cb':
-	 * struct ... *state = pp_task_cb;
-	 *
-	 * The caller is responsible for managing the memory for this data by
-	 * providing alloc/free callbacks to `run_hooks_opt`.
-	 *
-	 * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it.
-	 */
-	void *feed_pipe_cb_data;
-
 	/**
 	 * Some hooks need to create a fresh `feed_pipe_cb_data` internal state,
 	 * so they can keep track of progress without affecting one another.
@@ -128,7 +147,19 @@ struct hook_cb_data {
 	/* rc reflects the cumulative failure state */
 	int rc;
 	const char *hook_name;
-	const char *hook_path;
+
+	/**
+	 * A list of hook commands/paths to run for the 'hook_name' event.
+	 *
+	 * The 'string' member of each item holds the path (for traditional hooks)
+	 * or the unique friendly-name for hooks specified in configs.
+	 * The 'util' member of each item points to the corresponding struct hook.
+	 */
+	struct string_list *hook_command_list;
+
+	/* Iterator/cursor for the above list, pointing to the next hook to run. */
+	size_t hook_to_run_index;
+
 	struct run_hooks_opt *options;
 };
 
-- 
2.52.0.732.gb351b5166d.dirty


  parent reply	other threads:[~2026-02-18 22:24 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-04 16:51 [PATCH 0/4] Specify hooks via configs Adrian Ratiu
2026-02-04 16:51 ` [PATCH 1/4] hook: run a list of hooks Adrian Ratiu
2026-02-05 21:59   ` Junio C Hamano
2026-02-06 11:21     ` Adrian Ratiu
2026-02-09 14:27   ` Patrick Steinhardt
2026-02-09 18:16     ` Adrian Ratiu
2026-02-10 13:43       ` Patrick Steinhardt
2026-02-04 16:51 ` [PATCH 2/4] hook: introduce "git hook list" Adrian Ratiu
2026-02-09 14:28   ` Patrick Steinhardt
2026-02-09 18:26     ` Adrian Ratiu
2026-02-04 16:51 ` [PATCH 3/4] hook: include hooks from the config Adrian Ratiu
2026-02-09 14:28   ` Patrick Steinhardt
2026-02-09 19:10     ` Adrian Ratiu
2026-02-10 13:43       ` Patrick Steinhardt
2026-02-10 13:56         ` Adrian Ratiu
2026-02-04 16:51 ` [PATCH 4/4] hook: allow out-of-repo 'git hook' invocations Adrian Ratiu
2026-02-06 16:26 ` [PATCH 0/4] Specify hooks via configs Junio C Hamano
2026-02-18 22:23 ` [PATCH v2 0/8] " Adrian Ratiu
2026-02-18 22:23   ` [PATCH v2 1/8] hook: add internal state alloc/free callbacks Adrian Ratiu
2026-02-19 21:47     ` Junio C Hamano
2026-02-20 12:35       ` Adrian Ratiu
2026-02-20 17:21         ` Junio C Hamano
2026-02-20 12:42       ` Adrian Ratiu
2026-02-20 12:45     ` Patrick Steinhardt
2026-02-20 13:40       ` Adrian Ratiu
2026-02-18 22:23   ` Adrian Ratiu [this message]
2026-02-20 12:46     ` [PATCH v2 2/8] hook: run a list of hooks to prepare for multihook support Patrick Steinhardt
2026-02-20 13:51       ` Adrian Ratiu
2026-02-18 22:23   ` [PATCH v2 3/8] hook: add "git hook list" command Adrian Ratiu
2026-02-20 12:46     ` Patrick Steinhardt
2026-02-20 13:53       ` Adrian Ratiu
2026-02-18 22:23   ` [PATCH v2 4/8] hook: include hooks from the config Adrian Ratiu
2026-02-19 22:16     ` Junio C Hamano
2026-02-20 12:27       ` Adrian Ratiu
2026-02-20 12:46     ` Patrick Steinhardt
2026-02-20 14:31       ` Adrian Ratiu
2026-02-18 22:23   ` [PATCH v2 5/8] hook: allow disabling config hooks Adrian Ratiu
2026-02-20 12:46     ` Patrick Steinhardt
2026-02-20 14:47       ` Adrian Ratiu
2026-02-20 18:40         ` Patrick Steinhardt
2026-02-20 18:45           ` Junio C Hamano
2026-02-18 22:23   ` [PATCH v2 6/8] hook: allow event = "" to overwrite previous values Adrian Ratiu
2026-02-18 22:23   ` [PATCH v2 7/8] hook: allow out-of-repo 'git hook' invocations Adrian Ratiu
2026-02-18 22:23   ` [PATCH v2 8/8] hook: add -z option to "git hook list" Adrian Ratiu
2026-02-19 21:34   ` [PATCH v2 0/8] Specify hooks via configs Junio C Hamano
2026-02-20 12:51     ` Adrian Ratiu
2026-02-20 23:29   ` brian m. carlson
2026-02-21 14:27     ` Adrian Ratiu
2026-02-22  0:39       ` Adrian Ratiu
2026-02-25 18:37         ` Junio C Hamano
2026-02-26 12:21           ` Adrian Ratiu
2026-02-25 22:30         ` brian m. carlson
2026-02-26 12:41           ` Adrian Ratiu
2026-03-01 18:44 ` [PATCH v3 00/12][next] " Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 01/12] hook: add internal state alloc/free callbacks Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 02/12] hook: run a list of hooks to prepare for multihook support Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 03/12] hook: add "git hook list" command Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 04/12] string-list: add unsorted_string_list_remove() Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 05/12] hook: include hooks from the config Adrian Ratiu
2026-04-06 16:39     ` SZEDER Gábor
2026-04-08 11:28       ` Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 06/12] hook: allow disabling config hooks Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 07/12] hook: allow event = "" to overwrite previous values Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 08/12] hook: allow out-of-repo 'git hook' invocations Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 09/12] hook: add -z option to "git hook list" Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 10/12] hook: refactor hook_config_cache from strmap to named struct Adrian Ratiu
2026-03-01 18:44   ` [PATCH v3 11/12] hook: store and display scope for configured hooks in git hook list Adrian Ratiu
2026-03-01 18:45   ` [PATCH v3 12/12] hook: show disabled hooks in "git hook list" Adrian Ratiu
2026-03-02 16:48   ` [PATCH v3 00/12][next] Specify hooks via configs Junio C Hamano
2026-03-02 17:04     ` Adrian Ratiu
2026-03-02 18:48       ` Junio C Hamano

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20260218222352.55393-3-adrian.ratiu@collabora.com \
    --to=adrian.ratiu@collabora$(echo .)com \
    --cc=emilyshaffer@google$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=kristofferhaugsbakk@fastmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=ps@pks$(echo .)im \
    --cc=steadmon@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox