Re: [PATCH 1/1] builtin/receive-pack: avoid spinning no-op sideband async threads

[Jeff King <peff@peff.net>]

On Mon, Mar 02, 2026 at 09:17:04PM +0200, Adrian Ratiu wrote:

> It is important to call the hook_exists() API provided by hook.[ch]
> because it covers both config-defined hooks and the "traditional"
> hooks from the hookdir. find_hook() only covers the hookdir hooks.

Ah, OK. Traditionally hook_exists() was just a thin wrapper over
find_hook(), but it looks like that changed in your series. But either
way, it much more clearly expresses the intent to use hook_exists().

So this obviously looks good, but just some random thoughts below.

> @@ -934,6 +934,9 @@ static int run_receive_hook(struct command *commands,
>  	int saved_stderr = -1;
>  	int ret;
>  
> +	if (!hook_exists(the_repository, hook_name))
> +		return 0;

It is a little inelegant that we have to look up the hook data
separately ourselves here, and then it will be done again in
run_hooks_opt(). But I don't think there is an easy way to reorganize
it, short of something like:

  struct hook myhook = HOOK_INIT;

  load_hooks(&myhook, hook_name);
  if (myhook.nr)
	return 0; /* no hooks of this type */

  ...other prep work...

  run_hooks_opt(repo, &myhook, &opt);

I doubt that is worth it, as the lookup process should not be too
expensive. It looks like we cache the config parts of the lookup
already. We call access() to find the traditional hooks on each lookup,
but that is also true of the code before your series.

It would not matter at all for pre-receive, for example, but for
something like update, I guess we are doing a bunch of pointless
access() calls that could be cached. But again, not new in your series,
and nobody has really noticed. So we can either treat it as an
optimization for later, or just leave it be forever.

-Peff