git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Torsten Bögershausen" <tboegi@web.de>
To: "Junio C Hamano" <gitster@pobox.com>,
	"Torsten Bögershausen" <tboegi@web.de>
Cc: Jeff King <peff@peff.net>, "Kyle J. McKay" <mackyle@gmail.com>,
	msysgit@googlegroups.com, Git Mailing List <git@vger.kernel.org>
Subject: Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Date: Fri, 23 Jan 2015 22:24:17 +0100	[thread overview]
Message-ID: <54C2BC01.2030307@web.de> (raw)
In-Reply-To: <xmqqlhkusc4h.fsf@gitster.dls.corp.google.com>

On 2015-01-22 23.07, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> If I run that sequence manually:
>> chmod 755 .
>> touch x
>> chmod a-w .
>> rm x
>> touch y
>>
>> x is gone, (but shoudn't according to POSIX)
>> y is not created, "access denied"
> 
> Good (or is that Sad?).
> 
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -1039,7 +1039,17 @@ test_lazy_prereq NOT_ROOT '
>>  # When the tests are run as root, permission tests will report that
>>  # things are writable when they shouldn't be.
>>  test_lazy_prereq SANITY '
>> -       test_have_prereq POSIXPERM,NOT_ROOT
>> +       mkdir ds &&
>> +       touch ds/x &&
>> +       chmod -w ds &&
>> +       if rm ds/x
>> +       then
>> +               chmod +w ds
>> +               false
>> +       else
>> +               chmod +w ds
>> +               true
>> +       fi
>>  '
> 
> It looks like a better approach overall.
> 
> Because we cannot know where $(pwd) is when lazy prereq is evaluated
> (it typically is at the root of the trash hierarchy, but not always)
> and we would not want to add, leave or remove random files in the
> working tree that are not expected by the tests proper (e.g. a test
> that counts untracked paths are not expecting ds/ to be there), your
> actual "fix" may need to be a bit more careful, though.
> 
> Thanks.
> 

Hm,
I think there are 2 different possiblities to go further,
either to always switch off SANITY for CYGWIN (or Windows in general).
I haven't tested anything, the idea came up while writing this email.

The other way is to go away from the hard coded "we know we are root,
so SANITY must be false, or we know that Windows is not 100% POSIX",
and probe the OS/FS dynamically.

The following probably deserves the price for the most clumsy prerequisite
ever written.
(Copy&Paste of a real patch into the mailer, not sure if it applies)

It has been tested under Mac OS, root@Mac OS, Cygwin / Msysgit
What do you think ?



-- >8 --
Subject: [PATCH 1/2] test-lib.sh: Improve SANITY

SANITY was not set when running as root,
but this is not 100% reliable for CYGWIN:

A file is allowed to be deleted when the containing
directory does not have write permissions.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/test-lib.sh | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 93f7cad..b8f736f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1038,8 +1038,26 @@ test_lazy_prereq NOT_ROOT '
 
 # When the tests are run as root, permission tests will report that
 # things are writable when they shouldn't be.
+# Special check for CYGWIN (or Windows in general):
+# A file can be deleted, even if the containing directory does'nt
+# have write permissions
 test_lazy_prereq SANITY '
-	test_have_prereq POSIXPERM,NOT_ROOT
+	dsdir=$$ds
+	mkdir $dsdir &&
+	touch $dsdir/x &&
+	chmod -w $dsdir &&
+	if rm $dsdir/x
+	then
+		chmod +w $dsdir
+		rm -rf $dsdir
+		echo >&2 SANITY=false
+		false
+	else
+		chmod +w $dsdir
+		rm -rf $dsdir
+		echo >&2 SANITY=true
+		true
+	fi
 '
 
 GIT_UNZIP=${GIT_UNZIP:-unzip}
-- 


Subject: [PATCH 2/2] t2026 needs SANITY

When running as root 'prune directories with unreadable gitdir' in t2026 fails.
Protect this TC with SANITY

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/t2026-prune-linked-checkouts.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh
index 170aefe..2936d52 100755
--- a/t/t2026-prune-linked-checkouts.sh
+++ b/t/t2026-prune-linked-checkouts.sh
@@ -33,7 +33,7 @@ EOF
 	! test -d .git/worktrees
 '
 
-test_expect_success POSIXPERM 'prune directories with unreadable gitdir' '
+test_expect_success SANITY 'prune directories with unreadable gitdir' '
 	mkdir -p .git/worktrees/def/abc &&
 	: >.git/worktrees/def/def &&
 	: >.git/worktrees/def/gitdir &&

  parent reply	other threads:[~2015-01-23 21:24 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-14 15:39 t5539 broken under Mac OS X Torsten Bögershausen
2015-01-14 18:37 ` Junio C Hamano
2015-01-14 19:50   ` Torsten Bögershausen
2015-01-14 21:17     ` Jeff King
2015-01-15  5:48       ` Kyle J. McKay
2015-01-15 20:29         ` Junio C Hamano
2015-01-15 22:27           ` Jeff King
2015-01-15 22:39             ` Junio C Hamano
2015-01-15 23:57               ` Jeff King
2015-01-16  0:04                 ` Junio C Hamano
2015-01-16  1:32                   ` [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT Jeff King
2015-01-16  3:27                     ` Kyle J. McKay
2015-01-16  3:34                       ` Jeff King
2015-01-16  9:16                         ` Jeff King
2015-01-16 18:32                           ` Junio C Hamano
2015-01-16 19:02                             ` Junio C Hamano
2015-01-17 23:35                               ` Torsten Bögershausen
2015-01-21 22:33                                 ` Junio C Hamano
2015-01-22 21:51                                   ` Torsten Bögershausen
2015-01-22 22:07                                     ` Junio C Hamano
2015-01-23  6:00                                       ` Torsten Bögershausen
2015-02-12 22:36                                         ` Junio C Hamano
2015-02-14  8:36                                           ` [msysGit] " Torsten Bögershausen
2015-02-15 23:48                                             ` Junio C Hamano
2015-01-23 21:24                                       ` Torsten Bögershausen [this message]
2015-01-23 23:02                                         ` Junio C Hamano
2015-01-24  9:41                                         ` [msysGit] " Johannes Schindelin
2015-01-16 18:38                           ` Kyle J. McKay
2015-01-16 18:38                         ` Kyle J. McKay
2015-01-16 20:04                           ` Achim Gratz
2015-01-27  1:44                   ` t5539 broken under Mac OS X Erik Faye-Lund
2015-01-27  2:51                     ` Junio C Hamano
2015-01-27 16:35                       ` Erik Faye-Lund

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=54C2BC01.2030307@web.de \
    --to=tboegi@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mackyle@gmail.com \
    --cc=msysgit@googlegroups.com \
    --cc=peff@peff.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 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 NNTP newsgroup(s).