On Thu, Feb 20, 2025 at 9:13 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> writes:
>
Hi Junio.
> > @@ -23,7 +22,7 @@ static const char * const verify_tag_usage[] = {
> > int cmd_verify_tag(int argc,
> > const char **argv,
> > const char *prefix,
> > - struct repository *repo UNUSED)
> > + struct repository *repo)
> > {
> > int i = 1, verbose = 0, had_error = 0;
> > unsigned flags = 0;
> > @@ -50,13 +49,13 @@ int cmd_verify_tag(int argc,
> > flags |= GPG_VERIFY_OMIT_STATUS;
> > }
> >
> > - git_config(git_default_config, NULL);
> > + repo_config(repo, git_default_config, NULL);
>
> I seriously think that it is a horrible idea (but the previous step
> of this series is hardly the first one that commits the same sin) to
> move git_config() down only to deal with "repo might be NULL if run
> outside a repository". We should stop making such changes, and we
> should revert the changes we already made along that line, to solve
> it differently.
>
Yeah, I agree with this after going through your comment on the other
patch. We should look for a better solution.
> Wouldn't it work much better if we teach repo_config() to allow repo
> to be NULL to signal that we are outside any repository, and behave
> the same way the current git_config() works when called outside a
> repository? Even though the function is called repo_config(), it is
> *NOT* limited to read from $GIT_DIR/config but does read from the
> usual "repository configuration trumps per-user configuration which
> trumps system-side configuration" cascade, so it is natural to skip
> the repository configuration when called outside any repository but
> read the other configuration sources, which should be what happens
> when git_config() is called from outside the repository, no?
Yeah, I was studying the config.c and config.h files to understand better
how all these functions work.
The git_config() when called outside the repository, uses the global
the_repository
variable basically called the repo_config(). It does not necessarily
handle any situation
when the repo was NULL. It always uses the global the_repository variable.
I do not think we want to handle the repo_config() the same as the point of
all these are to reduce/remove the use of the_repository global variable.
While going through the config.c I saw the read_very_early_config()
which read the config
from the system and global settings and does not require any repo
variable. I think, to teach
the repo_config() to allow NULL value, we could call the
read_very_early_config() whenever
the repo is NULL as we know, this happens outside the repository. I
sent a rfc patch for this
and it can viewed here :-
https://public-inbox.org/git/20250227175456.1129840-1-usmanakinyemi202@xxxxxxxxx/T/#u
Another approach which I was thinking about is having a local
repository variable inside
the repo_config() whenever the repo variable passed to it is NULL.
What do you think ?
Thanks.
>
