Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

From: Stefan Beller <sbeller@google.com>
Date: Apr 12, 2018, 23:55 UTC
Message-ID: <CAGZ79kbnc17PZ9_=8QLkZgUZ0DHJKfWnxrekmgkLGFBU_0ieug@mail.gmail.com>
In-Reply-To: <20180412222047.5716-3-ao2@ao2.it>

Hi Antonio,

the subject line could also be
  fetch: fix custom submodule location
as I was not expecting this patch from the subject line.

On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:

Show 22 quoted lines

> Import the default git configuration before accessing the gitmodules
> file.
>
> This is important when a value different from the default one is set via
> the 'core.submodulesfile' config.
>
> Signed-off-by: Antonio Ospite <ao2@ao2.it>
> ---
>  builtin/fetch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index dcdfc66f0..d56636f74 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1428,8 +1428,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>         for (i = 1; i < argc; i++)
>                 strbuf_addf(&default_rla, " %s", argv[i]);
>
> -       config_from_gitmodules(gitmodules_fetch_config, NULL);
>         git_config(git_fetch_config, NULL);
> +       config_from_gitmodules(gitmodules_fetch_config, NULL);

Wouldn't this break the overwriting behavior? e.g. you configure a submodule URL in .gitmodules and then override it in .git/config, then we'd currently first load config from the modules file and then override it in git_config(git_fetch_config,..), whereas swapping them might have unintended consequences? Do the tests pass with this patch?