All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Matt McCutchen <matt@mattmccutchen.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH try 2] t1301-shared-repo.sh: don't let a default ACL interfere with the test
Date: Tue, 14 Oct 2008 15:32:13 -0700	[thread overview]
Message-ID: <7vzll66c5u.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <1224022216.2699.5.camel@mattlaptop2.local> (Matt McCutchen's message of "Tue, 14 Oct 2008 18:10:16 -0400")

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?

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

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

  reply	other threads:[~2008-10-14 22:33 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-14 22:07 [PATCH] t1301-shared-repo.sh: don't let a default ACL interfere with the test Matt McCutchen
2008-10-14 22:10 ` [PATCH try 2] " Matt McCutchen
2008-10-14 22:32   ` Junio C Hamano [this message]
2008-10-14 23:00     ` Matt McCutchen
2008-10-14 23:45       ` Deskin Miller
2008-10-15  6:13     ` Johannes Sixt
2008-10-15  6:30       ` Junio C Hamano
2008-10-15  7:18         ` Johannes Sixt
2008-10-15  7:57           ` Junio C Hamano
2008-10-15  8:10             ` Johannes Sixt
2008-10-16  5:23               ` Junio C Hamano
2008-10-17  2:28                 ` Matt McCutchen
2008-10-17  4:30                   ` Junio C Hamano
2008-10-17  4:33                     ` Matt McCutchen
2008-10-17  2:28                 ` [PATCH try 3] " Matt McCutchen
2008-10-17  2:32                 ` [PATCH try 4] " Matt McCutchen
2008-10-15 14:34       ` [PATCH try 2] " Matt McCutchen

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

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

  git send-email \
    --in-reply-to=7vzll66c5u.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=matt@mattmccutchen.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.