All of lore.kernel.org
 help / color / mirror / Atom feed
* t5539 broken under Mac OS X
@ 2015-01-14 15:39 Torsten Bögershausen
  2015-01-14 18:37 ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Torsten Bögershausen @ 2015-01-14 15:39 UTC (permalink / raw)
  To: Git Mailing List

t5539 doesn't seem to work as expected under Mac OX X 10.6
(10.9 is OK)

I am not root.
Are there any ideas how we can improve the situation, or how to debug ?


t>
t> ./t5539-fetch-http-shallow.sh ; echo $?

1..0 # SKIP Cannot run httpd tests as root
0
t>
t> GIT_TEST_HTTPD=t ./t5539-fetch-http-shallow.sh ; echo $?
error: Cannot run httpd tests as root
1
t>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  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
  0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2015-01-14 18:37 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Git Mailing List

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

> t5539 doesn't seem to work as expected under Mac OX X 10.6
> (10.9 is OK)
>
> I am not root.
> Are there any ideas how we can improve the situation, or how to debug ?

As to "how to debug", the first step is to grep for that message and
notice that it comes from here:

t/lib-httpd.sh:

    if ! test_have_prereq SANITY; then
            test_skip_or_die $GIT_TEST_HTTPD \
                    "Cannot run httpd tests as root"
    fi

and then grep for SANITY to find:

t/test-lib.sh:

    # 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

It appears that the check in lib-httpd.sh thinks you lack SANITY; is
the root directory of your system somehow writable by you?


>
>
> t>
> t> ./t5539-fetch-http-shallow.sh ; echo $?
>
> 1..0 # SKIP Cannot run httpd tests as root
> 0
> t>
> t> GIT_TEST_HTTPD=t ./t5539-fetch-http-shallow.sh ; echo $?
> error: Cannot run httpd tests as root
> 1
> t>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  2015-01-14 18:37 ` Junio C Hamano
@ 2015-01-14 19:50   ` Torsten Bögershausen
  2015-01-14 21:17     ` Jeff King
  0 siblings, 1 reply; 33+ messages in thread
From: Torsten Bögershausen @ 2015-01-14 19:50 UTC (permalink / raw)
  To: Junio C Hamano, Torsten Bögershausen; +Cc: Git Mailing List

On 2015-01-14 19.37, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> t5539 doesn't seem to work as expected under Mac OX X 10.6
>> (10.9 is OK)
>>
>> I am not root.
>> Are there any ideas how we can improve the situation, or how to debug ?
> 
> As to "how to debug", the first step is to grep for that message and
> notice that it comes from here:
> 
> t/lib-httpd.sh:
> 
>     if ! test_have_prereq SANITY; then
>             test_skip_or_die $GIT_TEST_HTTPD \
>                     "Cannot run httpd tests as root"
>     fi
> 
> and then grep for SANITY to find:
> 
> t/test-lib.sh:
> 
>     # 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
> 
> It appears that the check in lib-httpd.sh thinks you lack SANITY; is
> the root directory of your system somehow writable by you?
> 
Yes, that was a good hint, thanks.
The "problem" is that I am Admin on one machine, but not on the other,
and / was writable for the admin group for some reasons, and only on this machine.



But, why does e.g. t0004 behave more gracefully (and skips) and t5539 just dies ?

./t0004-unwritable.sh 
ok 1 - setup
ok 2 # skip write-tree should notice unwritable repository (missing SANITY of POSIXPERM,SANITY)


(And after changing the group of / t5539 passes, and so does t0004)

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  2015-01-14 19:50   ` Torsten Bögershausen
@ 2015-01-14 21:17     ` Jeff King
  2015-01-15  5:48       ` Kyle J. McKay
  0 siblings, 1 reply; 33+ messages in thread
From: Jeff King @ 2015-01-14 21:17 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Junio C Hamano, Git Mailing List

On Wed, Jan 14, 2015 at 08:50:47PM +0100, Torsten Bögershausen wrote:

> But, why does e.g. t0004 behave more gracefully (and skips) and t5539 just dies ?
> 
> ./t0004-unwritable.sh 
> ok 1 - setup
> ok 2 # skip write-tree should notice unwritable repository (missing SANITY of POSIXPERM,SANITY)

The http code uses test_skip_or_die when it runs into setup errors. The
intent there is that the user has either:

  1. Told us explicitly that they want http tests by setting
     GIT_TEST_HTTPD=true.

  2. Wants to run http tests if they can by setting GIT_TEST_HTTPD=auto
     (or leaving it unset, as that is the default).

In case (1), we treat this as a test failure. They asked for httpd
tests, and we could not run them. In case (2), we would just skip all of
the tests.

You may want to loosen your GIT_TEST_HTTPD setting (pre-83d842dc, you
had to set it to true to run the tests at all, but nowadays we have
auto).

-Peff

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  2015-01-14 21:17     ` Jeff King
@ 2015-01-15  5:48       ` Kyle J. McKay
  2015-01-15 20:29         ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Kyle J. McKay @ 2015-01-15  5:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Torsten Bögershausen, Junio C Hamano, Git Mailing List


On Jan 14, 2015, at 13:17, Jeff King wrote:
> On Wed, Jan 14, 2015 at 08:50:47PM +0100, Torsten Bögershausen wrote:
>
>> But, why does e.g. t0004 behave more gracefully (and skips) and  
>> t5539 just dies ?
>>
>> ./t0004-unwritable.sh
>> ok 1 - setup
>> ok 2 # skip write-tree should notice unwritable repository (missing  
>> SANITY of POSIXPERM,SANITY)
>
> The http code uses test_skip_or_die when it runs into setup errors.  
> The
> intent there is that the user has either:
>
>  1. Told us explicitly that they want http tests by setting
>     GIT_TEST_HTTPD=true.
>
>  2. Wants to run http tests if they can by setting GIT_TEST_HTTPD=auto
>     (or leaving it unset, as that is the default).
>
> In case (1), we treat this as a test failure. They asked for httpd
> tests, and we could not run them. In case (2), we would just skip  
> all of
> the tests.
>
> You may want to loosen your GIT_TEST_HTTPD setting (pre-83d842dc, you
> had to set it to true to run the tests at all, but nowadays we have
> auto).

I ran into this problem.  It seems like (at least on older Mac OS X)  
that the root directory is created like so:

   drwxrwxr-t  39 root  admin  /

And since the first (and likely only user) on Mac OS X is a member of  
the admin group, the SANITY test fails and complains even though  
you're not running as root (the failure message is misleading).

I ended up removing group write permission from / (which happened to  
find a bug in another script of mine) and then it was happy.

