From 74a5ff8faea1c8bbb08b3b82e9b97fd507425927 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 7 Nov 2025 10:15:48 -0500 Subject: [PATCH 1/2] t0401: test post-command for alias, version, typo When the top-level Git process is an alias, it doesn't load much config and thus the postCommand.strategy config setting is not loaded properly. This leads to the post-command hook being run more frequently than we want, including an alias for 'git add' running the hook even when the worktree did not change. Similar state is not loaded by 'git version' or 'git typo'. Signed-off-by: Derrick Stolee --- t/t0401-post-command-hook.sh | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/t/t0401-post-command-hook.sh b/t/t0401-post-command-hook.sh index 0cc41e6210b86e..04dc2d2009355d 100755 --- a/t/t0401-post-command-hook.sh +++ b/t/t0401-post-command-hook.sh @@ -78,12 +78,42 @@ test_expect_success 'with post-index-change config' ' test_cmp expect post-index-change.out && test_path_is_missing post-command.out && + # add keeps the worktree the same, so does not run post-command. + # and this should work through an alias. + git config alias.addalias add && + rm -f post-command.out post-index-change.out && + echo more stuff >>file && + git addalias file && + test_cmp expect post-index-change.out && + + # TODO: This is the opposite of what we want! We want this to + # be missing, but the current state has this happening in this + # way. + test_path_exists post-command.out && + echo stuff >>file && # reset --hard updates the worktree. + # even through an alias + git config alias.resetalias "reset --hard" && rm -f post-command.out post-index-change.out && - git reset --hard && + git resetalias && test_cmp expect post-index-change.out && - test_cmp expect post-command.out + test_cmp expect post-command.out && + + # TODO: We want to skip the post-command hook here! + rm -f post-command.out && + test_must_fail git && # get help text + test_path_exists post-command.out && + + # TODO: We want to skip the post-command hook here! + rm -f post-command.out && + git version && + test_path_exists post-command.out && + + # TODO: We want to skip the post-command hook here! + rm -f post-command.out && + test_must_fail git typo && + test_path_exists post-command.out ' test_done From 27343297b1e7cb32c8420c9366fdf02c4c3a79cd Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 7 Nov 2025 14:18:22 -0500 Subject: [PATCH 2/2] hooks: better handle config without gitdir When the post-command hook runs, we could be in several custom states: 1. The command is a regular builtin, but the repo setup is incomplete. (Example: "git version", "git rev-parse HEAD") 2. The command is a dashed command, but the top level process uses a space in its call. (Example: "git http-request") 3. The command is an alias that goes to a builtin. 4. The command has no arguments or is only helptext. Each of these cases leads to a place where we previously had not loaded the postCommand.strategy config and would execute the post-command hook without looking for a sentinel file. There are two fixes here: First, we use early config parsing so we can get config details without fully setting up the repository. This is how core.hooksPath works in these situations. Second, we need to make sure handle_hook_replacement() is called even when the repo's gitdir is NULL. This requires a small amount of care to say that a sentinel file cannot exist if the gitdir isn't set, as we would not have written one without initializing the gitdir. This gets all of the desired behaviors we want for this setting without doing anything extreme with how pre- and post-command hooks run otherwise. Signed-off-by: Derrick Stolee --- hook.c | 34 ++++++++++++++++++++++++++++------ t/t0401-post-command-hook.sh | 15 ++++----------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/hook.c b/hook.c index be69f6a4527662..9adb7aba23df51 100644 --- a/hook.c +++ b/hook.c @@ -219,9 +219,15 @@ static int write_post_index_change_sentinel(struct repository *r) */ static int post_index_change_sentinel_exists(struct repository *r) { - char *path = get_post_index_change_sentinel_name(r); + char *path; int res = 1; + /* It can't exist if we don't have a gitdir. */ + if (!r->gitdir) + return 0; + + path = get_post_index_change_sentinel_name(r); + if (unlink(path)) { if (is_missing_file_error(errno)) res = 0; @@ -233,6 +239,21 @@ static int post_index_change_sentinel_exists(struct repository *r) return res; } +static int check_worktree_change(const char *key, const char *value, + UNUSED const struct config_context *ctx, + void *data) +{ + int *enabled = data; + + if (!strcmp(key, "postcommand.strategy") && + !strcasecmp(value, "worktree-change")) { + *enabled = 1; + return 1; + } + + return 0; +} + /** * See if we can replace the requested hook with an internal behavior. * Returns 0 if the real hook should run. Returns nonzero if we instead @@ -242,9 +263,11 @@ static int handle_hook_replacement(struct repository *r, const char *hook_name, struct strvec *args) { - const char *strval; - if (repo_config_get_string_tmp(r, "postcommand.strategy", &strval) || - strcasecmp(strval, "worktree-change")) + int enabled = 0; + + read_early_config(r, check_worktree_change, &enabled); + + if (!enabled) return 0; if (!strcmp(hook_name, "post-index-change")) { @@ -290,8 +313,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name, }; /* Interject hook behavior depending on strategy. */ - if (r && r->gitdir && - handle_hook_replacement(r, hook_name, &options->args)) + if (r && handle_hook_replacement(r, hook_name, &options->args)) return 0; hook_path = find_hook(r, hook_name); diff --git a/t/t0401-post-command-hook.sh b/t/t0401-post-command-hook.sh index 04dc2d2009355d..f1d4700e7e8abe 100755 --- a/t/t0401-post-command-hook.sh +++ b/t/t0401-post-command-hook.sh @@ -85,11 +85,7 @@ test_expect_success 'with post-index-change config' ' echo more stuff >>file && git addalias file && test_cmp expect post-index-change.out && - - # TODO: This is the opposite of what we want! We want this to - # be missing, but the current state has this happening in this - # way. - test_path_exists post-command.out && + test_path_is_missing post-command.out && echo stuff >>file && # reset --hard updates the worktree. @@ -100,20 +96,17 @@ test_expect_success 'with post-index-change config' ' test_cmp expect post-index-change.out && test_cmp expect post-command.out && - # TODO: We want to skip the post-command hook here! rm -f post-command.out && test_must_fail git && # get help text - test_path_exists post-command.out && + test_path_is_missing post-command.out && - # TODO: We want to skip the post-command hook here! rm -f post-command.out && git version && - test_path_exists post-command.out && + test_path_is_missing post-command.out && - # TODO: We want to skip the post-command hook here! rm -f post-command.out && test_must_fail git typo && - test_path_exists post-command.out + test_path_is_missing post-command.out ' test_done