Re: [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.
- From
Stefan Beller <sbeller@google.com>
- Date
- Mar 7, 2017, 23:37 UTC
- Message-ID
- <CAGZ79kbdJa54YyKPHR9ycuJBVtyNY_2yaD7_5RPPu++Awiz5cA@mail.gmail.com>
- In-Reply-To
- <xmqqfuio674n.fsf@gitster.mtv.corp.google.com>
On Tue, Mar 7, 2017 at 2:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes: > >> Signed-off-by: Stefan Beller <sbeller@google.com>
>> + submodule_move_head(sub->path, "HEAD", NULL, \ > > What is this backslash doing here?
I am still bad at following coding conventions, apparently. will remove.
Show 20 quoted lines
>> @@ -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.