Re: [PATCH try 2] t1301-shared-repo.sh: don't let a default ACL interfere with the test

On Tue, 2008-10-14 at 15:32 -0700, Junio C Hamano wrote:

Matt McCutchen [off-list ref] writes:

quoted

This test creates files with several different umasks and expects the files to
be permissioned according to the umasks, so a default ACL on the test dir causes

Is "to permission" a verb?

I thought it could be, but I'll reword that sentence.

quoted

the test to fail.  To avoid that, remove the default ACL if possible with
setfacl(1).  (Will work on many systems.)

It is not clear in the comment in parentheses what provision you have made
not to harm people on systems without setfacl.

I think "if possible" which you already have is a good enough description
(i.e. "if setfacl fails we do not barf and if you do not have the command
you probably are not running with a funky default ACL to see this issue
anyway"), so I'd rather drop the comment in parentheses.

Sure.

quoted

Signed-off-by: Matt McCutchen <redacted>
---
This time with a signoff.

 t/t1301-shared-repo.sh |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index dc85e8b..2275caa 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh

@@ -7,6 +7,9 @@ test_description='Test shared repository initialization'
 
 . ./test-lib.sh
 
+# Remove a default ACL from the test dir if possible.
+setfacl -k . 2>/dev/null
+

Makes me wonder why this is _not_ inside test-lib.sh where it creates the
test (trash) directory.  That way, you would cover future tests that wants
to see a saner/simpler POSIX permission behaviour, wouldn't you?

Yes.  However, I don't anticipate there being any tests specifically
about file permissions other than t1301-shared-repo.sh, and if the user
has set a default ACL on the git source tree, we might want to let trash
directories obey that setting except in the one case where it breaks the
test.  What do you think?

Matt