On Tue, Mar 7, 2017 at 2:42 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> + submodule_move_head(sub->path, "HEAD", NULL, \
>
> What is this backslash doing here?
I am still bad at following coding conventions, apparently.
will remove.
>> @@ -532,8 +550,13 @@ void remove_marked_cache_entries(struct index_state *istate)
>>
>> for (i = j = 0; i < istate->cache_nr; i++) {
>> if (ce_array[i]->ce_flags & CE_REMOVE) {
>> - remove_name_hash(istate, ce_array[i]);
>> - save_or_free_index_entry(istate, ce_array[i]);
>> + const struct submodule *sub = submodule_from_ce(ce_array[i]);
>> + if (sub) {
>> + remove_submodule_according_to_strategy(sub);
>> + } else {
>> + remove_name_hash(istate, ce_array[i]);
>> + save_or_free_index_entry(istate, ce_array[i]);
>> + }
>
> I cannot readily tell as the proposed log message is on the sketchy
> side to explain the reasoning behind this, but this behaviour change
> is done unconditionally (in other words, without introducing a flag
> that is set by --recurse-submodules command line flag and switches
> behaviour based on the flag) here. What is the end-user visible
> effect with this change?
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).
> Can we have a new test in this same patch
> to demonstrate the desired behaviour introduced by this change (or
> the bug this change fixes)?
This doesn't fix a bug, but in case the flag is given (in patches 17,18)
this new code removes submodules that are no longer recorded in
the tree.