Re: [PATCH v2 2/3] unpack-trees: increment cache_bottom for sparse directories

[Derrick Stolee <derrickstolee@github.com>]

On 3/17/2022 11:55 AM, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@github.com>
> 
> Correct tracking of the 'cache_bottom' for cases where sparse directories
> are present in the index.

Thank you for the detailed background (that I cut from my reply).

> CORRECTING CACHE_BOTTOM
> -----------------------
> The problems observed in 't1092' come from 'cache_bottom' lagging behind in
> cases where the cache tree-based advancement doesn't occur. To solve this,
> then, the fix in 17a1bb570b is "reversed"; rather than skipping
> 'cache_bottom' advancement in 'mark_ce_used()', we skip the directory
> contents-based advancement for sparse directories. Now, every index entry
> can be accounted for in 'cache_bottom':

I have sufficient background to be confident that you are doing the
right thing here!

> -test_expect_failure 'add outside sparse cone' '
> +test_expect_success 'add outside sparse cone' '

> -test_expect_failure 'status/add: outside sparse cone' '
> +test_expect_success 'status/add: outside sparse cone' '

> -test_expect_failure 'reset with pathspecs inside sparse definition' '
> +test_expect_success 'reset with pathspecs inside sparse definition' '

Love to see it.

>  			if (matches) {
> -				o->cache_bottom += matches;
> +				/*
> +				 * Only increment the cache_bottom if the
> +				 * directory isn't a sparse directory index
> +				 * entry (if it is, it was already incremented)
> +				 * in 'mark_ce_used()'
> +				 */
> +				if (!src[0] || !S_ISSPARSEDIR(src[0]->ce_mode))
> +					o->cache_bottom += matches;
>  				return mask;

Looks great.

Thanks,
-Stolee