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

Jonathan Tan [off-list ref] writes:

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 <redacted>
---
 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.

Junio C Hamano [off-list ref] writes:

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.

I think that the hard part of this is how to present this to the end
user - right now, we just have one pattern of variable (remote.*.url,
where "*" is a wildcard) and one pattern of value with specific
properties (e.g. this is a glob, not a regular expression, and the
special value "**" is supported).

Once we figure that out, I would think that we could implement it in a
way similar to this patch. As for whether we should wait until the full
feature before merging any code that does includeIf based on a variable
(in order to avoid having code that would quickly be replaced by other
code), in this case, unless there is another use case for this, I think
we should proceed with the use case that we know about first
(conditional include of a file supplied by a remote repo administrator).