Junio C Hamano <gitster@xxxxxxxxx> writes:
> > @@ -335,9 +412,29 @@ static int git_config_include(const char *var, const char *value, void *data)
> > ret = handle_path_include(value, inc);
> >
> > if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
> > - (cond && include_condition_is_true(inc->opts, cond, cond_len)) &&
> > - !strcmp(key, "path"))
> > - ret = handle_path_include(value, inc);
> > + cond && !strcmp(key, "path")) {
> > + const char *url;
> > + size_t url_len;
> > +
> > + if (skip_prefix_mem(cond, cond_len, "hasremoteurl:", &url,
> > + &url_len)) {
> > + if (inc->opts->unconditional_remote_url) {
> > + config_fn_t old_fn = inc->fn;
> > +
> > + inc->fn = forbid_remote_url;
> > + ret = handle_path_include(value, inc);
> > + inc->fn = old_fn;
> > + } else {
> > + if (!inc->remote_urls)
> > + populate_remote_urls(inc);
> > + if (at_least_one_url_matches_glob(
> > + url, url_len, inc->remote_urls))
> > + ret = handle_path_include(value, inc);
> > + }
> > + } else if (include_condition_is_true(inc->opts, cond, cond_len)) {
> > + ret = handle_path_include(value, inc);
> > + }
>
> This looks iffy, especialy in a patch that is not marked as [RFC].
>
> I can see that include_condition_is_true() only passes inc->opts and
> you need some other parts of inc for your purpose, and it may be the
> primary reason why you munge this caller instead of adding function
> include_by_remoteurl() and making include_condition_is_true() call
> it. But wouldn't it be sufficient to pass inc (not inc->opts) to
> include_condition_is_true(), and have it dereference inc->opts when
> calling include_by_gitdir() and friends that want opts, while
> passing inc to include_by_remoteurl()?
Thanks for taking a look. I think the primary reason why I wrote it like
this was because originally (in v1) I included at the end, not inline,
but of course that is no longer the case so I should revisit it. I'll
have to pay attention to how inc->fn is swapped for forbid_remote_url
(to prevent remote URLs from being configured in included files), but
perhaps handle_path_include() can do that. I'll take a look.
