webkitpy should accept a different httpd.conf specified by the user
Created attachment 166545 [details] Patch
Comment on attachment 166545 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166545&action=review > Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py:603 > + os.environ = saved_environ.copy() You should wrap these in a try-finally block to ensure that os.environ gets restored no matter what happens. Longer term we might want to consider enhancing the environment wrapper in webkitpy.common.system to make this more mockable. > Tools/Scripts/webkitpy/layout_tests/servers/http_server.py:103 > + base_conf_file = self._path_to_config_file() I'm not a fan of the duplication between this and the code in base.py. I'm also not sure that this is even worth doing since AFAIK only chromium uses this routine and we're not likely to set this. I appreciate your diligence, but how about we just add a FIXME to consider supporting the variable and otherwise leave the files alone?
(In reply to comment #2) > (From update of attachment 166545 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166545&action=review > > > Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py:603 > > + os.environ = saved_environ.copy() > > You should wrap these in a try-finally block to ensure that os.environ gets restored no matter what happens. Alright. > > Tools/Scripts/webkitpy/layout_tests/servers/http_server.py:103 > > + base_conf_file = self._path_to_config_file() > > I'm not a fan of the duplication between this and the code in base.py. I'm also not sure that this is even worth doing since AFAIK only chromium uses this routine and we're not likely to set this. I appreciate your diligence, but how about we just add a FIXME to consider supporting the variable and otherwise leave the files alone? I'm OK with that, but it might make my life more difficult if I want to run http tests in chromium ;)
Created attachment 166556 [details] Patch for landing
Committed r130083: <http://trac.webkit.org/changeset/130083>
Comment on attachment 166556 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=166556&action=review > Tools/ChangeLog:21 > + In the long term, we should try to generate our configuration file > + and stop requiring all the different httpd.conf files we have as > + well as this hack. Yes please!
(In reply to comment #3) > I'm OK with that, but it might make my life more difficult if I want to run http tests in chromium ;) Only on Windows, I'm assuming that's not a big concern for you. Chromium uses the Apache config on the other platforms