On Wed, Aug 4, 2010 at 19:13, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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" ;-) 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html