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

From: Derrick Stolee <derrickstolee@github.com>
Date: Mar 21, 2022, 19:03 UTC
Message-ID: <df403c7b-5152-492a-96b5-575ccc1520de@github.com>
In-Reply-To: <cf8dc50c300a08f54d0cf69c82d4d979922a8143.1647532536.git.gitgitgadget@gmail.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).

Show 8 quoted lines

> 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.

Show 11 quoted lines

>  			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