From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 27 Mar 2026 14:46:35 GMT
Subject: Re: [PATCH v5 09/12] hook: warn when hook.<friendly-name>.jobs is set
Message-ID: <acaYS-p1muabeUqX@pks.im>
In-Reply-To: <20260326101819.1307742-10-adrian.ratiu@collabora.com>

On Thu, Mar 26, 2026 at 12:18:16PM +0200, Adrian Ratiu wrote:
> diff --git a/hook.c b/hook.c
> index d98b011563..0493993bbe 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -279,6 +279,44 @@ void hook_cache_clear(struct strmap *cache)
>  	strmap_clear(cache, 0);
>  }
>  
> +/*
> + * Return true if `name` is a hook friendly-name, i.e. it has at least one of
> + * .command, .event, or .parallel configured. These are the reliable clues
> + * that distinguish a friendly-name from an event name. Note: .enabled is
> + * deliberately excluded because it can appear under both namespaces.
> + */
> +static int is_friendly_name(struct hook_all_config_cb *cb, const char *name)
> +{
> +	struct hashmap_iter iter;
> +	struct strmap_entry *e;
> +
> +	if (strmap_get(&cb->commands, name) || strmap_get(&cb->parallel_hooks, name))
> +		return 1;
> +
> +	strmap_for_each_entry(&cb->event_hooks, &iter, e) {
> +		if (unsorted_string_list_lookup(e->value, name))
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Warn if any name in event_jobs is also a hook friendly-name. */
> +static void warn_jobs_on_friendly_names(struct hook_all_config_cb *cb_data)
> +{
> +	struct hashmap_iter iter;
> +	struct strmap_entry *e;
> +
> +	strmap_for_each_entry(&cb_data->event_jobs, &iter, e) {
> +		if (is_friendly_name(cb_data, e->key))
> +			warning(_("hook.%s.jobs is set but '%s' looks like a "
> +				  "hook friendly-name, not an event name; "
> +				  "hook.<event>.jobs uses the event name "
> +				  "(e.g. hook.post-receive.jobs), so this "
> +				  "setting will be ignored"), e->key, e->key);
> +	}
> +}

Makes sense. The bigger question of course is whether we should properly
separate those namespaces, so that this confusion cannot even happen in
the first place. I won't push for such a change though.

Patrick