On Tue, Jun 28 2022, Teng Long wrote:
> We can use GIT_TRACE2_CONFIG_PARAMS and trace2.configparams to
> dump the config which we are inteseted in to the tr2 log. If
> an "interesting" config exists in multiple scopes, it will be
> dumped multiple times. So, let's fix it to only print the
> final value instead.
>
> Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx>
> Helped-by: Jeff Hostetler <git@xxxxxxxxxxxxxxxxx>
> ---
> trace2/tr2_cfg.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/trace2/tr2_cfg.c b/trace2/tr2_cfg.c
> index ec9ac1a6ef..632bb6feec 100644
> --- a/trace2/tr2_cfg.c
> +++ b/trace2/tr2_cfg.c
> @@ -1,5 +1,6 @@
> #include "cache.h"
> #include "config.h"
> +#include "strmap.h"
> #include "trace2/tr2_cfg.h"
> #include "trace2/tr2_sysenv.h"
>
> @@ -10,6 +11,7 @@ static int tr2_cfg_loaded;
> static struct strbuf **tr2_cfg_env_vars;
> static int tr2_cfg_env_vars_count;
> static int tr2_cfg_env_vars_loaded;
> +static struct strset tr_cfg_set = STRSET_INIT;
>
> /*
> * Parse a string containing a comma-delimited list of config keys
> @@ -101,12 +103,17 @@ static int tr2_cfg_cb(const char *key, const char *value, void *d)
> {
> struct strbuf **s;
> struct tr2_cfg_data *data = (struct tr2_cfg_data *)d;
> + const char *prior_value;
>
> for (s = tr2_cfg_patterns; *s; s++) {
> struct strbuf *buf = *s;
> int wm = wildmatch(buf->buf, key, WM_CASEFOLD);
> if (wm == WM_MATCH) {
> - trace2_def_param_fl(data->file, data->line, key, value);
> + if (strset_contains(&tr_cfg_set, key)
> + || git_config_get_value(key, &prior_value))
> + continue;
> + trace2_def_param_fl(data->file, data->line, key, prior_value);
> + strset_add(&tr_cfg_set, key);
> return 0;
> }
> }
Is tr2_cfg_load_patterns() run at early startup, but this perhaps at
runtime? I wonder if this is OK under threading, i.e. concurrent access
to your strset.
The assumption you're making here doesn't hold in general, some config
is "last vaule wins", but for some other config all configured values
are important.
Do we care in this case? I really don't know, but perhaps we can declare
"dedup" using the same facility we're using to wildcard-match keys, and
either make that optional or the default, e.g.:
GIT_TRACE2_CONFIG_PARAMS='*:dedup,core.*:'
I.e. to make it a list of <glob>[:<options>].
Maybe not worth the effort...
