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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 12, 2021, 23:48 UTC
Message-ID: <xmqqk0ih24a0.fsf@gitster.g>
In-Reply-To: <967680d27aa7a2709e528ff989a9dd534886efba.1634077795.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 7 quoted lines

> I marked this as RFC because there are some design points that need to
> be resolved:
>
>  - The existing "include" and "includeIf" instructions are executed
>    immediately, whereas in order to be useful, the execution of
>    "includeIf hasremoteurl" needs to be delayed until all config files
>    are read. Are there better ways to do this?

An interesting chicken-and-egg problem. Even if an included configuration file does not have further "include", you may discover there are more remotes, which may add new includes to fire from the top-level configuration file.

What if we have multiple remotes? Is it a sufficient match for only one of them match what is mentioned in the includeIf condition? Should all of them must match the pattern instead? Majority, perhaps? Something else?

Show 8 quoted lines

>  - Is the conditionally-included file allowed to have its own
>    "include{,If}" instructions? I'm thinking that we should forbid it
>    because, for example, if we had 4 files as follows: A includes B and
>    C includes D, and we include A and C in our main config (in that
>    order), it wouldn't be clear whether B (because A was first included)
>    or C (because we should execute everything at the same depth first)
>    should be executed first. (In this patch, I didn't do anything about
>    includes.)

Interesting. The order of real inclusion obviously would affect the outcome of the "last one wins" rule. And this does not have to be limited to this "hasremote" condition, so we need to design it with a bit of care.

Would it be possible for a newly included file to invalidate an earlier condition that was used to decide whether to include another file or not? If not, then you can take a two-pass approach where the first pass is used to decide solely to discover which conditionally included files are taken, clear the slate and the parse these files in the textual order. In the case of your example above, the early part of the primary config would be the first to be read, then comes A's early part, then comes B in its entirety, then the rest of A, and then the middle part of the primary config, then C's early part, then D, and then the rest of C,... you got the idea.

If it is possible for an included file to invalidate a condition we have already evaluated to make a decision, it would become messy. For example, we might decide to include another file based on the value we discovered for a config variable:

    === .git/config ===
    [my] variable
    [your] variable = false

    [includeIf "configEq:my.variable==true"]
            file = fileA

but the included file may override the condition, e.g.

    === fileA ===
    [my] variable = false
    [your] variable = true

and applying the "last one wins" rule becomes messy. I do not offhand know what these

    $ git config --bool my.variable
    $ git config --bool your.variable

should say, and do not have a good explanation for possible outcomes.

Maybe the above example can serve as a way to guide us when we design the types of conditionals we allow in includeIf. This example tells us that it is probably a terrible idea to allow using values of configuration variables as part of "includeIf" condition.

There may lead to similar "'hasremoteurl' makes a well-behaved condition, because [remote] are additive and not 'last-one-wins', but we cannot add 'lacksremoteurl' as a condition, because a file we decide to include based on a 'lacks' predicate may invalidate the 'lacks' condition by defining such a remote" design decisions you'd need to make around the URLs of the remotes defined for the repository.