All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Tobias Klauser <tklauser@distanz.ch>,
	Christian Couder <chriscool@tuxfamily.org>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
	Git List <git@vger.kernel.org>
Subject: Re: [PATCH v4 2/2] interpret-trailers: add option for in-place editing
Date: Tue, 19 Jan 2016 14:09:37 -0800	[thread overview]
Message-ID: <xmqqvb6p8bmm.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAPig+cRHTs9q4k=CqtY2j=ZtTYMU6_SPeCHkQe4m5AGXOjg_Ww@mail.gmail.com> (Eric Sunshine's message of "Tue, 19 Jan 2016 16:45:15 -0500")

Eric Sunshine <sunshine@sunshineco.com> writes:

> You suspect correctly. It was exactly the comment added by f400e51c
> that misled me. (t/README does, on the other hand, mention "root", as
> I noticed after reading your previous response.)
>
> Thanks for spelling all this out. Hopefully, others reading your reply
> (now and later) will be less confused than I.

It is not too late to fix that, though.

-- >8 --
Subject: test-lib: clarify and tighten SANITY

f400e51c (test-lib.sh: set prerequisite SANITY by testing what we
really need, 2015-01-27) improved the way SANITY prerequisite was
determined, but made the resulting code (incorrectly) imply that
SANITY is all about effects of permission bits of the containing
directory has on the files contained in it by the comment it added,
its log message and the actual tests.

By the way, while we are on the subject, POSIXPERM is more about "if
we do chmod, does filesystem remember it so that ls -l reports the
same?"  Output from "git grep POSIXPERM t" shows that some users of
it also assume that it requires "we can make something executable by
doing chmod +x and unexecutable by doing chmod -x" (and that is
fine--running tests as root would not make an unexecutable file
executable).  The tests that require POSIXPERM but not SANITY can be
run by root (I am not saying that running tests as root is safe or
sane, though) and are expected to produce the same result as they
were run by a non-root user.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/test-lib.sh | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 446d8d5..68c31ae 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -997,20 +997,28 @@ test_lazy_prereq NOT_ROOT '
 	test "$uid" != 0
 '
 
-# 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.
+# SANITY is about "can you correctly predict what the filesystem would
+# do by only looking at the permission bits of the files and
+# directories?"  A typical example of !SANITY is running the test
+# suite as root, where a test may expect "chmod -r file && cat file"
+# to fail because file is supposed to be unreadable after a successful
+# chmod.  In an environment (i.e. combination of what filesystem is
+# being used and who is running the tests) that lacks SANITY, you may
+# be able to delete or create a file when the containing directory
+# doesn't have write permissions, or access a file even if the
+# containing directory doesn't have read or execute permissions.
+
 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 -r SANETESTD.1/x &&
 	chmod -rx SANETESTD.2 ||
 	error "bug in test sript: cannot prepare SANETESTD"
 
+	! test -r SANETESTD.1/x &&
 	! rm SANETESTD.1/x && ! test -f SANETESTD.2/x
 	status=$?
 

  reply	other threads:[~2016-01-19 22:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14 16:57 [PATCH v4 0/2] Add in-place editing support to git interpret-trailers Tobias Klauser
2016-01-14 16:57 ` [PATCH v4 1/2] trailer: allow to write to files other than stdout Tobias Klauser
2016-01-14 16:57 ` [PATCH v4 2/2] interpret-trailers: add option for in-place editing Tobias Klauser
2016-01-14 20:45   ` Junio C Hamano
2016-01-15 10:34     ` Tobias Klauser
2016-01-15 17:24       ` Junio C Hamano
2016-01-15 17:45         ` Tobias Klauser
2016-01-18 21:11     ` Eric Sunshine
     [not found]       ` <CAPc5daWpnReWJzeTJjvZap78H0oZKG-YGEP19Neusyahu5A6cQ@mail.gmail.com>
2016-01-18 22:13         ` Eric Sunshine
2016-01-19  8:28           ` Tobias Klauser
2016-01-19 17:52       ` Junio C Hamano
2016-01-19 17:56         ` Eric Sunshine
2016-01-19 18:10           ` Eric Sunshine
2016-01-19 20:58             ` Junio C Hamano
2016-01-19 21:45               ` Eric Sunshine
2016-01-19 22:09                 ` Junio C Hamano [this message]
2016-01-20  0:20                   ` Eric Sunshine

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=xmqqvb6p8bmm.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=sunshine@sunshineco.com \
    --cc=tklauser@distanz.ch \
    /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.