git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
	Stefan Beller <stefanbeller@googlemail.com>,
	git@vger.kernel.org
Subject: [PATCH] t1304: improve setfacl prerequisite setup
Date: Wed, 6 Jun 2012 09:28:24 -0400	[thread overview]
Message-ID: <20120606132824.GA2597@sigill.intra.peff.net> (raw)
In-Reply-To: <7v62b5pt2s.fsf@alter.siamese.dyndns.org>

t1304 first runs setfacl as an experiment to see whether the
filesystem supports ACLs, and skips the remaining tests if
it does not. However, our setfacl run did not exercise the
ACLs very well, and some filesystems may support our initial
setfacl, but not the rest of the test.

In particular, some versions of ecryptfs will erroneously
apply the umask on top of an inherited directory ACL,
causing our tests to fail. Let's be more careful and make
sure both that we can read back the user ACL we set, and
that the inherited ACL is propagated correctly. The latter
catches the ecryptfs bug, but may also catch other bugs
(e.g., an implementation which does not handle inherited
ACLs at all).

Since we're making the setup more complex, let's move it
into its own test. This will hide the output for us unless
the user wants to run "-v" to see it (and we don't need to
bother printing anything about setfacl failing; the
remaining tests will properly print "skip" due to the
missing prerequisite).

Signed-off-by: Jeff King <peff@peff.net>
---
The ecryptfs response was that it is probably a bug, so I think we
should go with this (especially because it is general enough to
potentially catch other weird errors).

 t/t1304-default-acl.sh | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 2b962cf..79045ab 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -14,16 +14,15 @@ umask 077
 # We need an arbitrary other user give permission to using ACLs. root
 # is a good candidate: exists on all unices, and it has permission
 # anyway, so we don't create a security hole running the testsuite.
-
-setfacl_out="$(setfacl -m u:root:rwx . 2>&1)"
-setfacl_ret=$?
-
-if test $setfacl_ret != 0
-then
-	say "Unable to use setfacl (output: '$setfacl_out'; return code: '$setfacl_ret')"
-else
-	test_set_prereq SETFACL
-fi
+test_expect_success 'checking for a working acl setup' '
+	if setfacl -m d:m:rwx -m u:root:rwx . &&
+	   getfacl . | grep user:root:rwx &&
+	   touch should-have-readable-acl &&
+	   getfacl should-have-readable-acl | egrep "mask::?rw-"
+	then
+		test_set_prereq SETFACL
+	fi
+'
 
 if test -z "$LOGNAME"
 then
-- 
1.7.11.rc1.4.g0d3b9b3

  reply	other threads:[~2012-06-06 13:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-04 15:40 Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask Stefan Beller
2012-06-04 20:18 ` Junio C Hamano
     [not found]   ` <CALbm-EatNCPjFRO4NyGfZuSa72-FXwZcd_7cFe-f_iMOdGL4MQ@mail.gmail.com>
2012-06-04 22:19     ` Junio C Hamano
2012-06-05  6:02       ` Matthieu Moy
2012-06-05  7:23         ` Stefan Beller
2012-06-05  7:43           ` Stefan Beller
2012-06-05  7:56           ` Jeff King
2012-06-05 11:29             ` Matthieu Moy
2012-06-05 12:23               ` Stefan Beller
2012-06-05 13:28                 ` Matthieu Moy
2012-06-05 14:04                   ` Jeff King
2012-06-05 14:10                     ` Jeff King
2012-06-05 14:28                       ` Jeff King
2012-06-05 15:05                         ` Jeff King
2012-06-05 16:31                           ` Junio C Hamano
2012-06-05 16:44                             ` Jeff King
2012-06-05 17:27                               ` Junio C Hamano
2012-06-06 13:28                                 ` Jeff King [this message]
2012-06-06 16:11                                   ` [PATCH] t1304: improve setfacl prerequisite setup Stefan Beller
2012-06-06 16:58                                   ` Junio C Hamano
2012-06-07  9:02                                     ` Jeff King
2012-06-07 16:16                                       ` Junio C Hamano
2012-06-08  1:18                                         ` Brandon Casey
2012-06-05 19:16                               ` Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask Tyler Hicks
2012-06-05 20:45                                 ` Stefan Beller
2012-06-05 13:29                 ` Jeff King

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=20120606132824.GA2597@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stefanbeller@googlemail.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).