Re: [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 22, 2022, 15:12 UTC
Message-ID: <xmqqfsjw7mwe.fsf@gitster.g>
In-Reply-To: <4176c5af-43fd-b7a1-346c-4d4aa9dbd4ae@jeffhostetler.com>

Jeff Hostetler <git@jeffhostetler.com> writes:

Show 7 quoted lines

> (I just sent a response to your V2 before I saw your V3, so I'll
> my response here so that it doesn't get lost.)
>
>
> We should not be doing this.  This would dump every repo-related
> boolean value on every command.  I see that in V3 that you have
> a "trace" flag to control this.  But again, this seems wrong here.

I noticed that "flag" while merging this with another topic to "seen" and found it strange, too. It "allows" the caller to choose which one gets logged per variable, but in a very hard-coded way; we probably would have been much better off to have a table of what gets logged and have repo_cfg_*() calls consult it. Even without configurability, at least it would move the hard-coded choice of what gets logged from code to data.

But ...

> I already have a GIT_TRACE2_CONFIG_PARAMS and trace2.configparams
> that will dump "interesting" config values to the trace2 log.
> Just set one of them to a list of regex's.  Look at the comment above
> trace2_cmd_list_config() in trace2.h for details.

... that does sound like the right way to go.

Show 9 quoted lines

> ...
> So if I'm reading this right, this will only print the value of
> "core.multipackindex".
>
> You can get this without the changes here using
>
> GIT_TRACE2_CONFIG_PARAMS="core.multipackindex,...any_other_values_of_interest..."
> before running the command (or use the config setting) before running
> your commandss.

Thanks.