Re: [PATCH v7 0/2] Conditional config includes based on remote URL

Jonathan Tan [off-list ref] writes:

Thanks, everyone, for your comments. I've followed Glen's code
suggestion and Junio's documentation suggestion, as you can see in the
range-diff.

Jonathan Tan (2):
  config: make git_config_include() static
  config: include file if remote URL matches a glob

 Documentation/config.txt |  27 ++++++++
 config.c                 | 132 ++++++++++++++++++++++++++++++++++++---
 config.h                 |  46 ++++----------
 t/t1300-config.sh        | 118 ++++++++++++++++++++++++++++++++++
 4 files changed, 282 insertions(+), 41 deletions(-)

Range-diff against v6:
1:  b2dcae03ed = 1:  b2dcae03ed config: make git_config_include() static
2:  de2be06818 ! 2:  7c70089074 config: include file if remote URL matches a glob
    @@ Documentation/config.txt: all branches that begin with `foo/`. This is useful if
     +Files included by this option (directly or indirectly) are not allowed
     +to contain remote URLs.
     ++
    -+This keyword is designed to be forwards compatible with a naming
    -+scheme that supports more variable-based include conditions, but
    -+currently Git only supports the exact keyword described above.
    ++Note that unlike other includeIf conditions, resolving this condition
    ++relies on information that is not yet known at the point of reading the
    ++condition. A typical use case is this option being present as a
    ++system-level or global-level config, and the remote URL being in a
    ++local-level config; hence the need to scan ahead when resolving this
    ++condition. In order to avoid the chicken-and-egg problem in which
    ++potentially-included files can affect whether such files are potentially
    ++included, Git breaks the cycle by prohibiting these files from affecting
    ++the resolution of these conditions (thus, prohibiting them from
    ++declaring remote URLs).
    +++
    ++As for the naming of this keyword, it is for forwards compatibiliy with
    ++a naming scheme that supports more variable-based include conditions,
    ++but currently Git only supports the exact keyword described above.
     +
      A few more notes on matching via `gitdir` and `gitdir/i`:
      
    @@ config.c: static int git_config_include(const char *var, const char *value, void
     +		if (inc->opts->unconditional_remote_url)
     +			inc->fn = forbid_remote_url;
      		ret = handle_path_include(value, inc);
    -+		if (inc->opts->unconditional_remote_url)
    -+			inc->fn = old_fn;
    ++		inc->fn = old_fn;
     +	}
      
      	return ret;
-- 
2.34.1.173.g76aa8bc2d0-goog

The implementation looks good, and I think that the precedent we are
setting with "hasconfig:" is pretty well captured on this thread. 

This looks good to me, though I'm not an expert in this area, so it
would be good for others to chime in.

Reviewed-by: Glen Choo <redacted>