Subject: Re: [GSOC][PATCH v4] describe: enable sparse index for describe

From: Victoria Dye <[email-masked]>
Date: Mon, 3 Apr 2023 09:34:10 -0700

Raghul Nanth A wrote:
> git describe compares the index with the working tree when (and only
> when) it is run with the "--dirty" flag. This is done by the
> run_diff_index() function. The function has been made aware of the
> sparse-index in the series that led to 8d2c3732 (Merge branch
> 'ld/sparse-diff-blame', 2021-12-21). Hence we can just set the
> requires-full-index to false for "describe".
> 

This is a good description of the patch and the reasoning behind the
changes.

> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 801919009e..2b46fb2a48 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -1514,6 +1514,36 @@ test_expect_success 'sparse-index is not expanded: stash' '
>  	ensure_not_expanded stash pop
>  '
>  
> +test_expect_success 'describe tested on all' '
> +	init_repos &&
> +
> +	# Add tag to be read by describe
> +
> +	run_on_all git tag -a v1.0 -m "Version 1" &&
> +	test_all_match git describe --dirty &&
> +	run_on_all rm g &&
> +	test_all_match git describe --dirty
> +'
> +
> +
> +test_expect_success 'sparse-index is not expanded: describe' '
> +	init_repos &&
> +
> +	# Add tag to be read by describe
> +
> +	git -C sparse-index tag -a v1.0 -m "Version 1" &&
> +
> +	ensure_not_expanded describe --dirty &&
> +	echo "test" >>sparse-index/g &&
> +	ensure_not_expanded describe --dirty &&
> +	echo "v1.0-dirty" >actual &&
> +
> +	# Check describe on dirty work tree
> +
> +	test_cmp sparse-index-out actual &&

This type of output comparison should be part of 'describe tested on all',
not the "ensure not expanded" test; the former tests the correctness of 'git
describe', whereas the latter focuses on index expansion. When unit testing,
it helps to keep the scope of each test fairly narrow with a specific focus
so that they can more easily isolate future regressions.

> +	ensure_not_expanded describe
> +'
> +
>  test_expect_success 'sparse index is not expanded: diff' '
>  	init_repos &&
>