-Kyle

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  2015-01-15  5:48       ` Kyle J. McKay
@ 2015-01-15 20:29         ` Junio C Hamano
  2015-01-15 22:27           ` Jeff King
  0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2015-01-15 20:29 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: Jeff King, Torsten Bögershausen, Git Mailing List

"Kyle J. McKay" <mackyle@gmail.com> writes:

> I ran into this problem.  It seems like (at least on older Mac OS X)  
> that the root directory is created like so:
>
>    drwxrwxr-t  39 root  admin  /
>
> And since the first (and likely only user) on Mac OS X is a member of  
> the admin group, the SANITY test fails and complains even though  
> you're not running as root (the failure message is misleading).

The design choice Mac OS X makes around filesystems may deserve the
!SANITY label ;-) but we may want to tighten the check for SANITY,
or better yet, rethink the interaction between POSIXPERM and SANITY.

What we want to express with SANITY is:

	On this system, if the user who is running the test
        does not have write permission to a file, write to such a
        file would fail.

So running our tests as a non-root admin user should be labeled as
being sane.  We just use a more expedient "if you can write into the
root directory, you must be root, asit is crazy to allow non-root
user to 'mv /etc /foo && mkdir /etc && write /etc/passwd'"
heuristics which is old-school.

This should not be the final patch (I think it should become a lazy
prereq as it does a lot more), but just for testing, how does this
look?

 t/test-lib.sh | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index bb1402d..cdafab5 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1033,7 +1033,16 @@ test_lazy_prereq USR_BIN_TIME '
 
 # 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
+if test_have_prereq POSIXPERM &&
+	! test -w / &&
+	>sanitytest &&
+	chmod a= sanitytest &&
+	! (>sanitytest) 2>/dev/null &&
+	chmod +w sanitytest &&
+	rm -f sanitytest
+then
+	test_set_prereq SANITY
+fi
 
 GIT_UNZIP=${GIT_UNZIP:-unzip}
 test_lazy_prereq UNZIP '

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  2015-01-15 20:29         ` Junio C Hamano
@ 2015-01-15 22:27           ` Jeff King
  2015-01-15 22:39             ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Jeff King @ 2015-01-15 22:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kyle J. McKay, Torsten Bögershausen, Git Mailing List

On Thu, Jan 15, 2015 at 12:29:39PM -0800, Junio C Hamano wrote:

> This should not be the final patch (I think it should become a lazy
> prereq as it does a lot more), but just for testing, how does this
> look?
> 
>  t/test-lib.sh | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index bb1402d..cdafab5 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1033,7 +1033,16 @@ test_lazy_prereq USR_BIN_TIME '
>  
>  # 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
> +if test_have_prereq POSIXPERM &&
> +	! test -w / &&
> +	>sanitytest &&
> +	chmod a= sanitytest &&
> +	! (>sanitytest) 2>/dev/null &&
> +	chmod +w sanitytest &&
> +	rm -f sanitytest
> +then
> +	test_set_prereq SANITY
> +fi

The current scheme does not require POSIXPERM. Would this mean that
some platforms no longer runs SANITY tests (e.g., Windows)?

Many of the SANITY-marked tests already require both, but not all. And
certainly lib-httpd actually cares whether you are _truly_ root, not
about weird filesystem permissions. Should lib-httpd literally be
checking the output of `id` (though I can imagine that is anything but
portable)?

-Peff

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  2015-01-15 22:27           ` Jeff King
@ 2015-01-15 22:39             ` Junio C Hamano
  2015-01-15 23:57               ` Jeff King
  0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2015-01-15 22:39 UTC (permalink / raw)
  To: Jeff King; +Cc: Kyle J. McKay, Torsten Bögershausen, Git Mailing List

Jeff King <peff@peff.net> writes:

> The current scheme does not require POSIXPERM. Would this mean that
> some platforms no longer runs SANITY tests (e.g., Windows)?
>
> Many of the SANITY-marked tests already require both, but not all.

Before writing that patchlet, I briefly looked at grep output and
thought that many that are protected only by SANITY lacked POSIXPERM
by mistake:

 t/t1004-read-tree-m-u-wf.sh:test_expect_success SANITY 'funny symlink in...
 t/t3600-rm.sh 'Test that "git rm -f" fails if its rm fails'
 t/t7300-clean.sh:test_expect_success SANITY 'removal failure' '
 t/t7300-clean.sh:test_expect_success SANITY 'git clean -d with an...

All of the above relies on a working chmod as far as I can tell, so
they should require POSIXPERM,SANITY, not just SANITY.

> And
> certainly lib-httpd actually cares whether you are _truly_ root, not
> about weird filesystem permissions. Should lib-httpd literally be
> checking the output of `id` (though I can imagine that is anything but
> portable)?

Even though t/README describes SANITY to require:

  Test is not run by root user, and an attempt to write to an
  unwritable file is expected to fail correctly.

