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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 24, 2017, 00:51 UTC
Message-ID: <xmqq1sqab436.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79ka+va5bRNQL3x3fvn+738iAfJXJfD2PKpt2hmGEg09mTQ@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 20 quoted lines

>>>       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.