22350 2008-11-18 19:44:39 -0800 Allow layout tests to run in php cgi mode 2008-11-25 00:58:12 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 deanm webkit-unassigned ap pam pinkerton oldest_to_newest 99269 0 deanm 2008-11-18 19:44:39 -0800 There are a bunch of layout tests written in php that call getallheaders(). This is only implemented for mod_php, and is really implemented by apache. For Chromium, we're running php through lighttpd. I wrote an shim to that gets us by, however it's a pain to have everyone update their php.ini files. I think it shouldn't be too hard to modify the php layout tests to access headers in a way that works under both mod_php and php as a CGI. Btw, here is my shim, which might help to illustrate what I'm talking about. <?php // This is a compat shim to make our php-cgi act more like apache mod_php. // http://www.qijoo.com/fapm/PHP/en/function.getallheaders.html // Well, sort of, lighttpd gives us headers like HTTP_UPPERCASE_WEE, and so // we do some ugly php to make that Uppercase-Wee... function getallheaders() { foreach($_SERVER as $name => $value) { if(substr($name, 0, 5) == 'HTTP_') { $name = strtolower(substr($name, 5)); $name = join("-", array_map('ucwords', explode("_", $name))); $headers[$name] = $value; } } return $headers; } ?> 99271 1 25258 deanm 2008-11-18 20:14:55 -0800 Created attachment 25258 First stab at removing apache only PHP getallheaders() calls. I haven't tested this, but I think something along these lines should work. 99281 2 eric 2008-11-19 00:14:10 -0800 Why do we use lighthttpd for our testing again? Nothing against lighty, but it seems we should just pick a single web server and make that standard for all of WebKit/Chromium testing, unless there are compelling reasons to use different ones for each project. 99283 3 deanm 2008-11-19 00:20:46 -0800 The short story is that apache cygwin has a lot of problems. Windows apache (but not on cygwin) doesn't work well with cygwin, think about perl scripts in the layout tests, etc... If we wanted to put a lot of effort into it, we may be able to switch back over to Apache, but it is really much more involved than you might think. Lighttp has turned out to be much faster and easier to use, bundle, configure, etc, especially now that we're running tests on 3 platforms. The getallheaders() fix is trivial, so I shouldn't see any problems with it. 99295 4 ap 2008-11-19 04:54:42 -0800 Another option would be to switch ToT WebKit to lighttpd for tests, but I think there's value in using "the real thing". 99423 5 deanm 2008-11-19 20:07:22 -0800 I realize it would much better if everyone ran Apache. We have a bit of a complicated setup, and we have tried very hard to make our checkout hermetic. Because of this, we have a little hermetic cygwin, etc. Long story short, we were having lots of problems with Apache deadlocking in our configuration, and moving to lighttpd has made everything much better. Originally it was just to fix the Apache on Windows issues, but now I think lighttpd is much simpler overall for all of our platforms. Now background aside, I think this PHP change makes sense anyway. The $_SERVER variable should be preferred over getallheaders(). Lots of PHP code is written this way, and it supports both configurations of mod_php and CGI. It also makes the code significantly simpler, as you can just dictionary address and get the header you're interested in. Let me know if you think there are any issues with the patch. The fact we're using lighttpd is something we're not going to be able to change for now. Thanks -- dean 99435 6 25258 ap 2008-11-20 00:09:39 -0800 Comment on attachment 25258 First stab at removing apache only PHP getallheaders() calls. r=me, assuming you've verified that this works with Apache 1.x, too (or at least keep an eye on buildbots after landing). 100147 7 deanm 2008-11-24 20:52:42 -0800 Things look OK from my side, but I'm probably not running the tests the same way you guys are. Could you patch in this change and just do a run of LayoutTests/http/ to make sure? I think it's ready to be committed, if there are any problems on the bots obviously roll it back and I'll work on it. Thanks 100182 8 ap 2008-11-25 00:58:12 -0800 Committed as <http://trac.webkit.org/changeset/38749>. I only ran tests on Leopard, keeping an eye on Tiger build bots... I wrote a ChangeLog for this patch, but could you please include one in your future patches? 25258 2008-11-18 20:14:55 -0800 2008-11-20 00:09:39 -0800 First stab at removing apache only PHP getallheaders() calls. z.diff text/plain 2906 deanm ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2h0dHAvdGVzdHMvbWlzYy9yZWZyZXNoLWhlYWRlcnMu cGhwIGIvTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9taXNjL3JlZnJlc2gtaGVhZGVycy5waHAKaW5k ZXggZTJkMDU1ZS4uZDc1NTZiZiAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9t aXNjL3JlZnJlc2gtaGVhZGVycy5waHAKKysrIGIvTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9taXNj L3JlZnJlc2gtaGVhZGVycy5waHAKQEAgLTEsMTkgKzEsMTYgQEAKIDw/cGhwCiAgICRnb3RNYXhB Z2U9ZmFsc2U7CiAgICRnb3ROb0NhY2hlPWZhbHNlOwotICAkaGVhZGVycyA9IGdldGFsbGhlYWRl cnMoKTsgCiAKLSAgZm9yZWFjaCAoJGhlYWRlcnMgYXMgJG5hbWUgPT4gJGNvbnRlbnQpIHsKLSAg ICBpZiAoMCA9PSBzdHJjYXNlY21wKCRuYW1lLCAiQ2FjaGUtQ29udHJvbCIpICYmIDAgPT0gc3Ry Y2FzZWNtcCgkY29udGVudCwgIm1heC1hZ2U9MCIpKQotICAgIHsKLSAgICAgICRnb3RNYXhBZ2Ug PSB0cnVlOwotICAgIH0KKyAgaWYgKDAgPT0gc3RyY2FzZWNtcCgkX1NFUlZFUlsiSFRUUF9DQUNI RV9DT05UUk9MIl0sICJtYXgtYWdlPTAiKSkKKyAgeworICAgICRnb3RNYXhBZ2UgPSB0cnVlOwor ICB9CiAgICAgCi0gICAgaWYgKCgwID09IHN0cmNhc2VjbXAoJG5hbWUsICJDYWNoZS1Db250cm9s IikgJiYgMCA9PSBzdHJjYXNlY21wKCRjb250ZW50LCAibm8tY2FjaGUiKSkgfHwKLSAgICAgICAg KDAgPT0gc3RyY2FzZWNtcCgkbmFtZSwgIlByYWdtYSIpICYmIDAgPT0gc3RyY2FzZWNtcCgkY29u dGVudCwgIm5vLWNhY2hlIikpKQotICAgIHsKLSAgICAgICRnb3ROb0NhY2hlID0gdHJ1ZTsKLSAg ICB9CisgIGlmICgoMCA9PSBzdHJjYXNlY21wKCRfU0VSVkVSWyJIVFRQX0NBQ0hFX0NPTlRST0wi XSwgIm5vLWNhY2hlIikpIHx8CisgICAgICAoMCA9PSBzdHJjYXNlY21wKCRfU0VSVkVSWyJIVFRQ X1BSQUdNQSJdLCAibm8tY2FjaGUiKSkpCisgIHsKKyAgICAkZ290Tm9DYWNoZSA9IHRydWU7CiAg IH0KICAgCiAgIGlmICgkZ290Tm9DYWNoZSkgewpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvaHR0 cC90ZXN0cy9taXNjL3Jlc291cmNlcy9yZWRpcmVjdC1yZXN1bHQucGhwIGIvTGF5b3V0VGVzdHMv aHR0cC90ZXN0cy9taXNjL3Jlc291cmNlcy9yZWRpcmVjdC1yZXN1bHQucGhwCmluZGV4IGUwODFl MzQuLjQwYzMxZDUgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL2h0dHAvdGVzdHMvbWlzYy9yZXNv dXJjZXMvcmVkaXJlY3QtcmVzdWx0LnBocAorKysgYi9MYXlvdXRUZXN0cy9odHRwL3Rlc3RzL21p c2MvcmVzb3VyY2VzL3JlZGlyZWN0LXJlc3VsdC5waHAKQEAgLTQsNyArNCw3IEBACiBpZiAod2lu ZG93LmxheW91dFRlc3RDb250cm9sbGVyKQogICAgIGxheW91dFRlc3RDb250cm9sbGVyLmR1bXBB c1RleHQoKTsKIAotdmFyIHVzZXJBZ2VudCA9ICI8P3BocCAkaGVhZGVycyA9IGdldGFsbGhlYWRl cnMoKTsgZWNobyAkaGVhZGVyc1siVXNlci1BZ2VudCJdOyA/PiI7Cit2YXIgdXNlckFnZW50ID0g Ijw/cGhwIGVjaG8gJF9TRVJWRVJbJ0hUVFBfVVNFUl9BR0VOVCddOyA/PiI7CiBpZiAodXNlckFn ZW50Lm1hdGNoKC9XZWJLaXQvKSkgewogICAgIGRvY3VtZW50LndyaXRlKCJQQVNTOiBVc2VyLUFn ZW50IGhlYWRlciBjb250YWlucyB0aGUgc3RyaW5nICdXZWJLaXQnLiIpOwogfSBlbHNlIHsKZGlm ZiAtLWdpdCBhL0xheW91dFRlc3RzL2h0dHAvdGVzdHMvbWlzYy9yZXNvdXJjZXMvcmVmZXJyZXIt cmVzdWx0LnBocCBiL0xheW91dFRlc3RzL2h0dHAvdGVzdHMvbWlzYy9yZXNvdXJjZXMvcmVmZXJy ZXItcmVzdWx0LnBocAppbmRleCAxNjg5YzRmLi4xMzViZjEwIDEwMDY0NAotLS0gYS9MYXlvdXRU ZXN0cy9odHRwL3Rlc3RzL21pc2MvcmVzb3VyY2VzL3JlZmVycmVyLXJlc3VsdC5waHAKKysrIGIv TGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9taXNjL3Jlc291cmNlcy9yZWZlcnJlci1yZXN1bHQucGhw CkBAIC0yLDcgKzIsNyBAQAogPGJvZHk+CiA8c2NyaXB0PgogCi12YXIgcmVmZXJlciA9ICI8P3Bo cCAkaGVhZGVycyA9IGdldGFsbGhlYWRlcnMoKTsgZWNobyAkaGVhZGVyc1siUmVmZXJlciJdOyA/ PiI7Cit2YXIgcmVmZXJlciA9ICI8P3BocCBlY2hvICRfU0VSVkVSWyJIVFRQX1JFRkVSRVIiXTsg Pz4iOwogaWYgKHJlZmVyZXIubWF0Y2goL3JlZmVycmVyLmh0bWwvKSkgewogICAgIGRvY3VtZW50 LndyaXRlKCJQQVNTOiBSZWZlcmVyIGhlYWRlciBleGlzdHMgYW5kIGNvbnRhaW5zIHRoZSBzdHJp bmcgJ3JlZmVycmVyLmh0bWwnLiIpOwogfSBlbHNlIHsKZGlmZiAtLWdpdCBhL0xheW91dFRlc3Rz L2h0dHAvdGVzdHMvbWlzYy94aHRtbC5waHAgYi9MYXlvdXRUZXN0cy9odHRwL3Rlc3RzL21pc2Mv eGh0bWwucGhwCmluZGV4IDJiOGFhZjQuLjQ2NTg5M2UgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3Rz L2h0dHAvdGVzdHMvbWlzYy94aHRtbC5waHAKKysrIGIvTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9t aXNjL3hodG1sLnBocApAQCAtNCw3ICs0LDcgQEAKIGlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRy b2xsZXIpCiAgICAgbGF5b3V0VGVzdENvbnRyb2xsZXIuZHVtcEFzVGV4dCgpOwogCi12YXIgYWNj ZXB0ID0gIjw/cGhwICRoZWFkZXJzID0gZ2V0YWxsaGVhZGVycygpOyBlY2hvICRoZWFkZXJzWyJB Y2NlcHQiXTsgPz4iOwordmFyIGFjY2VwdCA9ICI8P3BocCBlY2hvICRfU0VSVkVSWyJIVFRQX0FD Q0VQVCJdOyA/PiI7CiBkb2N1bWVudC53cml0ZSgiQWNjZXB0OiAiICsgYWNjZXB0ICsgIjxicj48 YnI+Iik7CiBpZiAoYWNjZXB0Lm1hdGNoKC9hcHBsaWNhdGlvblwveGh0bWxcK3htbC8pKSB7CiAg ICAgZG9jdW1lbnQud3JpdGUoIlBBU1M6IFRoZSBicm93c2VyIGFza3MgZm9yIFhIVE1MLiIpOwo=