Subject: Re: [PATCH 2/3] builtin/fetch: parse recurse-submodules-default at default options parsing

From: Junio C Hamano <[email-masked]>
Date: Fri, 23 Jun 2017 17:51:25 -0700

Stefan Beller <[email-masked]> writes:

>>>       if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
>>> -             if (recurse_submodules_default) {
>>> -                     int arg = parse_fetch_recurse_submodules_arg("--recurse-submodules-default", recurse_submodules_default);
>>> -                     set_config_fetch_recurse_submodules(arg);
>>> -             }
>>> +             if (recurse_submodules_default != RECURSE_SUBMODULES_DEFAULT)
>>> +                     set_config_fetch_recurse_submodules(recurse_submodules_default);
>>
>> This is not a new thing, and it may not even be a problem, but I
>> have to wonder why this needs to be done conditionally.  "The
>> ...
>
> As far as I suspect, the original author considered
> evaluating the additional config too expensive.
>
>     gitmodules_config(); // <- this specifically?
>     git_config(submodule_config, NULL);
>
> And that is why we only react if any switch is
> given to recurse. 

I am not talking about the outer "if" condition.  

The inner 

    "if (recurse_submodules_default != RECURSE_SUBMODULES_DEFAULT)"

block, is what I am questioning, i.e. I am wondering why
set_config_fetch_recurse_submodules() need to be conditionally
called.

The two statement you quoted above will be executed either way,
regardless of the value of recurse_submodule_default.

Unless --recurse-submodules=off is given, in which case the outer
"if" condition rejects.  And I think you are explaining that part,
but that was not what I am questioning.