Re: [PATCH 11/16] unpack_trees(): add support for narrow checkout
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 15, 2008, 19:34 UTC
- Message-ID
- <7vsks1b3tq.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1221397685-27715-12-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 34 quoted lines
> 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.