On 3/22/2019 10:53 AM, Ævar Arnfjörð Bjarmason wrote:
On Fri, Mar 22 2019, Jeff Hostetler wrote:
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.
Right, it also occurred to me that e.g. /home tends to be on NFS on some
systems, but very rarely /etc. I'd hate for trace reporting for git to
stall because NFS slows to a halt, so aside from temporary
implementation details (e.g. -c on the CLI not working) there's a good
case to be made for "read this from /etc/gitconfig only".
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.
I'd much rather just drop it in /etc/gitconfig with documented caveats
than introduce a new and permanent thing like /etc/gittrace2 due to a
current implementation detail.
Unlike something in core.* or whatever this facility is specialized
enough that I think it's fine to make it a bit of a special snowflake
given what it does and the target audience.
But even with those caveats it's still useful to see it in 'git config
-l --show-origin' for inspecting, and e.g. have it just work out of the
box with say the default puppetry for the likes of GitLab that now knows
how to set stuff in its /etc/gitconfig, but would need a special case
just for this.
ok, if we're comfortable with those caveats, i'll proceed as
you suggested. thanks for the quick response.
Jeff
