git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Thomas Gummerer <t.gummerer@gmail.com>
Cc: Johannes Sixt <j6t@kdbg.org>, Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH tg/add-chmod+x-fix 2/2] t3700-add: protect one --chmod=+x test with POSIXPERM
Date: Wed, 21 Sep 2016 11:12:02 -0700	[thread overview]
Message-ID: <xmqqtwd986ml.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160920193444.GG8254@hank> (Thomas Gummerer's message of "Tue, 20 Sep 2016 20:34:44 +0100")

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

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

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

  reply	other threads:[~2016-09-21 18:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-20  6:16 [PATCH tg/add-chmod+x-fix 1/2] t3700-add: create subdirectory gently Johannes Sixt
2016-09-20  6:18 ` [PATCH tg/add-chmod+x-fix 2/2] t3700-add: protect one --chmod=+x test with POSIXPERM Johannes Sixt
2016-09-20 19:34   ` Thomas Gummerer
2016-09-21 18:12     ` Junio C Hamano [this message]
2016-09-21 20:47       ` Johannes Sixt
2016-09-21 20:47       ` Junio C Hamano
2016-09-21 20:58         ` Johannes Sixt
2016-09-21 21:12           ` Junio C Hamano
2016-09-22  5:06             ` Johannes Sixt
2016-09-22 21:01             ` Thomas Gummerer
2016-09-20 19:28 ` [PATCH tg/add-chmod+x-fix 1/2] t3700-add: create subdirectory gently Thomas Gummerer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtwd986ml.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=t.gummerer@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).