All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Torsten Bögershausen" <tboegi@web.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] test-lib.sh: Dynamic test for the prerequisite SANITY
Date: Tue, 27 Jan 2015 14:20:19 -0800	[thread overview]
Message-ID: <xmqqh9vbkgrg.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <54C7B115.7020405@web.de> ("Torsten =?utf-8?Q?B=C3=B6gershaus?= =?utf-8?Q?en=22's?= message of "Tue, 27 Jan 2015 16:39:01 +0100")

Torsten Bögershausen <tboegi@web.de> writes:

>  # When the tests are run as root, permission tests will report that
>  # things are writable when they shouldn't be.

This no longer is relevant, I think.

> +# Special check for CYGWIN (or Windows in general):

Misleading comment in the end result, as your new test drops SANITY
correctly on POSIX for the root user, too.  In a commit log message
it is correct to say "This adds special check for Cygwin", but the
resulting code is sensible with or without Cygwin, I would think,
with the justification to "test by checking what we really want, not
by inferring from the result of indirectly testing something else".

> +# A file can be deleted, even if the containing directory does'nt
> +# have write permissions

We also rely on SANITY to make sure that "chmod -rx directory" makes
"directory/file" undiscoverable.

How about extending it like this (not tested)?

-- >8 --
From: Torsten Bögershausen <tboegi@web.de>
Date: Tue, 27 Jan 2015 16:39:01 +0100
Subject: [PATCH] test-lib.sh: set prerequisite SANITY by testing what we really need

What we wanted out of the SANITY precondition is that the filesystem
behaves sensibly with permission bits settings.

 - You should not be able to remove a file in a read-only directory,

 - You should not be able to tell if a file in a directory exists if
   the directory lacks read or execute permission bits.

We used to cheat by approximating that condition with "is the /
writable?" test and/or "are we running as root?" test.  Neither test
is sufficient or appropriate in more exotic environments like
Cygwin.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/test-lib.sh | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index b2b2ec7..446d8d5 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -997,9 +997,28 @@ test_lazy_prereq NOT_ROOT '
 	test "$uid" != 0
 '
 
-# When the tests are run as root, permission tests will report that
-# things are writable when they shouldn't be.
-test -w / || test_set_prereq SANITY
+# On a filesystem that lacks SANITY, a file can be deleted even if
+# the containing directory doesn't have write permissions, or a file
+# can be accessed even if the containing directory doesn't have read
+# or execute permissions, causing our tests that validate that Git
+# works sensibly in such situations.
+test_lazy_prereq SANITY '
+	mkdir SANETESTD.1 SANETESTD.2 &&
+
+	chmod +w SANETESTD.1 SANETESTD.2 &&
+	>SANETESTD.1/x 2>SANETESTD.2/x &&
+	chmod -w SANETESTD.1 &&
+	chmod -rx SANETESTD.2 ||
+	error "bug in test sript: cannot prepare SANETESTD"
+
+	! rm SANETESTD.1/x && ! test -f SANETESTD.2/x
+	status=$?
+
+	chmod +rwx SANETESTD.1 SANETESTD.2 &&
+	rm -rf SANETESTD.1 SANETESTD.2 ||
+	error "bug in test sript: cannot clean SANETESTD"
+	return $status
+'
 
 GIT_UNZIP=${GIT_UNZIP:-unzip}
 test_lazy_prereq UNZIP '
-- 
2.3.0-rc1-180-g1a69fe5

  parent reply	other threads:[~2015-01-27 22:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-27 15:39 [PATCH 2/2] test-lib.sh: Dynamic test for the prerequisite SANITY Torsten Bögershausen
2015-01-27 19:53 ` Chris Packham
2015-01-27 22:20 ` Junio C Hamano [this message]
2015-01-28  8:28   ` Torsten Bögershausen
2015-01-28 17:38     ` Junio C Hamano
2015-01-28 19:19       ` Torsten Bögershausen
2015-01-28 20:38         ` Junio C Hamano

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=xmqqh9vbkgrg.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=tboegi@web.de \
    /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.