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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:

> 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?

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





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux