* [PATCH 0/3] running tests as fakeroot
@ 2010-04-21 13:12 Jonathan Nieder
2010-04-21 13:38 ` [PATCH 1/3] test-lib: tests can have multiple prerequisites Jonathan Nieder
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jonathan Nieder @ 2010-04-21 13:12 UTC (permalink / raw)
To: git
Cc: Sylvain Rabot, Robin H. Johnson, Fernando J. Pereda, Lea Wiemann,
Panagiotis Issaris, Mike Hommey, Marco Nelissen
Hi,
The Debian build system makes it very tempting to build as
fakeroot: basically, one has a choice of
debian/rules build
fakeroot debian/rules binary
or just
fakeroot debian/rules binary
to build a .deb from the current sources. I hadn’t succumbed
to temptation until today; but now that I have, I want to fix it.
I’ll discuss whether this is a good idea more in patch 3.
Patches 1 and 2 are a bit simpler. Patch 1 allows tests to have
multiple prerequisites, which seems like a good idea generally;
patch 2 adds a missing POSIXPERM prerequisite to an existing test.
I hope you like the patches. Sorry for the huge cc list; I did a
quick search to find people interested one way or another, and
aparently there are many. I am sending patches 1 and 2 only to the
git list to avoid making the problem worse.
Thoughts?
Jonathan Nieder (3):
test-lib: tests can have multiple prerequisites
t1004 (read-tree): the unremovable symlink test requires POSIXPERM
Permit tests to be run as a (fake) root user
t/t1004-read-tree-m-u-wf.sh | 2 +-
t/test-lib.sh | 22 ++++++++++++++--------
2 files changed, 15 insertions(+), 9 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] test-lib: tests can have multiple prerequisites
2010-04-21 13:12 [PATCH 0/3] running tests as fakeroot Jonathan Nieder
@ 2010-04-21 13:38 ` Jonathan Nieder
2010-04-21 14:57 ` Jonathan Nieder
2010-04-26 19:17 ` Jeff King
2010-04-21 13:44 ` [PATCH 2/3] t1004 (read-tree): the unremovable symlink test requires POSIXPERM Jonathan Nieder
2010-04-21 14:20 ` [PATCH 3/3] Permit tests to be run as a (fake) root user Jonathan Nieder
2 siblings, 2 replies; 8+ messages in thread
From: Jonathan Nieder @ 2010-04-21 13:38 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Don Slutz, Jeff King
Treat the prereq argument to test_expect_success and
test_expect_failure as a space-separated list of prerequisites.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Alas, I didn’t notice the thread with the almost identical patch [1]
before writing this one.
Since I am painting it, I prefer this way. I find it intuitive and
can’t really see where the fuss about using some other coding style
came from. So I would be very happy if someone who does understand
the fuss could come up with a comment to add to the commit message
about this. :)
t/test-lib.sh | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c582964..a53b6cf 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -315,12 +315,15 @@ test_set_prereq () {
satisfied=" "
test_have_prereq () {
- case $satisfied in
- *" $1 "*)
- : yes, have it ;;
- *)
- ! : nope ;;
- esac
+ for prereq_to_test
+ do
+ case $satisfied in
+ *" $prereq_to_test "*)
+ : yes, have it ;;
+ *)
+ return 1 ;;
+ esac
+ done
}
# You are not expected to call test_ok_ and test_failure_ directly, use
@@ -370,7 +373,7 @@ test_skip () {
esac
done
if test -z "$to_skip" && test -n "$prereq" &&
- ! test_have_prereq "$prereq"
+ ! test_have_prereq $prereq
then
to_skip=t
fi
--
1.7.1.rc1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] t1004 (read-tree): the unremovable symlink test requires POSIXPERM
2010-04-21 13:12 [PATCH 0/3] running tests as fakeroot Jonathan Nieder
2010-04-21 13:38 ` [PATCH 1/3] test-lib: tests can have multiple prerequisites Jonathan Nieder
@ 2010-04-21 13:44 ` Jonathan Nieder
2010-04-21 14:20 ` [PATCH 3/3] Permit tests to be run as a (fake) root user Jonathan Nieder
2 siblings, 0 replies; 8+ messages in thread
From: Jonathan Nieder @ 2010-04-21 13:44 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Now that it is possible to express this, declare the test from
8a785dc (Add tests to catch problems with un-unlinkable symlinks,
2008-03-18) to require both POSIXPERM and SYMLINKS.
This test would fail on Windows on filesystems that do support
symbolic links and in other situations where symlinks are usable but
Unix-style permissions are not.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
It seems that an almost identical patch was submitted while the
prerequisite mechanism was in its infancy.
http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118385
t/t1004-read-tree-m-u-wf.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh
index f19b4a2..62854a7 100755
--- a/t/t1004-read-tree-m-u-wf.sh
+++ b/t/t1004-read-tree-m-u-wf.sh
@@ -177,7 +177,7 @@ test_expect_success SYMLINKS 'funny symlink in work tree' '
'
-test_expect_success SYMLINKS 'funny symlink in work tree, un-unlink-able' '
+test_expect_success 'POSIXPERM SYMLINKS' 'funny symlink in work tree, un-unlink-able' '
rm -fr a b &&
git reset --hard &&
--
1.7.1.rc1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] Permit tests to be run as a (fake) root user
2010-04-21 13:12 [PATCH 0/3] running tests as fakeroot Jonathan Nieder
2010-04-21 13:38 ` [PATCH 1/3] test-lib: tests can have multiple prerequisites Jonathan Nieder
2010-04-21 13:44 ` [PATCH 2/3] t1004 (read-tree): the unremovable symlink test requires POSIXPERM Jonathan Nieder
@ 2010-04-21 14:20 ` Jonathan Nieder
2 siblings, 0 replies; 8+ messages in thread
From: Jonathan Nieder @ 2010-04-21 14:20 UTC (permalink / raw)
To: git
Cc: Sylvain Rabot, Robin H. Johnson, Fernando J. Pereda, Lea Wiemann,
Panagiotis Issaris, Mike Hommey, Marco Nelissen
Skip tests that require sane behavior wrt permissions if the current
uid is 0.
So now if you run
fakeroot sh -c '
make all test install install-doc prefix=/usr/local \
DESTDIR=wherever >packaged-git.log 2>&1 &&
cd wherever &&
tar -cf - . |
gzip
' >packaged-git.tar.gz
then this should successfully build a binary tarball for git, running
some tests while at it.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Goals
-----
Before the POSIXPERM mechanism existed (which makes this change too
easy to avoid considering), I think many people wanted to discourage
running tests as root to avoid hosing people’s machines. Which makes
some sense.
But it is possible and may even be valuable to run tests in a
sandbox. [1] [8] If that were the only reason to run tests as root, I
would suggest requiring some particular environment variable to be
set as a safety.
In Debian, I think it might be policy to allow building as “root”. In
this context, that almost always happens through some sort of ptrace or
LD_PRELOAD hack like fakeroot. If a user knows to, it is easy to fall
back to building as an unprivileged user, but really I think a user
should not have to know to. [2] [3] If some non-POSIXPERM test fails
when run as (fake) root, that is something that would be nice to
know. [4]
Which prerequisite?
-------------------
It is possible to use autoconf-style tests to check for each
capability we would like to lack. [5] Certainly, POSIXPERM does not
actually describe the exact set of abilities one gains by running as
an unprivileged user. [6] But unsetting POSIXPERM does successfully
convey a simple truth: if you run tests as root, you have lost the ability
to pay attention to permissions and keep your sanity. [7]
[1] http://thread.gmane.org/gmane.comp.version-control.git/86885/focus=87664
[2] http://thread.gmane.org/gmane.comp.version-control.git/140128
[3] http://thread.gmane.org/gmane.comp.version-control.git/17904/focus=17910
[4] http://thread.gmane.org/gmane.comp.version-control.git/75561/focus=75568
[5] http://thread.gmane.org/gmane.comp.version-control.git/18667
[6] http://thread.gmane.org/gmane.comp.version-control.git/121502/focus=121531
[7] http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118385
[8] http://thread.gmane.org/gmane.comp.version-control.git/52654/focus=52689
Holy cow, that Solaris bug is still not fixed [9]. Maybe it
would be worthwhile to make an unlink() wrapper in compat/ to
avoid new uses leaving users on such operating systems in danger.
[9] http://bugs.opensolaris.org/view_bug.do?bug_id=6581318
Thanks for reading. I hope the story was not too dull.
t/test-lib.sh | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a53b6cf..0f51757 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -782,7 +782,10 @@ case $(uname -s) in
# exec does not inherit the PID
;;
*)
- test_set_prereq POSIXPERM
+ if test $(id -u) != 0
+ then
+ test_set_prereq POSIXPERM
+ fi
test_set_prereq BSLASHPSPEC
test_set_prereq EXECKEEPSPID
;;
--
1.7.1.rc1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] test-lib: tests can have multiple prerequisites
2010-04-21 13:38 ` [PATCH 1/3] test-lib: tests can have multiple prerequisites Jonathan Nieder
@ 2010-04-21 14:57 ` Jonathan Nieder
2010-04-26 19:17 ` Jeff King
1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Nieder @ 2010-04-21 14:57 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Don Slutz, Jeff King
Jonathan Nieder wrote:
> Treat the prereq argument to test_expect_success and
> test_expect_failure as a space-separated list of prerequisites.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Alas, I didn’t notice the thread with the almost identical patch [1]
> before writing this one.
>
> Since I am painting it, I prefer this way. I find it intuitive and
> can’t really see where the fuss about using some other coding style
> came from.
Whoops, forgot to link. Maybe this would be a good entry point:
[1] http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118595
Sorry about that.
Jonathan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] test-lib: tests can have multiple prerequisites
2010-04-21 13:38 ` [PATCH 1/3] test-lib: tests can have multiple prerequisites Jonathan Nieder
2010-04-21 14:57 ` Jonathan Nieder
@ 2010-04-26 19:17 ` Jeff King
2010-04-27 0:06 ` Junio C Hamano
1 sibling, 1 reply; 8+ messages in thread
From: Jeff King @ 2010-04-26 19:17 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Johannes Sixt, Don Slutz
On Wed, Apr 21, 2010 at 08:38:06AM -0500, Jonathan Nieder wrote:
> Alas, I didn’t notice the thread with the almost identical patch [1]
> before writing this one.
>
> Since I am painting it, I prefer this way. I find it intuitive and
> can’t really see where the fuss about using some other coding style
> came from. So I would be very happy if someone who does understand
> the fuss could come up with a comment to add to the commit message
> about this. :)
I liked commas better, but since you are doing the work of writing the
patches, I will let it go. :)
I do wonder if it might be less error-prone to have:
test_expect_success PREREQ1 PREREQ2 'desc' 'test'
instead of
test_expect_success 'PREREQ1 PREREQ2' 'desc' 'test'
which should just be something like
while test $# -gt 2; do
test_have_prereq "$1" || skip=t
shift
done
But if you think that's dumb, just ignore it. It is really not worth
spending that much time on.
-Peff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] test-lib: tests can have multiple prerequisites
2010-04-26 19:17 ` Jeff King
@ 2010-04-27 0:06 ` Junio C Hamano
2010-04-27 1:25 ` Jeff King
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2010-04-27 0:06 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, git, Johannes Sixt, Don Slutz
Jeff King <peff@peff.net> writes:
> I do wonder if it might be less error-prone to have:
>
> test_expect_success PREREQ1 PREREQ2 'desc' 'test'
>
> instead of
>
> test_expect_success 'PREREQ1 PREREQ2' 'desc' 'test'
Spot what is wrong with this:
test_expect_success make sure we have the repo '
test -d .git
'
I think "we may or may not have a single prereq word" would be less error
prone. (you could make sure "test_have_prereq" gets no lowercase letter
to mitigate the issue, though).
I tend to like the comma myself, but quoted, space separated list is fine
by me.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] test-lib: tests can have multiple prerequisites
2010-04-27 0:06 ` Junio C Hamano
@ 2010-04-27 1:25 ` Jeff King
0 siblings, 0 replies; 8+ messages in thread
From: Jeff King @ 2010-04-27 1:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, git, Johannes Sixt, Don Slutz
On Mon, Apr 26, 2010 at 05:06:55PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I do wonder if it might be less error-prone to have:
> >
> > test_expect_success PREREQ1 PREREQ2 'desc' 'test'
> >
> > instead of
> >
> > test_expect_success 'PREREQ1 PREREQ2' 'desc' 'test'
>
> Spot what is wrong with this:
>
> test_expect_success make sure we have the repo '
> test -d .git
> '
To me it's easy to spot, but there is nothing wrong with it that the
shell will spot, so that is a downside. Whatever we go with, it would
perhaps be less error prone to set "NOPREREQ1" when we don't have
PREREQ1, and check that either $1 or NO$1 is set when checking
prerequisites. But that is probably just over-engineering. This is not
an error we have seen a lot of, and it is not that hard to spot with
code review.
-Peff
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-04-27 1:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-21 13:12 [PATCH 0/3] running tests as fakeroot Jonathan Nieder
2010-04-21 13:38 ` [PATCH 1/3] test-lib: tests can have multiple prerequisites Jonathan Nieder
2010-04-21 14:57 ` Jonathan Nieder
2010-04-26 19:17 ` Jeff King
2010-04-27 0:06 ` Junio C Hamano
2010-04-27 1:25 ` Jeff King
2010-04-21 13:44 ` [PATCH 2/3] t1004 (read-tree): the unremovable symlink test requires POSIXPERM Jonathan Nieder
2010-04-21 14:20 ` [PATCH 3/3] Permit tests to be run as a (fake) root user Jonathan Nieder
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.