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

From: Victoria Dye <[email-masked]>
Date: Tue, 26 Apr 2022 08:26:08 -0700

Derrick Stolee wrote:
> On 4/25/2022 1:49 PM, Victoria Dye via GitGitGadget wrote:
>> From: Victoria Dye <[email-masked]>
>>
>> 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 &&
> 
> 
>> -	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.
> 

That was unintentional. I originally had the 'stash' cases in the big
'sparse-index is not expanded' test, but decided to move them into their own
test for more appropriate granularity. In doing that, I accidentally took
some of the 'checkout-index' tests with it. I'll move them back in V2.

Thanks for the catch!

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