and it has been that way from day one, c91cfd19 (tests: A SANITY
test prereq for testing if we're root, 2010-08-06) is clear that
this is about "'chmod -w' is a good way to test unwritable files"

lib-httpd should, if it cares about the root-ness, be checking that
in a more direct way, "test_have_prereq RUNNING_AS_ROOT".  Making
the implementation of that portable is another matter, though.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  2015-01-15 22:39             ` Junio C Hamano
@ 2015-01-15 23:57               ` Jeff King
  2015-01-16  0:04                 ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Jeff King @ 2015-01-15 23:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kyle J. McKay, Torsten Bögershausen, Git Mailing List

On Thu, Jan 15, 2015 at 02:39:56PM -0800, Junio C Hamano wrote:

> Before writing that patchlet, I briefly looked at grep output and
> thought that many that are protected only by SANITY lacked POSIXPERM
> by mistake:
> 
>  t/t1004-read-tree-m-u-wf.sh:test_expect_success SANITY 'funny symlink in...
>  t/t3600-rm.sh 'Test that "git rm -f" fails if its rm fails'
>  t/t7300-clean.sh:test_expect_success SANITY 'removal failure' '
>  t/t7300-clean.sh:test_expect_success SANITY 'git clean -d with an...
> 
> All of the above relies on a working chmod as far as I can tell, so
> they should require POSIXPERM,SANITY, not just SANITY.

Yeah, skimming the grep output, I had the same feeling. But I did not
investigate closely.

> lib-httpd should, if it cares about the root-ness, be checking that
> in a more direct way, "test_have_prereq RUNNING_AS_ROOT".  Making
> the implementation of that portable is another matter, though.

Exactly. I am happy to submit a patch, but I cannot think of any
mechanisms besides:

  1. Calling `id`, which I suspect is very not portable.

  2. Writing a C program to check getuid(). That's portable for most
     Unixes. It looks like we already have a hacky wrapper on mingw that
     will always return "1".

Is (2) too gross?

-Peff

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  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-27  1:44                   ` t5539 broken under Mac OS X Erik Faye-Lund
  0 siblings, 2 replies; 33+ messages in thread
From: Junio C Hamano @ 2015-01-16  0:04 UTC (permalink / raw)
  To: Jeff King; +Cc: Kyle J. McKay, Torsten Bögershausen, Git Mailing List

Jeff King <peff@peff.net> writes:

> Exactly. I am happy to submit a patch, but I cannot think of any
> mechanisms besides:
>
>   1. Calling `id`, which I suspect is very not portable.
>
>   2. Writing a C program to check getuid(). That's portable for most
>      Unixes. It looks like we already have a hacky wrapper on mingw that
>      will always return "1".
>
> Is (2) too gross?

Not overly gross compared to some existing test-*.c files, I would
say.

I wondered what 'perl -e 'print $>' would say in mingw, and if that
is portable enough, though.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-16  0:04                 ` Junio C Hamano
@ 2015-01-16  1:32                   ` Jeff King
  2015-01-16  3:27                     ` Kyle J. McKay
  2015-01-27  1:44                   ` t5539 broken under Mac OS X Erik Faye-Lund
  1 sibling, 1 reply; 33+ messages in thread
From: Jeff King @ 2015-01-16  1:32 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: msysgit, Kyle J. McKay, Torsten Bögershausen, Git Mailing List

On Thu, Jan 15, 2015 at 04:04:24PM -0800, Junio C Hamano wrote:

> I wondered what 'perl -e 'print $>' would say in mingw, and if that
> is portable enough, though.

Good thinking. I guess the best way to find out is to convince somebody
from msysgit to try this patch. :)

We may simply find that nobody there even has apache installed on their
box, and they do not run the http tests at all.

-- >8 --
The SANITY prerequisite is really about whether the
filesystem will respect the permissions we set, and being
root is only one part of that. But the httpd tests really
just care about not being root, as they are trying to avoid
weirdness in apache (see a1a3011 for details).

Let's switch out SANITY for a new NOT_ROOT prerequisite,
which will let us tweak SANITY more freely.

We implement NOT_ROOT by checking perl's "$>" variable,
since we cannot rely on the "id" program being available
everywhere (and we would rather avoid writing a custom C
program to run geteuid if we can).

Note that we cannot just call this "ROOT" and ask for
"!ROOT". The possible outcomes are:

  1. we know we are root

  2. we know we are not root

  3. we could not tell, because perl was not installed or
     barfed showing us $>

We should conservatively treat (3) as "does not have the
prerequisite", which means that a naive negation would not
work.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/lib-httpd.sh | 2 +-
 t/test-lib.sh  | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index fd53b57..d154d1e 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -37,7 +37,7 @@ then
 	test_done
 fi
 
-if ! test_have_prereq SANITY; then
+if ! test_have_prereq NOT_ROOT; then
 	test_skip_or_die $GIT_TEST_HTTPD \
 		"Cannot run httpd tests as root"
 fi
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bb1402d..60020ca 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1040,3 +1040,8 @@ test_lazy_prereq UNZIP '
 	"$GIT_UNZIP" -v
 	test $? -ne 127
 '
+
+test_lazy_prereq NOT_ROOT '
+	uid=$(perl -e "print \$<") &&
+	test "$uid" != 0
+'
-- 
2.2.1.425.g441bb3c

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  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
  0 siblings, 1 reply; 33+ messages in thread
From: Kyle J. McKay @ 2015-01-16  3:27 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, msysgit, Torsten Bögershausen, Git Mailing List

On Jan 15, 2015, at 17:32, Jeff King wrote:

> On Thu, Jan 15, 2015 at 04:04:24PM -0800, Junio C Hamano wrote:
>
>> I wondered what 'perl -e 'print $>' would say in mingw, and if that
>> is portable enough, though.
>
> Good thinking. I guess the best way to find out is to convince  
> somebody
> from msysgit to try this patch. :)
>
> We may simply find that nobody there even has apache installed on  
> their
> box, and they do not run the http tests at all.
>
[...]
> We implement NOT_ROOT by checking perl's "$>" variable,
> since we cannot rely on the "id" program being available
> everywhere (and we would rather avoid writing a custom C
> program to run geteuid if we can).

Does it make a difference that id is POSIX [1]?

So the test "if [ $(id -u) = 0 ]" or similar ought to work.

"id -u" works for me in MSYS and cygwin (each appears to have it's own  
id.exe).

> +
> +test_lazy_prereq NOT_ROOT '
> +	uid=$(perl -e "print \$<") &&
> +	test "$uid" != 0
> +'

Does NO_PERL affect this?  Or is Perl always required to run the tests.

Also "$<" is real user id.  Don't you want effective user id ("$>"),  
that's what the comment says...

Both "$<" and "$>" work for me in MSYS and cygwin although if I run it  
from cmd.exe using strawberry perl, both "$<" and "$>" give 0.   
(There's no id.exe for cmd.exe unless it finds the cygwin/msys one.)

As long as NO_PERL is not also intended to affect "make test" either  
the perl or id version seems fine  to me (as long as it's Perl's "$>")  
since I doubt the tests would run with just cmd.exe. :)

-Kyle

[1] http://pubs.opengroup.org/onlinepubs/009604499/utilities/id.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  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:38                         ` Kyle J. McKay
  0 siblings, 2 replies; 33+ messages in thread
From: Jeff King @ 2015-01-16  3:34 UTC (permalink / raw)
  To: Kyle J. McKay
  Cc: Junio C Hamano, msysgit, Torsten Bögershausen, Git Mailing List

On Thu, Jan 15, 2015 at 07:27:34PM -0800, Kyle J. McKay wrote:

> >We implement NOT_ROOT by checking perl's "$>" variable,
> >since we cannot rely on the "id" program being available
> >everywhere (and we would rather avoid writing a custom C
> >program to run geteuid if we can).
> 
> Does it make a difference that id is POSIX [1]?

I don't know. Do all of the platforms where we run http tests have it
(and conforming to POSIX-ish options or output)? It may be OK to guess
yes and see if anybody complains (the worst case is skipping http
tests).

> "id -u" works for me in MSYS and cygwin (each appears to have it's own
> id.exe).

That's comforting. MSYS was the one I was most worried about. What UID
do they report? I.e., do they correctly tell us if we are root (or
more accurately, if we are not root)?

> >+test_lazy_prereq NOT_ROOT '
> >+	uid=$(perl -e "print \$<") &&
> >+	test "$uid" != 0
> >+'
> 
> Does NO_PERL affect this?  Or is Perl always required to run the tests.

No, we use a very limited subset of perl in our tests when necessary
(basic enough that any perl5 will do), regardless of the NO_PERL
setting.

> Also "$<" is real user id.  Don't you want effective user id ("$>"), that's
> what the comment says...

Yeah, I bungled this initially and thought I fixed it, but clearly not.
:-/

I'll re-roll, but if we can get away with "id -u" I think that's
preferable.

-Peff

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  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 18:38                           ` Kyle J. McKay
  2015-01-16 18:38                         ` Kyle J. McKay
  1 sibling, 2 replies; 33+ messages in thread
From: Jeff King @ 2015-01-16  9:16 UTC (permalink / raw)
  To: Kyle J. McKay
  Cc: Junio C Hamano, msysgit, Torsten Bögershausen, Git Mailing List

On Thu, Jan 15, 2015 at 10:34:46PM -0500, Jeff King wrote:

> > "id -u" works for me in MSYS and cygwin (each appears to have it's own
> > id.exe).
> 
> That's comforting. MSYS was the one I was most worried about. What UID
> do they report? I.e., do they correctly tell us if we are root (or
> more accurately, if we are not root)?

So here's a re-roll with `id -u`, as that may be the simplest way to get
people to test (with the patch applied, running t5550 as a normal user
should work, and as root should skip the tests).

-- >8 --
Subject: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT

The SANITY prerequisite is really about whether the
filesystem will respect the permissions we set, and being
root is only one part of that. But the httpd tests really
just care about not being root, as they are trying to avoid
weirdness in apache (see a1a3011 for details).

Let's switch out SANITY for a new NOT_ROOT prerequisite,
which will let us tweak SANITY more freely.

We implement NOT_ROOT by checking `id -u`, which is in POSIX
and seems to be available even on MSYS.  Note that we cannot
just call this "ROOT" and ask for "!ROOT". The possible
outcomes are:

  1. we know we are root

  2. we know we are not root

  3. we could not tell, because `id` was not available

We should conservatively treat (3) as "does not have the
prerequisite", which means that a naive negation would not
work.

Helped-by: Kyle J. McKay <mackyle@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/lib-httpd.sh | 2 +-
 t/test-lib.sh  | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index fd53b57..d154d1e 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -37,7 +37,7 @@ then
 	test_done
 fi
 
-if ! test_have_prereq SANITY; then
+if ! test_have_prereq NOT_ROOT; then
 	test_skip_or_die $GIT_TEST_HTTPD \
 		"Cannot run httpd tests as root"
 fi
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bb1402d..be50c77 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1040,3 +1040,8 @@ test_lazy_prereq UNZIP '
 	"$GIT_UNZIP" -v
 	test $? -ne 127
 '
+
+test_lazy_prereq NOT_ROOT '
+	uid=$(id -u) &&
+	test "$uid" != 0
+'
-- 
2.2.1.425.g441bb3c

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  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-16 18:38                           ` Kyle J. McKay
  1 sibling, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2015-01-16 18:32 UTC (permalink / raw)
  To: Jeff King
  Cc: Kyle J. McKay, msysgit, Torsten Bögershausen, Git Mailing List

Jeff King <peff@peff.net> writes:

> So here's a re-roll with `id -u`, as that may be the simplest way to get
> people to test (with the patch applied, running t5550 as a normal user
> should work, and as root should skip the tests).
>
> -- >8 --
> Subject: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
>
> The SANITY prerequisite is really about whether the
> filesystem will respect the permissions we set, and being
> root is only one part of that....

I checked the use of POSIXPERM that is not tied to SANITY and found
a few questionable ones (this is orthogonal from the earlier list of
glitches I mentioned, which is SANITY without POSIXPERM).

I think we will later make SANITY to require NOT_ROOT and POSIXPERM,
at which point many existing tests that require POSIXPERM,SANITY can
be simplified to require only SANITY, but that will be a follow-up
change to this fix.

-- >8 --
Subject: tests: correct misuses of POSIXPERM

POSIXPERM requires that a later call to stat(2) (hence "ls -l")
faithfully reproduces what an earlier chmod(2) did.  Some
filesystems cannot satisify this.

SANITY requires that a file or a directory is indeed accessible (or
inaccessible) when its permission bits would say it ought to be
accessible (or inaccessible).  Running tests as root would lose this
prerequisite for obvious reasons.

Fix a few tests that misuse POSIXPERM.

t0061-run-command.sh has two uses of POSIXPERM.

 - One checks that an attempt to execute a file that is marked as
   unexecutable results in a failure with EACCES; I do not think
   having root-ness or any other capability that busts the
   filesystem permission mode bits will make you run an unexecutable
   file, so this should be left as-is.  The test does not have
   anything to do with SANITY.

 - The other one expects 'git nitfol' runs the alias when an
   alias.nitfol is defined and a directory on the PATH is marked as
   unreadable and unsearchable.  I _think_ the test tries to reject
   the alternative expectation that we want to refuse to run the
   alias because it would break "no alias may mask a command" rule
   if a file 'git-nitfol' exists in the unreadable directory but we
   cannot even determine if that is the case.  Under !SANITY that
   busts the permission bits, this test no longer checks that, so it
   must be protected with SANITY.

t1509-root-worktree.sh expects to be run on a / that is writable by
the user and sees if Git behaves "sensibly" when /.git is the
repository to govern a worktree that is the whole filesystem, and
also if Git behaves "sensibly" when / itself is a bare repository
with refs, objects, and friends (I find the definition of "behaves
sensibly" under these conditions hard to fathom, but it is a
different matter).

The implementation of the test is very much problematic.

 - It requires POSIXPERM, but it does not do chmod or checks modes
   in any way.

 - It runs "rm /*" and "rm -fr /refs /objects ..." in one of the
   tests, and also does "cd / && git init --bare".  If done on a
   live system that takes advantages of the "feature" being tested,
   these obviously will clobber the system.  But there is no guard
   against such a breakage.

 - It uses "test $UID = 0" to see rootness, which now should be
   spelled "! test_have_prereq NOT_ROOT"

 t/t0061-run-command.sh   |  2 +-
 t/t1509-root-worktree.sh | 17 +++++++++++++----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 17e969d..9acf628 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -34,7 +34,7 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '
 	grep "fatal: cannot exec.*hello.sh" err
 '
 
-test_expect_success POSIXPERM 'unreadable directory in PATH' '
+test_expect_success POSIXPERM,SANITY 'unreadable directory in PATH' '
 	mkdir local-command &&
 	test_when_finished "chmod u+rwx local-command && rm -fr local-command" &&
 	git config alias.nitfol "!echo frotz" &&
diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh
index 335420f..b6977d4 100755
--- a/t/t1509-root-worktree.sh
+++ b/t/t1509-root-worktree.sh
@@ -98,8 +98,16 @@ test_foobar_foobar() {
 	'
 }
 
-if ! test_have_prereq POSIXPERM || ! [ -w / ]; then
-	skip_all="Dangerous test skipped. Read this test if you want to execute it"
+if ! test -w /
+then
+	skip_all="Test requiring writable / skipped. Read this test if you want to run it"
+	test_done
+fi
+
+if  test -e /refs || test -e /objects || test -e /info || test -e /hooks ||
+    test -e /.git || test -e /foo || test -e /me
+then
+	skip_all="Skip test that clobbers existing files in /"
 	test_done
 fi
 
@@ -108,8 +116,9 @@ if [ "$IKNOWWHATIAMDOING" != "YES" ]; then
 	test_done
 fi
 
-if [ "$UID" = 0 ]; then
-	skip_all="No you can't run this with root"
+if ! test_have_prereq NOT_ROOT
+then
+	skip_all="No you can't run this as root"
 	test_done
 fi
 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-16  3:34                       ` Jeff King
  2015-01-16  9:16                         ` Jeff King
@ 2015-01-16 18:38                         ` Kyle J. McKay
  2015-01-16 20:04                           ` Achim Gratz
  1 sibling, 1 reply; 33+ messages in thread
From: Kyle J. McKay @ 2015-01-16 18:38 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, msysgit, Torsten Bögershausen, Git Mailing List

On Jan 15, 2015, at 19:34, Jeff King wrote:

> On Thu, Jan 15, 2015 at 07:27:34PM -0800, Kyle J. McKay wrote:
>
>> "id -u" works for me in MSYS and cygwin (each appears to have it's  
>> own
>> id.exe).
>
> That's comforting. MSYS was the one I was most worried about. What UID
> do they report? I.e., do they correctly tell us if we are root (or
> more accurately, if we are not root)?

It's funny, really.  The MSYS version gives a different answer than  
the cygwin version although both are non-zero.  The MSYS perl gives  
the same answer as the MSYS id and the cygwin perl gives the same  
answer as the cygwin id.

I'm not even sure what it would mean to "be root" on one of those  
systems.

The closest I can think of would be to run as the "SYSTEM" user.  And  
that's not nearly as simple as just "sudo -s". [1].

I haven't tested that.  I will try to remember to give that a try next  
time I'm feeling the need for some frustration. ;)

-Kyle

[1] http://cygwin.com/ml/cygwin/2010-04/msg00651.html

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-16  9:16                         ` Jeff King
  2015-01-16 18:32                           ` Junio C Hamano
@ 2015-01-16 18:38                           ` Kyle J. McKay
  1 sibling, 0 replies; 33+ messages in thread
From: Kyle J. McKay @ 2015-01-16 18:38 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, msysgit, Torsten Bögershausen, Git Mailing List

On Jan 16, 2015, at 01:16, Jeff King wrote:

> Subject: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
[...]
> We implement NOT_ROOT by checking `id -u`, which is in POSIX
> and seems to be available even on MSYS.  Note that we cannot
> just call this "ROOT" and ask for "!ROOT". The possible
> outcomes are:
>
>  1. we know we are root
>
>  2. we know we are not root
>
>  3. we could not tell, because `id` was not available
>
> We should conservatively treat (3) as "does not have the
> prerequisite", which means that a naive negation would not
> work.
[...]
> +
> +test_lazy_prereq NOT_ROOT '
> +	uid=$(id -u) &&
> +	test "$uid" != 0
> +'

That looks good to me and worked as expected when I tried it.

-Kyle

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-16 18:32                           ` Junio C Hamano
@ 2015-01-16 19:02                             ` Junio C Hamano
  2015-01-17 23:35                               ` Torsten Bögershausen
  0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2015-01-16 19:02 UTC (permalink / raw)
  To: Jeff King
  Cc: Kyle J. McKay, msysgit, Torsten Bögershausen, Git Mailing List

Junio C Hamano <gitster@pobox.com> writes:

> I think we will later make SANITY to require NOT_ROOT and POSIXPERM,
> at which point many existing tests that require POSIXPERM,SANITY can
> be simplified to require only SANITY, but that will be a follow-up
> change to this fix.

And here is such a follow-up.

-- >8 --
Subject: [PATCH] tests: SANITY requires POSIXPERM

SANITY requires that a file or a directory is indeed accessible (or
inaccessible) when its permission bits would say it ought to be
accessible (or inaccessible).  Running tests as root would lose this
prerequisite for obvious reasons, and a test that requires SANITY
implies it needs POSIXPERM working.

Redefine SANITY in terms of POSIXPERM and NOT_ROOT and simplify
tests that require both to only require SANITY.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0001-init.sh          |  2 +-
 t/t0004-unwritable.sh    |  8 ++++----
 t/t0061-run-command.sh   |  2 +-
 t/t0070-fundamental.sh   |  2 +-
 t/t3700-add.sh           | 10 +++++-----
 t/t4056-diff-order.sh    |  2 +-
 t/t5537-fetch-shallow.sh |  2 +-
 t/t7508-status.sh        |  2 +-
 t/test-lib.sh            |  4 +++-
 9 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index e62c0ff..4aa8660 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -261,7 +261,7 @@ test_expect_success 'init notices EEXIST (2)' '
 	test_path_is_file newdir/a
 '
 
-test_expect_success POSIXPERM,SANITY 'init notices EPERM' '
+test_expect_success SANITY 'init notices EPERM' '
 	rm -fr newdir &&
 	mkdir newdir &&
 	chmod -w newdir &&
diff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh
index e3137d6..d5729d4 100755
--- a/t/t0004-unwritable.sh
+++ b/t/t0004-unwritable.sh
@@ -15,26 +15,26 @@ test_expect_success setup '
 
 '
 
-test_expect_success POSIXPERM,SANITY 'write-tree should notice unwritable repository' '
+test_expect_success SANITY 'write-tree should notice unwritable repository' '
 	test_when_finished "chmod 775 .git/objects .git/objects/??" &&
 	chmod a-w .git/objects .git/objects/?? &&
 	test_must_fail git write-tree
 '
 
-test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository' '
+test_expect_success SANITY 'commit should notice unwritable repository' '
 	test_when_finished "chmod 775 .git/objects .git/objects/??" &&
 	chmod a-w .git/objects .git/objects/?? &&
 	test_must_fail git commit -m second
 '
 
-test_expect_success POSIXPERM,SANITY 'update-index should notice unwritable repository' '
+test_expect_success SANITY 'update-index should notice unwritable repository' '
 	test_when_finished "chmod 775 .git/objects .git/objects/??" &&
 	echo 6O >file &&
 	chmod a-w .git/objects .git/objects/?? &&
 	test_must_fail git update-index file
 '
 
-test_expect_success POSIXPERM,SANITY 'add should notice unwritable repository' '
+test_expect_success SANITY 'add should notice unwritable repository' '
 	test_when_finished "chmod 775 .git/objects .git/objects/??" &&
 	echo b >file &&
 	chmod a-w .git/objects .git/objects/?? &&
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 9acf628..52722ee 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -34,7 +34,7 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '
 	grep "fatal: cannot exec.*hello.sh" err
 '
 
-test_expect_success POSIXPERM,SANITY 'unreadable directory in PATH' '
+test_expect_success SANITY 'unreadable directory in PATH' '
 	mkdir local-command &&
 	test_when_finished "chmod u+rwx local-command && rm -fr local-command" &&
 	git config alias.nitfol "!echo frotz" &&
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 5ed69a6..ccd88e2 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -17,7 +17,7 @@ test_expect_success 'mktemp to nonexistent directory prints filename' '
 	grep "doesnotexist/test" err
 '
 
-test_expect_success POSIXPERM,SANITY 'mktemp to unwritable directory prints filename' '
+test_expect_success SANITY 'mktemp to unwritable directory prints filename' '
 	mkdir cannotwrite &&
 	chmod -w cannotwrite &&
 	test_when_finished "chmod +w cannotwrite" &&
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index fe274e2..2bc2bcc 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -191,7 +191,7 @@ test_expect_success 'git add --refresh with pathspec' '
 	grep baz actual
 '
 
-test_expect_success POSIXPERM,SANITY 'git add should fail atomically upon an unreadable file' '
+test_expect_success SANITY 'git add should fail atomically upon an unreadable file' '
 	git reset --hard &&
 	date >foo1 &&
 	date >foo2 &&
@@ -202,7 +202,7 @@ test_expect_success POSIXPERM,SANITY 'git add should fail atomically upon an unr
 
 rm -f foo2
 
-test_expect_success POSIXPERM,SANITY 'git add --ignore-errors' '
+test_expect_success SANITY 'git add --ignore-errors' '
 	git reset --hard &&
 	date >foo1 &&
 	date >foo2 &&
@@ -213,7 +213,7 @@ test_expect_success POSIXPERM,SANITY 'git add --ignore-errors' '
 
 rm -f foo2
 
-test_expect_success POSIXPERM,SANITY 'git add (add.ignore-errors)' '
+test_expect_success SANITY 'git add (add.ignore-errors)' '
 	git config add.ignore-errors 1 &&
 	git reset --hard &&
 	date >foo1 &&
@@ -224,7 +224,7 @@ test_expect_success POSIXPERM,SANITY 'git add (add.ignore-errors)' '
 '
 rm -f foo2
 
-test_expect_success POSIXPERM,SANITY 'git add (add.ignore-errors = false)' '
+test_expect_success SANITY 'git add (add.ignore-errors = false)' '
 	git config add.ignore-errors 0 &&
 	git reset --hard &&
 	date >foo1 &&
@@ -235,7 +235,7 @@ test_expect_success POSIXPERM,SANITY 'git add (add.ignore-errors = false)' '
 '
 rm -f foo2
 
-test_expect_success POSIXPERM,SANITY '--no-ignore-errors overrides config' '
+test_expect_success SANITY '--no-ignore-errors overrides config' '
        git config add.ignore-errors 1 &&
        git reset --hard &&
        date >foo1 &&
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
index c0460bb..b7abfb2 100755
--- a/t/t4056-diff-order.sh
+++ b/t/t4056-diff-order.sh
@@ -62,7 +62,7 @@ test_expect_success 'missing orderfile' '
 	test_must_fail git diff -Obogus_file --name-only HEAD^..HEAD
 '
 
-test_expect_success POSIXPERM,SANITY 'unreadable orderfile' '
+test_expect_success SANITY 'unreadable orderfile' '
 	>unreadable_file &&
 	chmod -r unreadable_file &&
 	test_must_fail git diff -Ounreadable_file --name-only HEAD^..HEAD
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index a980574..56bead1 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -173,7 +173,7 @@ EOF
 	)
 '
 
-test_expect_success POSIXPERM,SANITY 'shallow fetch from a read-only repo' '
+test_expect_success SANITY 'shallow fetch from a read-only repo' '
 	cp -R .git read-only.git &&
 	find read-only.git -print | xargs chmod -w &&
 	test_when_finished "find read-only.git -type d -print | xargs chmod +w" &&
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 8ed5788..6037415 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1035,7 +1035,7 @@ EOF
 	test_i18ncmp expect output
 '
 
-test_expect_success POSIXPERM,SANITY 'status succeeds in a read-only repository' '
+test_expect_success SANITY 'status succeeds in a read-only repository' '
 	(
 		chmod a-w .git &&
 		# make dir1/tracked stat-dirty
diff --git a/t/test-lib.sh b/t/test-lib.sh
index b2b2ec7..37d1b0e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -999,7 +999,9 @@ 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 -w / || test_set_prereq SANITY
+test_lazy_prereq SANITY '
+	test_have_prereq POSIXPERM,NOT_ROOT
+'
 
 GIT_UNZIP=${GIT_UNZIP:-unzip}
 test_lazy_prereq UNZIP '
-- 
2.3.0-rc0-149-g0286818

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-16 18:38                         ` Kyle J. McKay
@ 2015-01-16 20:04                           ` Achim Gratz
  0 siblings, 0 replies; 33+ messages in thread
From: Achim Gratz @ 2015-01-16 20:04 UTC (permalink / raw)
  To: msysgit; +Cc: git

Kyle J. McKay writes:
>>> "id -u" works for me in MSYS and cygwin (each appears to have it's
>>> own id.exe).
>>
>> That's comforting. MSYS was the one I was most worried about. What UID
>> do they report? I.e., do they correctly tell us if we are root (or
>> more accurately, if we are not root)?

Checking for UID 0 won't work on Cygwin in the general case.  That fools
literally dozens of Perl module tests that find out the user can
actually do something they think (s)he should be unable to.

> It's funny, really.  The MSYS version gives a different answer than
> the cygwin version although both are non-zero.  The MSYS perl gives
> the same answer as the MSYS id and the cygwin perl gives the same
> answer as the cygwin id.

That result changes depending on the content /etc/passwd (which arguably
is a either a bug or a feature depending on which way you look at it).
But Windows itself doesn't have the notion of a root user at all, so
looking for one isn't going to be helpful.

> I'm not even sure what it would mean to "be root" on one of those
> systems.

It means you have the capabilities that a root user would be expected to
have.  For most intents and purposes on Windows this would mean the user
running the command is in group 544 ("Administrators" in an english
version of Windows).

> The closest I can think of would be to run as the "SYSTEM" user.  And
> that's not nearly as simple as just "sudo -s". [1].

The SYSTEM user isn't a good approximation of root under UN*X for
reasonably modern Windows versions.

http://support.microsoft.com/kb/120929

For more discussion on the UID 0 topic from a Cygwin perspective, see

http://thread.gmane.org/gmane.os.cygwin.applications/28129
http://thread.gmane.org/gmane.os.cygwin.applications/28203


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf microQ V2.22R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-16 19:02                             ` Junio C Hamano
@ 2015-01-17 23:35                               ` Torsten Bögershausen
  2015-01-21 22:33                                 ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Torsten Bögershausen @ 2015-01-17 23:35 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King
  Cc: Kyle J. McKay, msysgit, Torsten Bögershausen, Git Mailing List

Hm, being one day offline and there are lots of ideas and
new patches, I like that.
I run these test under msys and cygwin on latest pu (a3dc223ff234481356c):


./t0001-init.sh
./t0004-unwritable.sh
./t0061-run-command.sh
./t0070-fundamental.sh
./t1004-read-tree-m-u-wf.sh
./t1300-repo-config.sh
./t1301-shared-repo.sh
./t1308-config-set.sh
./t2026-prune-linked-checkouts.sh
./t3600-rm.sh
./t3700-add.sh
./t4039-diff-assume-unchanged.sh
./t4056-diff-order.sh
./t5537-fetch-shallow.sh
./t7300-clean.sh
./t7503-pre-commit-hook.sh
./t7504-commit-msg-hook.sh
./t7508-status.sh

(msys passes or skips all)

Without digging further, these fail on my cygwin:

$ grep "not ok" p.txt
not ok 29 - init notices EPERM
not ok 2 - write-tree should notice unwritable repository
not ok 3 - commit should notice unwritable repository
not ok 4 - update-index should notice unwritable repository
not ok 5 - add should notice unwritable repository
not ok 3 - mktemp to unwritable directory prints filename
not ok 13 - funny symlink in work tree, un-unlink-able
not ok 23 - proper error on non-accessible files
not ok 4 - prune directories with unreadable gitdir
not ok 15 - Test that "git rm -f" fails if its rm fails
not ok 16 - When the rm in "git rm -f" fails, it should not remove the file from the index
not ok 20 - Re-add foo and baz
not ok 21 - Modify foo -- rm should refuse
not ok 22 - Modified foo -- rm -f should work
not ok 23 - Re-add foo and baz for HEAD tests
not ok 24 - foo is different in index from HEAD -- rm should refuse
not ok 23 - git add should fail atomically upon an unreadable file
not ok 24 - git add --ignore-errors
not ok 25 - git add (add.ignore-errors)
not ok 26 - git add (add.ignore-errors = false)
not ok 27 - --no-ignore-errors overrides config
not ok 4 - unreadable orderfile
not ok 28 - removal failure
not ok 61 - status succeeds in a read-only repository

If we remove POSIXPERM from CYGWIN, all tests pass ;-)
but some are skipped :
< ok 26 - init creates a new deep directory (umask vs. shared)
< ok 3 - run_command reports EACCES
< ok 4 - unreadable directory in PATH
< ok 113 - preserves existing permissions
< ok 2 - shared=1 does not clear bits preset by umask 002
< ok 3 - shared=1 does not clear bits preset by umask 022
< ok 5 - update-server-info honors core.sharedRepository
< ok 6 - shared = 0660 (r--r-----) ro
< ok 7 - shared = 0660 (rw-rw----) rw
< ok 8 - shared = 0640 (r--r-----) ro
< ok 9 - shared = 0640 (rw-r-----) rw
< ok 10 - shared = 0600 (r--------) ro
< ok 11 - shared = 0600 (rw-------) rw
< ok 12 - shared = 0666 (r--r--r--) ro
< ok 13 - shared = 0666 (rw-rw-rw-) rw
< ok 14 - shared = 0664 (r--r--r--) ro
< ok 15 - shared = 0664 (rw-rw-r--) rw
< ok 16 - info/refs respects umask in unshared repo
< ok 17 - git reflog expire honors core.sharedRepository
< ok 18 - forced modes
< ok 4 - find-copies-harder is not confused by mode bits
< ok 10 - shallow fetch from a read-only repo
< ok 32 - git clean -d with an unreadable empty directory
< ok 7 - with non-executable hook
< ok 8 - --no-verify with non-executable hook
< ok 13 - with non-executable hook
< ok 14 - with non-executable hook (editor)
< ok 15 - --no-verify with non-executable hook
< ok 16 - --no-verify with non-executable hook (editor)

I'm not sure what is the best way forward, it seems as if CYGIN is "half POSIX" now.


-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-17 23:35                               ` Torsten Bögershausen
@ 2015-01-21 22:33                                 ` Junio C Hamano
  2015-01-22 21:51                                   ` Torsten Bögershausen
  0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2015-01-21 22:33 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

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

> Hm, being one day offline and there are lots of ideas and
> new patches, I like that.
> I run these test under msys and cygwin on latest pu (a3dc223ff234481356c):
> ...
> (msys passes or skips all)
>
> Without digging further, these fail on my cygwin:
> ...
> I'm not sure what is the best way forward, it seems as if CYGIN is "half POSIX" now.

Are you reporting differences between the state before these patches
and after, or just the fact that with these patches the named tests
break (which may or may not be broken before the patches)?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-21 22:33                                 ` Junio C Hamano
@ 2015-01-22 21:51                                   ` Torsten Bögershausen
  2015-01-22 22:07                                     ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Torsten Bögershausen @ 2015-01-22 21:51 UTC (permalink / raw)
  To: Junio C Hamano, Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

On 2015-01-21 23.33, Junio C Hamano wrote:
 > Are you reporting differences between the state before these patches
> and after, or just the fact that with these patches the named tests
> break (which may or may not be broken before the patches)?
> 
The intention was to report what is now breaking.
One example is this one:
---------------------
git.git/master:
ok 15 # skip Test that "git rm -f" fails if its rm fails (missing SANITY)

git.git/pu:
not ok 15 - Test that "git rm -f" fails if its rm fails
#    
#        chmod a-w . &&
#        test_must_fail git rm -f baz &&
#        chmod 775 .
#     

The next step could be to dig further:

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"

--------------
I can see that there are 3 groups of OS/FS combinations:
Group 1:
  File access bits are not maintained, and not obeyed.
  Typical: VFAT, Git for Windows, (and some network protocols like SAMBA,
	           depending on the OS/FS involved and/or the mount options)
  Typically core.filemode is false after "git init"

Group 2:
  File access bits are maintained and obeyed:
  POSIX/Unix/Linux/Mac OS and CYGWIN
  Typically core.filemode is true after "git init"

Group 3 :
  File access bits are maintained, but not (fully) obeyed
  running as root under Linux/Unix...
  Or Windows, when a file is allowed to be deleted from a directory without write permissions.

-----------------
In short, the following seems to be an improvement:
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
 '

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  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-01-23 21:24                                       ` [msysGit] " Torsten Bögershausen
  0 siblings, 2 replies; 33+ messages in thread
From: Junio C Hamano @ 2015-01-22 22:07 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

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.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  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-01-23 21:24                                       ` [msysGit] " Torsten Bögershausen
  1 sibling, 1 reply; 33+ messages in thread
From: Torsten Bögershausen @ 2015-01-23  6:00 UTC (permalink / raw)
  To: Junio C Hamano, Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

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?).
It feels that this is by design:
In old days under MS/DOS the only way to hinder people
from deleting a file was to make it "read only" with help
of the ATTRIB command.
https://en.wikipedia.org/wiki/ATTRIB

