[PATCH v3 00/12] config-hook cleanups and three small git-hook features
- From
Adrian Ratiu <adrian.ratiu@collabora.com>
- Date
- Mar 25, 2026, 19:54 UTC
- Message-ID
- <20260325195503.1139418-1-adrian.ratiu@collabora.com>
- In-Reply-To
- <20260309005416.2760030-1-adrian.ratiu@collabora.com>
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