Re: [WIP v2 2/2] config: include file if remote URL matches a glob

To expand a little more on this:

[snip]

The nice thing about 'hasRemoteUrl' in this case is that we don't need
to know the location of the user's big-oss-project/ checkout on disk. We
can set that config globally and they can checkout big-oss-project as
many times and as many places as they wish. It wouldn't be possible to
ship configs via a package manager or other automated script without it.

Ah, thanks for the elaboration!

quoted

NEEDSWORK: The way this works is that if we see such an include, we
shunt all subsequent configs into a stash (while looking for URLs), then
process the stash. In particular, this means that more memory is needed,
and the nature of error reporting changes (currently, if a callback
returns nonzero for a variable, processing halts immediately, but with
this patch, all the config might be read from disk before the callback
even sees the variable). I'll need to expand on this and write a
documentation section.

Hm. I'm not so sure about making another structure for storing config
into memory, because we already do that during the regular config parse
(to make things like git_config_get_string() fast). Can we not re-walk
the in-memory config at the end of the normal parse, rather than reading
from disk twice?

I think git_config()/repo_config() callback even does that for you for free...?

The main thing is that we wouldn't know if an entry would have been
overridden by a value from an includeif.hasremoteurl or not.

What's the easiest way to "try it and see", to add tooling and find out
whether the config files would be reopened during the second parse?
Because I suspect that we won't actually reopen those files, due to the
config cache.

As Ævar says, strace should work. The hard part is implementing the
recursive config parse, but it looks like the way to go, so I'll try it
and see how it goes.

[1] https://lore.kernel.org/git/211106.8635o9hogz.gmgdl@evledraar.gmail.com/ (local)

So couldn't we do something like....

pass #1:
 if (include)
   if (not hasRemoteUrl)
     open up path & parse
 put config into in-memory cache normally
pass #2: (and this pass would need to be added to repo_config() probably)
 if (include)
   if (hasRemoteUrl)
     open up path & parse
     insert in-order into in-memory cache
 don't touch existing configs otherwise

I think it's in practice similar to the approach you're using (getting
around the weird ordering with a cache in memory), but we could reuse
the existing config cache rather than creating a new and different one.

What do you mean by "insert in-order"? If you mean figuring out which
variables would be overridden (and for multi-valued variables, what
order to put all the values in), I think that's the hard part.

Another thing is that at the point where we read the config
(config_with_options()), we have a callback, so we would need to make
sure that we're writing to the in-memory cache in the first place (as
opposed to passing a callback that does something else). That might be
doable by changing the API, but in ay case, I'll try the recursive
config parse first.