Later NTFS introduced the (ACL like) secutity information,
and (unless I am completely wrong) the "delete file" is part
of the "modify" permission, not write.
> 
>> 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.
> 
So true, what is a better place or way to run the test ?
Can we use /tmp  (Which may be a different file system)?
Or can we use $HOME/$$ds (Which is an artificial HOME)

We already "pollute" the $PWD here
test_lazy_prereq CASE_INSENSITIVE_FS '
  	echo good >CamelCase &&
	echo bad >camelcase &&
	test "$(cat CamelCase)" != good
'
and here:
test_lazy_prereq UTF8_NFD_TO_NFC '
....

Would 
mkdir $HOME/ds
be a better approach then ?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-22 22:07                                     ` Junio C Hamano
  2015-01-23  6:00                                       ` Torsten Bögershausen
@ 2015-01-23 21:24                                       ` Torsten Bögershausen
  2015-01-23 23:02                                         ` Junio C Hamano
  2015-01-24  9:41                                         ` [msysGit] " Johannes Schindelin
  1 sibling, 2 replies; 33+ messages in thread
From: Torsten Bögershausen @ 2015-01-23 21:24 UTC (permalink / raw)
  To: Junio C Hamano, Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

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 &&

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-23 21:24                                       ` [msysGit] " Torsten Bögershausen
@ 2015-01-23 23:02                                         ` Junio C Hamano
  2015-01-24  9:41                                         ` [msysGit] " Johannes Schindelin
  1 sibling, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2015-01-23 23:02 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

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

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

