Jonathan Tan <jonathantanmy@xxxxxxxxxx> 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 <chooglen@xxxxxxxxxx>
