git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Matt McCutchen <matt@mattmccutchen.net>
Date
Oct 14, 2008, 23:00 UTC
Message-ID
<1224025227.2699.13.camel@mattlaptop2.local>
In-Reply-To
<7vzll66c5u.fsf@gitster.siamese.dyndns.org>
On Tue, 2008-10-14 at 15:32 -0700, Junio C Hamano wrote:
Show 5 quoted lines
> Matt McCutchen <matt@mattmccutchen.net> 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.
Show 10 quoted lines
> > 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.
Show 22 quoted lines
> > Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
> > ---
> > 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
Previous: Junio C HamanoNext: Deskin Miller
Message 4 of 17 in “t1301-shared-repo.sh: don't let a default ACL interfere with the test”
  1. t1301-shared-repo.sh: don't let a default ACL interfere with the testMatt McCutchen, Oct 14, 2008
  2. t1301-shared-repo.sh: don't let a default ACL interfere with the testMatt McCutchen, Oct 14, 2008
  3. Junio C HamanoOct 14, 2008
  4. Matt McCutchenOct 14, 2008
  5. Deskin MillerOct 14, 2008
  6. Johannes SixtOct 15, 2008
  7. Junio C HamanoOct 15, 2008
  8. Johannes SixtOct 15, 2008
  9. Junio C HamanoOct 15, 2008
  10. Johannes SixtOct 15, 2008
  11. Junio C HamanoOct 16, 2008
  12. Matt McCutchenOct 17, 2008
  13. Junio C HamanoOct 17, 2008
  14. Matt McCutchenOct 17, 2008
  15. t1301-shared-repo.sh: don't let a default ACL interfere with the testMatt McCutchen, Oct 17, 2008
  16. t1301-shared-repo.sh: don't let a default ACL interfere with the testMatt McCutchen, Oct 17, 2008
  17. Matt McCutchenOct 15, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.