Except that we may want to be more careful to detect errors from the
initial mkdir and clean-up part (which should abort the test, not
just declare !SANITY), I think the basic idea is sound.

	test_dir=$TRASH_DIRECTORY/.sanity-test-dir
        ! mkdir "$test_dir" &&
        >"$test_dir/x" &&
        chmod -w "$test_dir" ||
	error "bug in test sript: cannot prepare .sanity-test-dir"

        rm "$test_dir/x"
        status=$?

        chmod +w "$test_dir" &&
        rm -r "$test_dir" ||
	error "bug in test sript: cannot clean .sanity-test-dir"

	return $status

or something along that line?

>
> -- >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}

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-01-23 21:24                                       ` [msysGit] " Torsten Bögershausen
  2015-01-23 23:02                                         ` Junio C Hamano
@ 2015-01-24  9:41                                         ` Johannes Schindelin
  1 sibling, 0 replies; 33+ messages in thread
From: Johannes Schindelin @ 2015-01-24  9:41 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Junio C Hamano, Jeff King, Kyle J. McKay, msysgit, Git Mailing List

On 2015-01-23 22:24, Torsten Bögershausen wrote:
> [...] either to always switch off SANITY for CYGWIN (or Windows in general).

Nice one! You gave me the chuckle for the day ;-)

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  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-27  1:44                   ` Erik Faye-Lund
  2015-01-27  2:51                     ` Junio C Hamano
  1 sibling, 1 reply; 33+ messages in thread
From: Erik Faye-Lund @ 2015-01-27  1:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Kyle J. McKay, Torsten Bögershausen, Git Mailing List

On Fri, Jan 16, 2015 at 1:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> Exactly. I am happy to submit a patch, but I cannot think of any
>> mechanisms besides:
>>
>>   1. Calling `id`, which I suspect is very not portable.
>>
>>   2. Writing a C program to check getuid(). That's portable for most
>>      Unixes. It looks like we already have a hacky wrapper on mingw that
>>      will always return "1".
>>
>> Is (2) too gross?
>
> Not overly gross compared to some existing test-*.c files, I would
> say.
>
> I wondered what 'perl -e 'print $>' would say in mingw, and if that
> is portable enough, though.

$ perl -e 'print $>'
500

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  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
  0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2015-01-27  2:51 UTC (permalink / raw)
  To: Erik Faye-Lund
  Cc: Jeff King, Kyle J. McKay, Torsten Bögershausen, Git Mailing List

Erik Faye-Lund <kusmabite@gmail.com> writes:

> On Fri, Jan 16, 2015 at 1:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jeff King <peff@peff.net> writes:
>>
>>> Exactly. I am happy to submit a patch, but I cannot think of any
>>> mechanisms besides:
>>>
>>>   1. Calling `id`, which I suspect is very not portable.
>>>
>>>   2. Writing a C program to check getuid(). That's portable for most
>>>      Unixes. It looks like we already have a hacky wrapper on mingw that
>>>      will always return "1".
>>>
>>> Is (2) too gross?
>>
>> Not overly gross compared to some existing test-*.c files, I would
>> say.
>>
>> I wondered what 'perl -e 'print $>' would say in mingw, and if that
>> is portable enough, though.
>
> $ perl -e 'print $>'
> 500

Thanks for a follow-up.

Is "id -u" not useful over there?  I ask because that is what is
used in the version tentatively queued on 'pu' for NOT_ROOT
prerequisite (the jk/sanity topic).

The SANITY prerequisite in that topic needs to be replaced with the
one from Torsten that attempts to check what we want to know in a
more direct way; i.e. "after making a directory or a file read-only,
does the filesystem really honours that, or lets us clobber?" is
what we need to know to skip some tests, and we should check that,
instead of "is / writable by us?" or "are we root?".

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: t5539 broken under Mac OS X
  2015-01-27  2:51                     ` Junio C Hamano
