Re: [PATCH tg/add-chmod+x-fix 2/2] t3700-add: protect one --chmod=+x test with POSIXPERM

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 21, 2016, 18:12 UTC
Message-ID: <xmqqtwd986ml.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160920193444.GG8254@hank>

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 8 quoted lines

>>  I am surprised that add --chmod=+x changes only the index, but not
>>  the file on disk!?!
>
> I *think* --chmod is mainly thought of as a convenience for git users
> on a filesystem that doesn't have an executable flag.  So it was
> introduced this way as the permissions on the file system don't matter
> in that case.  A change of that behaviour may make sense for this
> though.

Perhaps we shouldn't even test this, then? I can see future people wanting to change this behaviour, while I can see argument for not touching the working tree file, too. It is essential for the main purpose of the command (i.e. "I want to flip the executable bit for the path in the index") to make sure that the bit in the index is changed. Comparing the index with the working tree using "status" is probably not how you would want to do so. A future breakage may cause the indexed blob name to change by mistake, and status would happily report difference but you would not notice its output is saying "Hey, they are different between the index and the working tree", while you are expecting ONLY the change in the executable bit.

How about doing

	git add foo4 &&
        git add --chmod=+x foo4 &&
	test_mode_in_index 100755 foo4

instead?

Show 20 quoted lines

>
>>  t/t3700-add.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
>> index 16ab2da..13e0dd2 100755
>> --- a/t/t3700-add.sh
>> +++ b/t/t3700-add.sh
>> @@ -361,7 +361,7 @@ test_expect_success 'git add --chmod=[+-]x changes index with already added file
>>  	test_mode_in_index 100644 xfoo3
>>  '
>>  
>> -test_expect_success 'file status is changed after git add --chmod=+x' '
>> +test_expect_success POSIXPERM 'file status is changed after git add --chmod=+x' '
>>  	echo "AM foo4" >expected &&
>>  	echo foo >foo4 &&
>>  	git add foo4 &&
>> -- 
>> 2.10.0.85.gea34e30
>>