Re: [PATCH v2 4/8] hook: include hooks from the config

[Junio C Hamano <gitster@pobox.com>]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> Examples:
>
>   $ git config --get-regexp "^hook\."
>   hook.bar.command=~/bar.sh
>   hook.bar.event=pre-commit

This is all good when you know where you defined your pre-commit
hook, but you would want to know in which scope the configuration is
made, wouldn't you, especially when you are trying to diagnose why
some command that you do not necessarily recognise when you run a
Git command?

> @@ -10,17 +11,22 @@ struct repository;
>   * Represents a hook command to be run.
>   * Hooks can be:
>   * 1. "traditional" (found in the hooks directory)
> - * 2. "configured" (defined in Git's configuration, not yet implemented).
> + * 2. "configured" (defined in Git's configuration via hook.<name>.event).

Wouldn't it be easier to understand if we do "<name>" -> "<friendly-name>"
to match the member name used in the struct below?

>   * The 'kind' field determines which part of the union 'u' is valid.
>   */
>  struct hook {
>  	enum {
>  		HOOK_TRADITIONAL,
> +		HOOK_CONFIGURED,
>  	} kind;
>  	union {
>  		struct {
>  			const char *path;
>  		} traditional;
> +		struct {
> +			const char *friendly_name;
> +			const char *command;

If we wanted to report which config scope defined a particular hook
we need to record where the configured hook came from in this
struct, right?

> +		} configured;
>  	} u;

> +	if (repo->hook_config_cache) {
> +		hook_cache_clear(repo->hook_config_cache);
> +		FREE_AND_NULL(repo->hook_config_cache);
> +	}

It is a minor point, but after applying the hole series, there are
only two calls to hook_cachje_clear(X) and they both are followed by
free(X) or FREE_AND_NULL(X).  I wonder if we simply want
hook_cache_free() instead of _clear()?