Re: [PATCH v3 2/2] config: include file if remote URL matches a glob

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 23, 2021, 01:22 UTC
Message-ID: <xmqqtug3aco7.fsf@gitster.g>
In-Reply-To: <1c1a07a0b65d4bbbb0f2628a3ddf1980e37d5065.1637020610.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 25 quoted lines

> This is a feature that supports config file inclusion conditional on
> whether the repo has a remote with a URL that matches a glob.
>
> Similar to my previous work on remote-suggested hooks [1], the main
> motivation is to allow remote repo administrators to provide recommended
> configs in a way that can be consumed more easily (e.g. through a
> package installable by a package manager - it could, for example,
> contain a file to be included conditionally and a post-install script
> that adds the include directive to the system-wide config file).
>
> In order to do this, Git reruns the config parsing mechanism upon
> noticing the first URL-conditional include in order to find all remote
> URLs, and these remote URLs are then used to determine if that first and
> all subsequent includes are executed. Remote URLs are not allowed to be
> configued in any URL-conditionally-included file.
>
> [1] https://lore.kernel.org/git/cover.1623881977.git.jonathantanmy@google.com/
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  Documentation/config.txt |  11 ++++
>  config.c                 | 121 ++++++++++++++++++++++++++++++++++++---
>  config.h                 |   7 +++
>  t/t1300-config.sh        | 100 ++++++++++++++++++++++++++++++++
>  4 files changed, 231 insertions(+), 8 deletions(-)

Here is just a design level comment, without trying to outline the implementation in my head like I usually do before making any suggestion, but it strikes me somewhat sad that config.c needs to know specifically about "remote_url".

I wonder if this can be a more generalized framework that allows us to say "we introduce a new [includeIf] variant to get another file included only if some condition is met for the configuration variables we read without the includeIf directive", with variations of "condition" including

 - a literal X is among the values of multi-valued variable Y.
 - a pattern X matches one of the values of multi-valued variable Y.
 - a literal Y is the name of an existing configuration variable.
 - a pattern Y matches the name of an existing configuration variable.

If that is done, I would imagine that the feature can become a thin specialization e.g. "there is an existing configuration variable whose name is 'remotes.https://github.com/janathantanmy/git.url'"

Perhaps I am dreaming?

Thanks.