Re: [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 8, 2017, 01:14 UTC
- Message-ID
- <xmqqlgsg4lj4.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <CAGZ79kbdJa54YyKPHR9ycuJBVtyNY_2yaD7_5RPPu++Awiz5cA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
Show 17 quoted lines
> 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.