Re: [PATCH v6 00/15] Trace2 tracing facility

> 
> 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.
>