Re: [PATCH v3 02/14] t1092: behavior for adding sparse files

[Derrick Stolee <stolee@gmail.com>]

On 9/22/2021 7:06 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> Add some tests to demonstrate the current behavior around adding files
>> outside of the sparse-checkout cone. Currently, untracked files are
>> handled differently from tracked files. A future change will make these
>> cases be handled the same way.
>>
>> Further expand checking that a failed 'git add' does not stage changes
>> to the index.
>>
>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>> ---
>>  t/t1092-sparse-checkout-compatibility.sh | 28 ++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
>> index 886e78715fe..3fb764f5eb9 100755
>> --- a/t/t1092-sparse-checkout-compatibility.sh
>> +++ b/t/t1092-sparse-checkout-compatibility.sh
>> @@ -187,6 +187,16 @@ test_sparse_match () {
>>  	test_cmp sparse-checkout-err sparse-index-err
>>  }
>>  
>> +test_sparse_unstaged () {
>> +	file=$1 &&
>> +	for repo in sparse-checkout sparse-index
>> +	do
>> +		git -C $repo status --porcelain >$repo-out &&
>> +		! grep "^A  $file\$" $repo-out &&
>> +		! grep "^M  $file\$" $repo-out || return 1
> 
> Is addition and modification very special, as opposed to other kinds
> of changes?  Is the reason why we say "we do not want to see
> addition nor modification" here because there is no concrete X that
> we can say "we do want to see X" in various callers of this helper?
> 
> I am also wondering, if this is asserting that $file is not added to
> the index, why we are using "git status" and not "ls-files", for
> example.  Wouldn't
> 
>     test_must_fail git ls-files --error-unmatch "$file"
> 
> be a more direct way to do so?

The intention is to avoid a staged change, even if that is a modification
or addition. The --error-unmatch approach only works if we are trying to
avoid an add, and we need to know that the path does not exist in the
index already if we want to take that approach.

Upon reflection, perhaps what we really want is this:

	git diff --staged -- $path >diff &&
	test_must_be_empty diff

This applies to the previous patch, too.

Thanks,
-Stolee