Skip to content

Commit bba9292

Browse files
derrickstoleedscho
authored andcommitted
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 <stolee@gmail.com>
1 parent 7731b44 commit bba9292

File tree

2 files changed

+32
-17
lines changed

2 files changed

+32
-17
lines changed

hook.c

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,15 @@ static int write_post_index_change_sentinel(struct repository *r)
219219
*/
220220
static int post_index_change_sentinel_exists(struct repository *r)
221221
{
222-
char *path = get_post_index_change_sentinel_name(r);
222+
char *path;
223223
int res = 1;
224224

225+
/* It can't exist if we don't have a gitdir. */
226+
if (!r->gitdir)
227+
return 0;
228+
229+
path = get_post_index_change_sentinel_name(r);
230+
225231
if (unlink(path)) {
226232
if (is_missing_file_error(errno))
227233
res = 0;
@@ -233,6 +239,21 @@ static int post_index_change_sentinel_exists(struct repository *r)
233239
return res;
234240
}
235241

242+
static int check_worktree_change(const char *key, const char *value,
243+
UNUSED const struct config_context *ctx,
244+
void *data)
245+
{
246+
int *enabled = data;
247+
248+
if (!strcmp(key, "postcommand.strategy") &&
249+
!strcasecmp(value, "worktree-change")) {
250+
*enabled = 1;
251+
return 1;
252+
}
253+
254+
return 0;
255+
}
256+
236257
/**
237258
* See if we can replace the requested hook with an internal behavior.
238259
* 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,
242263
const char *hook_name,
243264
struct strvec *args)
244265
{
245-
const char *strval;
246-
if (repo_config_get_string_tmp(r, "postcommand.strategy", &strval) ||
247-
strcasecmp(strval, "worktree-change"))
266+
int enabled = 0;
267+
268+
read_early_config(r, check_worktree_change, &enabled);
269+
270+
if (!enabled)
248271
return 0;
249272

250273
if (!strcmp(hook_name, "post-index-change")) {
@@ -290,8 +313,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
290313
};
291314

292315
/* Interject hook behavior depending on strategy. */
293-
if (r && r->gitdir &&
294-
handle_hook_replacement(r, hook_name, &options->args))
316+
if (r && handle_hook_replacement(r, hook_name, &options->args))
295317
return 0;
296318

297319
hook_path = find_hook(r, hook_name);

t/t0401-post-command-hook.sh

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,7 @@ test_expect_success 'with post-index-change config' '
8585
echo more stuff >>file &&
8686
git addalias file &&
8787
test_cmp expect post-index-change.out &&
88-
89-
# TODO: This is the opposite of what we want! We want this to
90-
# be missing, but the current state has this happening in this
91-
# way.
92-
test_path_exists post-command.out &&
88+
test_path_is_missing post-command.out &&
9389
9490
echo stuff >>file &&
9591
# reset --hard updates the worktree.
@@ -100,20 +96,17 @@ test_expect_success 'with post-index-change config' '
10096
test_cmp expect post-index-change.out &&
10197
test_cmp expect post-command.out &&
10298
103-
# TODO: We want to skip the post-command hook here!
10499
rm -f post-command.out &&
105100
test_must_fail git && # get help text
106-
test_path_exists post-command.out &&
101+
test_path_is_missing post-command.out &&
107102
108-
# TODO: We want to skip the post-command hook here!
109103
rm -f post-command.out &&
110104
git version &&
111-
test_path_exists post-command.out &&
105+
test_path_is_missing post-command.out &&
112106
113-
# TODO: We want to skip the post-command hook here!
114107
rm -f post-command.out &&
115108
test_must_fail git typo &&
116-
test_path_exists post-command.out
109+
test_path_is_missing post-command.out
117110
'
118111

119112
test_done

0 commit comments

Comments
 (0)