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

[Jeff Hostetler <git@jeffhostetler.com>]

On 6/23/22 5:38 AM, Teng Long wrote:
> On Wed, 22 Jun 2022 08:51:18 -0400, Jeff Hostetler wrote:
> 
[...]
> I remove the changes of "repo_config_bool" from my v4 patch
> then I try to know about "GIT_TRACE2_CONFIG_PARAMS" and
> "trace2.configparams". Thereby, when I:
> 
> Execute "GIT_TRACE2_PERF=1 GIT_TRACE2_CONFIG_PARAMS=core.multipackIndex git rev-list  --test-bitmap HEAD"
> 
> 15:21:38.812782 git.c:461                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
> 15:21:38.812797 git.c:461                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
> 
> I checked my configs, I found if there exists multiple level configs.
> it'll print multiple times. Like If I config all the global, system
> and local on "core.multipackIndex=false" , the output will be:
> 
> 15:41:50.614108 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
> 15:41:50.614123 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
> 15:41:50.614136 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
> 
> And if I modified the local scope of core.multipackIndex to "true",
> the output will be:
> 
> 15:45:39.200172 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
> 15:45:39.200186 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
> 15:45:39.200200 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:true
> 
> I'm not sure it's an intentional design or here should be only
> print the final value that takes effect or should print all the
> values if config multiple scopes on the same config.
> 
> Hence, I made a temporary patch below to try to add some
> identifying information to know why we output these lines, like:

I had intended it to only print the final "effective" value
that the command would actually use.  I don't remember if I
ever experimented with config values that are set at multiple
levels, but testing it now shows that you're right it does
print a "def_param" message for each level.

I would suggest we fix it to only print the final value
since we really just want to know whether the command will
or will not use a feature.  The inheritance is somewhat
irrelevant.

But the change to do that may be quite a mess, so printing
the scope name as you have here is fine.  And may help with
support when debugging users having confusing problems, so
I'm fine with it.

Thanks,
Jeff