Re: [PATCH v2 1/4] git: read local config in --list-cmds

[Duy Nguyen <pclouds@gmail.com>]

On Mon, Mar 18, 2019 at 1:16 AM Todd Zullinger <tmz@pobox.com> wrote:
>
> From: Jeff King <peff@peff.net>
>
> Normally code that is checking config before we've decided to do
> setup_git_directory() would use read_early_config(), which uses
> discover_git_directory() to tentatively see if we're in a repo,
> and if so to add it to the config sequence.
>
> But list_cmds() uses the caching configset mechanism which
> rightly does not use read_early_config(), because it has no
> idea if it's being called early.
>
> Call setup_git_directory_gently() so we can pick up repo-level
> config (like completion.commands).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  git.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/git.c b/git.c
> index 2dd588674f..10e49d79f6 100644
> --- a/git.c
> +++ b/git.c
> @@ -62,6 +62,13 @@ static int list_cmds(const char *spec)
>  {
>         struct string_list list = STRING_LIST_INIT_DUP;
>         int i;
> +       int nongit;
> +
> +       /*
> +       * Set up the repository so we can pick up any repo-level config (like
> +       * completion.commands).
> +       */
> +       setup_git_directory_gently(&nongit);

This gave me a pause because we could try to find .git more than
necessary (e.g. when --list-cmds is requested without "config"). But I
don't think that happens often enough to be worried about.

It also subtly changes list_aliases() code flow, because the
read_early_config() call inside will use the already-discovered gitdir
 here instead of trying to rediscover. So, everything is still fine.

You probably want to drop the comment block about repository setup
inside list_cmds_by_config() too.

>
>         while (*spec) {
>                 const char *sep = strchrnul(spec, ',');
-- 
Duy