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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2008-10-14 at 15:32 -0700, Junio C Hamano wrote:
> Matt McCutchen <matt@xxxxxxxxxxxxxxxxx> writes:
> > 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.

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

> > Signed-off-by: Matt McCutchen <matt@xxxxxxxxxxxxxxxxx>
> > ---
> > 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux