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: [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: [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: [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
