Re: [PATCH 11/16] unpack_trees(): add support for narrow checkout

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

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> diff --git a/cache.h b/cache.h
> index 2b2c90f..1fc0f83 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -167,6 +167,9 @@ struct cache_entry {
>  #define CE_HASHED    (0x100000)
>  #define CE_UNHASHED  (0x200000)
>  
> +/* Only remove in work directory, not index */
> +#define CE_WD_REMOVE (0x400000)
> +
>  /*
>   * Extended on-disk flags
>   */
> diff --git a/unpack-trees.c b/unpack-trees.c
> index e59d144..968cc98 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -96,6 +96,15 @@ static int check_updates(struct unpack_trees_options *o)
>  	if (o->update && o->verbose_update) {
>  		for (total = cnt = 0; cnt < index->cache_nr; cnt++) {
>  			struct cache_entry *ce = index->cache[cnt];
> +
> +			if (ce->ce_flags & CE_WD_REMOVE) {
> +				total++;
> +				continue;
> +			}
> +
> +			if (ce_no_checkout(ce))
> +				continue;
> +
>  			if (ce->ce_flags & (CE_UPDATE | CE_REMOVE))
>  				total++;
>  		}

This check for ce_no_checkout() before the existing check to see if
CE_UPDATE and/or CE_REMOVE are on puzzles me (you have the same check
sequence in later loop that does the actualy worktree updates).  If
apply_narrow_checkout() check is working correctly, would we ever give
CE_UPDATE and/or CE_REMOVE bits to a cache entry with CE_NO_CHECKOUT set?

IOW, I think this extra check is Ok as it is expected to be no-op when
combined with the existing check, but it leaves me wondering if it is
hiding some codepath that incorrectly sets CE_UPDATE/CE_REMOVE to an entry
outside the narrow checkout area.

I have two more messages but other than these the series is nicely done
and a pleasure to read.