Thread (88 messages) 88 messages, 4 authors, 2017-03-15

Re: [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.

From: Stefan Beller <hidden>
Date: 2017-03-07 23:46:49

Possibly related (same subject, not in this thread)

On Tue, Mar 7, 2017 at 2:42 PM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
Signed-off-by: Stefan Beller <redacted>
quoted
+             submodule_move_head(sub->path, "HEAD", NULL, \
What is this backslash doing here?
I am still bad at following coding conventions, apparently.
will remove.
quoted
@@ -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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help