Re: [PATCHv2 4/5] unpack-trees: oneway_merge to update submodules

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

Stefan Beller <sbeller@google.com> writes:

> diff --git a/unpack-trees.c b/unpack-trees.c
> index bf8b602901..ac59524a7f 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -2139,6 +2139,9 @@ int oneway_merge(const struct cache_entry * const *src,
>  			    ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE))
>  				update |= CE_UPDATE;
>  		}
> +		if (S_ISGITLINK(old->ce_mode) && should_update_submodules() &&
> +		    !verify_uptodate(old, o))
> +			update |= CE_UPDATE;
>  		add_entry(o, old, update, 0);
>  		return 0;
>  	}

This is more sensible than the previous one that broke same() by
unconditionally checking the working tree state, even when the
machinery is told not to look at the working tree.

The code in the pre-context of this hunk, (i.e. the original one you
are half-way mimicking), we realize that the original cache entry
and the cache entry we are checking out are exactly the same and we
overwrite when we know that the path in the working tree is dirty,
and we are not told to "skip" that path in the working tree
(i.e. sparse checkout).  That happens only when we are told to
o->update and o->reset.

Shouldn't we be setting the update bit only when o->update is in
effect, just like we can see in the pre-context of this hunk?  I do
not know if we need to require o->reset and/or need to pay attention
to the skip worktree bit in this new case.