On 3/22/2019 9:17 AM, Johannes Schindelin wrote:
Hi Ævar,
On Thu, 21 Mar 2019, Ævar Arnfjörð Bjarmason wrote:
On Thu, Mar 21 2019, Johannes Schindelin wrote:
On Sun, 17 Mar 2019, Ævar Arnfjörð Bjarmason wrote:
On Fri, Feb 15 2019, Jeff Hostetler wrote:
I could see having a "~/.git_tr2_config" or something similar in
some place like "/etc" that only contained the Trace2 settings. It
would be safe to read very early inside main() and we would not
consider it to be part of the real config. For example, "git
config" would not know about it. Then you could enforce a
system-wide setting without any of the env var issues.
I haven't written a patch for this, but it seems to me that we could
just start reading /etc/gitconfig via some custom config callback
code early as e.g. 58b284a2e91 ("worktree: add per-worktree config
files", 2018-10-21) does for the worktree config.
Oy. Oy, oy, oy.
Maybe use `read_early_config()` instead? That would be *a lot*
cleaner. You could maybe use a9bcf6586d1a (alias: use the early config
machinery to expand aliases, 2017-06-14) as an inspiration.
Thanks. I was thinking *only* to do /etc/gitconfig and not the whole
.git/config -> ~/.gitconfig etc. sequence just in terms of saving
critical time (this is the performance trace path, after all...).
But on a second reading I see that read_early_config() can do that if
you set config_source->file, opts->respect_includes etc. I.e. it just
(depending on options) resolves to git_config_from_file() which
58b284a2e91 used directly.
Sure, it can exclude the repo and user config, but would that not be
rather confusing?
This was hidden in my earlier message.
There's a lot a config machinery here with lots of chicken-n-egg
problems. I want to know at the top of main() as quickly as possible
whether trace2 should be enabled.
I don't want to slow down git by spinning up a bunch of trace2 state
and wait until the git-dir is discovered, the "-c" args are processed,
and we dispatch into the builtin layer and the config is enumerated
to know if it should really be on or not.
I also didn't want to introduce another full iteration of the full
config system for startup performance reasons.
I played with read_early_config() at one point and it always seemed
to introduce side-effects. Perhaps I was calling it earlier than it
was expecting and that triggered some of the git-dir discovery or
something. I don't remember all the details, I just remember that it
changed some behaviors in subtle ways.
Perhaps I could call something like git_config_from_file() with the
right set of magic bits to get it to parse exactly 1 system config
file that would contain my trace2 settings. Hopefully, this will
not have any side-effects.
But if we lump them in with the main /etc/gitconfig settings, we
would have to explain that these trace2 config settings are
system-only and ARE NOT overridden by "-c", global, local, ...
config settings. This would get confusing if the user tried to
set local values and did:
git config --list --show-origin
and it showed system and local values but yet "stubbornly" refused
to use the local values over the system values. (I think this was
Johannes' point.)
That's why I was suggesting a system trace2 config file that is a
peer of /etc/gitconfig (maybe /etc/gittrace2) that would have these
values and not be expected to interact with the main config system.
That is, we just use the git_config_ routines to parse the file format,
rather than inventing another file format, but not change the
expectation of the established config value inheritance system.
If there's no objections, I'll take a look at doing this.
Thanks
Jeff
