On Tue, Mar 7, 2017 at 5:14 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> submodule_from_ce returns always NULL, when such flag is not given.
>> From 10/18:
>>
>> +const struct submodule *submodule_from_ce(const struct cache_entry *ce)
>> +{
>> + if (!S_ISGITLINK(ce->ce_mode))
>> + return NULL;
>> +
>> + if (!should_update_submodules())
>> + return NULL;
>> +
>> + return submodule_from_path(null_sha1, ce->name);
>> +}
>>
>> should_update_submodules is always false if such a flag is not set,
>> such that we end up in the else case which is literally the same as
>> the removed lines (they are just indented).
>
> I see.
>
> I didn't think a function this deep in the callchain that does not
> take any parameter could possibly change the behaviour based on the
> end-user input. I was expecting that such a state (i.e. are we
> recursive? are we allowed to forcibly update the working tree
> files?) would be kept part of something like "struct checkout" and
> passed around the callchain.
>
> That was why I didn't look at how that function answers "should
> update?" question, and got puzzled. Because it would imply there is
> some hidden state that is accessible by everybody--a global variable
> or something--which would point at a deeper design issue.
Well it is just as deep as e.g. reacting on some bits of
struct unpack_trees_options in unpack-trees.c ?
But I can see how this is an issue with design.
I also think my previous answer was a straw man.
We only need to differentiate between 'ce' being a submodule
or not, because it should not be marked CE_REMOVE in
the non-recursive state.
Side-question:
Is there some doc (commit message), that explains the difference
between CE_REMOVE and CE_WT_REMOVE ?
Thanks,
Stefan