Bug 22350 – Allow layout tests to run in php cgi mode

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Dean McNamee]

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;
}
?>

[Dean McNamee]

Created attachment 25258 [details]
First stab at removing apache only PHP getallheaders() calls.

I haven't tested this, but I think something along these lines should work.

[Eric Seidel (no email)]

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.

[Dean McNamee]

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.

[Alexey Proskuryakov]

Another option would be to switch ToT WebKit to lighttpd for tests, but I think there's value in using "the real thing".

[Dean McNamee]

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

[Alexey Proskuryakov]

Comment on attachment 25258 [details]
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).

[Dean McNamee]

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

[Alexey Proskuryakov]

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?