@ 2015-01-27 16:35                       ` Erik Faye-Lund
  0 siblings, 0 replies; 33+ messages in thread
From: Erik Faye-Lund @ 2015-01-27 16:35 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Kyle J. McKay, Torsten Bögershausen, Git Mailing List

On Tue, Jan 27, 2015 at 3:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> On Fri, Jan 16, 2015 at 1:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Jeff King <peff@peff.net> writes:
>>>
>>>> Exactly. I am happy to submit a patch, but I cannot think of any
>>>> mechanisms besides:
>>>>
>>>>   1. Calling `id`, which I suspect is very not portable.
>>>>
>>>>   2. Writing a C program to check getuid(). That's portable for most
>>>>      Unixes. It looks like we already have a hacky wrapper on mingw that
>>>>      will always return "1".
>>>>
>>>> Is (2) too gross?
>>>
>>> Not overly gross compared to some existing test-*.c files, I would
>>> say.
>>>
>>> I wondered what 'perl -e 'print $>' would say in mingw, and if that
>>> is portable enough, though.
>>
>> $ perl -e 'print $>'
>> 500
>
> Thanks for a follow-up.
>
> Is "id -u" not useful over there?  I ask because that is what is
> used in the version tentatively queued on 'pu' for NOT_ROOT
> prerequisite (the jk/sanity topic).

It's pretty much the same thing:

$ id -u
500

> The SANITY prerequisite in that topic needs to be replaced with the
> one from Torsten that attempts to check what we want to know in a
> more direct way; i.e. "after making a directory or a file read-only,
> does the filesystem really honours that, or lets us clobber?" is
> what we need to know to skip some tests, and we should check that,
> instead of "is / writable by us?" or "are we root?".

$ test -w / && echo yes
yes

$ mkdir foo && chmod a= foo
$ test -w w && echo yes
$ rm -r foo
rm: directory `foo' is write protected; descend into it anyway? n
$ rm -r foo < /dev/null
$ ls -la foo
ls: foo: No such file or directory
$

So, Windows does only kind-of respect read-only flags. Dunno if this
tells you something useful, though.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  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
  0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2015-02-12 22:36 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

So after discussing this one and queuing the resulting three-patch
series jk/sanity that consists of the three patches:

    * jk/sanity (2015-01-27) 3 commits
     - test-lib.sh: set prerequisite SANITY by testing what we really need
     - tests: correct misuses of POSIXPERM
     - t/lib-httpd: switch SANITY check for NOT_ROOT

     Waiting for ack or counter-proposal from Torsten.
     Otherwise looking good.

Do we want to proceed with these, or do we want any more work done
on them?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-02-12 22:36                                         ` Junio C Hamano
@ 2015-02-14  8:36                                           ` Torsten Bögershausen
  2015-02-15 23:48                                             ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Torsten Bögershausen @ 2015-02-14  8:36 UTC (permalink / raw)
  To: Junio C Hamano, Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

On 2015-02-12 23.36, Junio C Hamano wrote:
> So after discussing this one and queuing the resulting three-patch
> series jk/sanity that consists of the three patches:
> 
>     * jk/sanity (2015-01-27) 3 commits
>      - test-lib.sh: set prerequisite SANITY by testing what we really need
>      - tests: correct misuses of POSIXPERM
>      - t/lib-httpd: switch SANITY check for NOT_ROOT
> 
>      Waiting for ack or counter-proposal from Torsten.
>      Otherwise looking good.
> 
> Do we want to proceed with these, or do we want any more work done
> on them?
> 
I managed to run the tests with POSIXPERM and/or SANITY under
Cygwin, Msysgit, Linux, root@linux,  Mac and root@Mac.
All passed.

The work to "be done", what I can see: please amend the commit message: 
 s/more exotic//

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
  2015-02-14  8:36                                           ` [msysGit] " Torsten Bögershausen
@ 2015-02-15 23:48                                             ` Junio C Hamano
  0 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2015-02-15 23:48 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Jeff King, Kyle J. McKay, msysgit, Git Mailing List

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

> The work to "be done", what I can see: please amend the commit message: 
>  s/more exotic//

Thanks for reminding; I thought this was excised already but
apparently hasn't (yet).

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2015-02-15 23:48 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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                                       ` [msysGit] " Torsten Bögershausen
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

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.