From: Adrian Ratiu Date: Wed, 25 Mar 2026 19:54:51 GMT Subject: [PATCH v3 00/12] config-hook cleanups and three small git-hook features 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 "), @@ 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 Signed-off-by: Adrian Ratiu @@ 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