Ævar Arnfjörð Bjarmason [off-list ref] writes:
Some tests depend on not being able to read files after chmod -w. This
doesn't work when running the tests as root.
Obviously you meant s/read/write/ or "chmod -r" ;-)
We discussed this prerequisite in the past as "SANITY", in the dual sense
that (1) nobody sane should be running tests as root and (2) for root many
normal assumptions programs make do not hold. If we throw out the former
by saying that it is safe to run tests under fakeroot, we would need
something like this patch to cover the latter. The patch is a step in the
right direction.
Having said that.
I wonder if we want to be so specific, as your patch does, to single out
"you can write even to a-w file" aspect of rootness, or just want to cover
the rootness more broadly so that other rooty conditions like "if you can
read even an a-r file, then the assumptions the test makes will not hold"
and "if you can kill other's processes, ...ditto..." can also be covered
with a single prerequisite token.
Also I think there was a discussion and proposed patch to support more
than one prerequisite tokens, concatenated with "," or something, like:
test_expect_success POSIXPERM,SANITY 'notice unwritable repo' '
... test that depends on posixperm and not running
... as root comes here
'
so that you don't have to invent permutations of prerequisite tokens.
Thanks.
On Wed, Aug 4, 2010 at 19:13, Junio C Hamano [off-list ref] wrote:
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
Some tests depend on not being able to read files after chmod -w. This
doesn't work when running the tests as root.
Obviously you meant s/read/write/ or "chmod -r" ;-)
Yeah. I'll fix that in a future submission. Pending the question of
whether there's any interest.
We discussed this prerequisite in the past as "SANITY", in the dual sense
that (1) nobody sane should be running tests as root and (2) for root many
normal assumptions programs make do not hold. If we throw out the former
by saying that it is safe to run tests under fakeroot, we would need
something like this patch to cover the latter. The patch is a step in the
right direction.
I sometimes run stuff like this as root, although obviously not often
enough to have bumped into this before with Git. But sometimes you
don't care about user seperation, e.g. inside a virtual machine.
When I bumped into this I was writing a script to run in cron that
would build and install pu daily in /usr/local if tests passed. Since
I needed root to install there I was running the whole script as root,
it was easier than giving the script permission to sudo or granularly
adjusting the permissions of /usr/local.
In any case, seemingly randomly breaking if the tests are run as root
is a bad thing which should be fixed.
I wonder if we want to be so specific, as your patch does, to single out
"you can write even to a-w file" aspect of rootness, or just want to cover
the rootness more broadly so that other rooty conditions like "if you can
read even an a-r file, then the assumptions the test makes will not hold"
and "if you can kill other's processes, ...ditto..." can also be covered
with a single prerequisite token.
Well, in practice it was the one and only thing that broke under
root. And it's something that can be compartmentalized easily and
tested for without cheating and checking if UID == 0.
Also I think there was a discussion and proposed patch to support more
than one prerequisite tokens, concatenated with "," or something, like:
test_expect_success POSIXPERM,SANITY 'notice unwritable repo' '
... test that depends on posixperm and not running
... as root comes here
'
so that you don't have to invent permutations of prerequisite tokens.
I almost implemented something like that when rolling this series. Do
you happen to have the Message-ID of the thread or some pertinent
search keywords? Maybe I'll resurrect it. We're using that sort of
thing in a couple of places.
I didn't implement it because I was worried about handling the 'FOO &&
BAR' and 'FOO || BAR' cases, and combinations thereof. That's probably
overthinking it though, AND-ing them together seems to be good enough,
and simple to do.
Hi Ævar,
Junio C Hamano wrote:
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
Some tests depend on not being able to read files after chmod -w. This
doesn't work when running the tests as root.
Obviously you meant s/read/write/ or "chmod -r" ;-)
We discussed this prerequisite in the past as "SANITY"
[...]
Also I think there was a discussion and proposed patch to support more
than one prerequisite tokens, concatenated with "," or something
Thanks for picking up this topic. Here’s some related work:
- Incomplete series using space-separated test prerequisites:
http://thread.gmane.org/gmane.comp.version-control.git/145427
- Why that series is incomplete:
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=104;bug=579273
- Comma-separated test prerequisites:
http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118434
- Earlier.
http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118385
test_expect_success POSIXPERM,SANITY
Last time I looked, IFS=, as used by Hannes looked like a sane
approach. I noticed too late and did not follow up; sorry about that.