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

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  read-cache.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 9054369dd0..9a2abacf7a 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -18,6 +18,8 @@
>  #include "varint.h"
>  #include "split-index.h"
>  #include "utf8.h"
> +#include "submodule.h"
> +#include "submodule-config.h"
>  
>  /* Mask for the name length in ce_flags in the on-disk index */
>  
> @@ -520,6 +522,22 @@ int remove_index_entry_at(struct index_state *istate, int pos)
>  	return 1;
>  }
>  
> +static void remove_submodule_according_to_strategy(const struct submodule *sub)
> +{
> +	switch (sub->update_strategy.type) {
> +	case SM_UPDATE_UNSPECIFIED:
> +	case SM_UPDATE_CHECKOUT:
> +	case SM_UPDATE_REBASE:
> +	case SM_UPDATE_MERGE:
> +		submodule_move_head(sub->path, "HEAD", NULL, \

What is this backslash doing here?

> +				    SUBMODULE_MOVE_HEAD_FORCE);
> +		break;
> +	case SM_UPDATE_NONE:
> +	case SM_UPDATE_COMMAND:
> +		; /* Do not touch the submodule. */
> +	}
> +}
> +
>  /*
>   * Remove all cache entries marked for removal, that is where
>   * CE_REMOVE is set in ce_flags.  This is much more effective than
> @@ -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?  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)?

>  		}
>  		else
>  			ce_array[j++] = ce_array[i];