Re: [PATCH 2/7] stash: integrate with sparse index

From: Derrick Stolee <derrickstolee@github.com>
Date: Apr 26, 2022, 12:53 UTC
Message-ID: <f332fec9-da00-0063-4c88-765fa3639ea1@github.com>
In-Reply-To: <f6cf05a5bee9e4ebc174bab0385a13cc1cdb4014.1650908958.git.gitgitgadget@gmail.com>

On 4/25/2022 1:49 PM, Victoria Dye via GitGitGadget wrote:

> From: Victoria Dye <vdye@github.com>
> 
> Enable sparse index in 'git stash' by disabling
> 'command_requires_full_index'.

>  ensure_not_expanded () {
>  	rm -f trace2.txt &&
> -	echo >>sparse-index/untracked.txt &&
> +	if test -z $WITHOUT_UNTRACKED_TXT

Do we need quotes around "$WITHOUT_UNTRACKED_TXT"?

I mean, I suppose we don't since the tests pass when this variable is unset, but I think it is a good practice to be careful about empty variables. Or am I wrong?

> +	then
> +		echo >>sparse-index/untracked.txt
> +	fi &&

Show 8 quoted lines

> -	ensure_not_expanded checkout-index -f a &&
> -	ensure_not_expanded checkout-index -f --all &&
> -	for ref in update-deep update-folder1 update-folder2 update-deep
> -	do
> -		echo >>sparse-index/README.md &&
> -		ensure_not_expanded reset --hard $ref || return 1
> -	done &&
> -

Moving these to be within the stash tests is interesting.

Show 30 quoted lines

> +test_expect_success 'sparse-index is not expanded: stash' '
> +	init_repos &&
> +
> +	echo >>sparse-index/a &&
> +	ensure_not_expanded stash &&
> +	ensure_not_expanded stash list &&
> +	ensure_not_expanded stash show stash@{0} &&
> +	! ensure_not_expanded stash apply stash@{0} &&
> +	ensure_not_expanded stash drop stash@{0} &&
> +
> +	echo >>sparse-index/deep/new &&
> +	! ensure_not_expanded stash -u &&
> +	(
> +		WITHOUT_UNTRACKED_TXT=1 &&
> +		! ensure_not_expanded stash pop
> +	) &&
> +
> +	ensure_not_expanded stash create &&
> +	oid=$(git -C sparse-index stash create) &&
> +	ensure_not_expanded stash store -m "test" $oid &&
> +	ensure_not_expanded reset --hard &&
> +	! ensure_not_expanded stash pop &&
> +
> +	ensure_not_expanded checkout-index -f a &&
> +	ensure_not_expanded checkout-index -f --all &&
> +	for ref in update-deep update-folder1 update-folder2 update-deep
> +	do
> +		echo >>sparse-index/README.md &&
> +		ensure_not_expanded reset --hard $ref || return 1
> +	done

It is not obvious why that's necessary here. Perhaps a later change will build upon these checkout-index commands within this test?

Thanks, -Stolee