Re: [PATCH v3 1/8] config: teach repo_config to allow `repo` to be NULL

[Usman Akinyemi <usmanakinyemi202@gmail.com>]

On Thu, Mar 6, 2025 at 11:23 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Usman Akinyemi <usmanakinyemi202@gmail.com> writes:
>
> >  void repo_config(struct repository *repo, config_fn_t fn, void *data)
> >  {
> > +     if (!repo) {
> > +             read_very_early_config(fn, data);
> > +             return;
> > +     }
> >       git_config_check_init(repo);
> >       configset_iter(repo->config, fn, data);
> >  }
> > diff --git a/config.h b/config.h
> > index 5c730c4f89..1e5b22dfc4 100644
> > --- a/config.h
> > +++ b/config.h
> > @@ -219,6 +219,9 @@ void read_very_early_config(config_fn_t cb, void *data);
> >   * repo-specific one; by overwriting, the higher-priority repo-specific
> >   * value is left at the end).
> >   *
> > + * In cases where the repository variable is NULL, repo_config() will
> > + * call read_early_config().
> > + *
>
> early or very early?
>
> I am wondering if we should describe the effect we want out of the
> design more prominently than the way we try to obtain the effect
> here.  In other words, instead of (rather, in addition to) saying
> that we call helper X, wouldn't it be more helpful to future
> developers why we call X, to convey the intent, so that they know
> how to adjust when for example what X does change or X even
> disappears?  E.g.,
>
>         When repo==NULL, skip reading the per-repository
>         configuration file but still use the system- and globa-
>         configuration, by calling X.  Note that this ignores
>         one-time configuration override "git -c var=val" given from
>         the command line.  The only use case the feature to allow
>         passing repo==NULL was designed for is to support handling
>         "git foo -h" (which lets git.c:run_builtin() to pass NULL
>         and have the cmd_foo() call repo_config() before calling
>         parse_options() to notice "-h", give help and exit) for a
>         command that ordinarily require a repository, so this
>         limitation may be OK (but if needed you are welcome to fix
>         it).
>
> That way, folks who are planning to update read_veriy_early_config()
> so that it pays attention to the "git -c var=val" in the future will
> be rest assured that they won't be breaking this caller with their
> planned change.
>
> Of course I didn't spend enough brainpower to make the above comment
> more concise and to the point, which the final version should be,
> but hopefully you got the idea.
Yeah, thanks Junio, I understand clearly. Your comment is well explained also.
I will refine it and add it in an updated patch.
Thank you.
>
> Thanks.
>