Re: [PATCH 2/2] test-lib.sh: Dynamic test for the prerequisite SANITY

public inbox for git@vger.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 Bögershausen"'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

next prev 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

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:b2b2ec7 dfblob:446d8d5 )
 OR (
bs:"test-lib.sh: set prerequisite SANITY by testing what we really need" )
	(help)

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$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=tboegi@web$(echo .)de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox