Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Torsten Bögershausen tbo...@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). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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// -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
On 2015-01-22 23.07, Junio C Hamano wrote:
Torsten Bögershausen tbo...@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.
(CopyPaste 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 tbo...@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 tbo...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Torsten Bögershausen tbo...@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 tbo...@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}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Torsten Bögershausen tbo...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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 ' -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
On 2015-01-22 23.07, Junio C Hamano wrote: Torsten Bögershausen tbo...@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 ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Torsten Bögershausen tbo...@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)? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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 mack...@gmail.com Signed-off-by: Jeff King p...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Jeff King p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Junio C Hamano gits...@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 gits...@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
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
