Summary: | [Win] http server is not starting. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Per Arne Vollan <pvollan> | ||||||
Component: | Tools / Tests | Assignee: | Per Arne Vollan <pvollan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, ap, bfulgham, commit-queue, glenn, lforschler, ossy | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 161047 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Per Arne Vollan
2016-08-24 04:02:41 PDT
Created attachment 286847 [details]
Patch
Comment on attachment 286847 [details]
Patch
Looks good. It's a shame we can't keep Windows and other OS's more consistent here.
Do you need to update the XAMPP 'php.ini' file to match what was done on macOS? Or perhaps Windows is running an older PHP that doesn't need it? #### ; Let <meta charset> work. A charset provided in Content-Type takes precedence, and PHP 5.6+ defaults to UTF-8. default_charset = "" #### Comment on attachment 286847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286847&action=review > Tools/ChangeLog:8 > + Avoid setting php ini folder twice, it has been set in http.conf. Is the line in http.conf needed? It seems wrong to have it, but then to override globally. (In reply to comment #4) > Comment on attachment 286847 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=286847&action=review > > > Tools/ChangeLog:8 > > + Avoid setting php ini folder twice, it has been set in http.conf. > > Is the line in http.conf needed? It seems wrong to have it, but then to > override globally. PerArne: What happens if we remove the PHPINIDir line from the Windows apache configuration file? I think the php.conf in XAMPP is just the default setup, but I haven't investigated this myself. Can you try that before landing this change? Comment on attachment 286847 [details]
Patch
Brent looked at the XAMPP php.ini, and looks like there are things in it that are needed.
Created attachment 286866 [details]
Patch
Thanks for reviewing! I agree with your comments, I have updated the patch :) (In reply to comment #5) > (In reply to comment #4) > > Comment on attachment 286847 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=286847&action=review > > > > > Tools/ChangeLog:8 > > > + Avoid setting php ini folder twice, it has been set in http.conf. > > > > Is the line in http.conf needed? It seems wrong to have it, but then to > > override globally. > > PerArne: What happens if we remove the PHPINIDir line from the Windows > apache configuration file? I think the php.conf in XAMPP is just the default > setup, but I haven't investigated this myself. > > Can you try that before landing this change? I tried this, and it actually seems to fix some http tests :) Comment on attachment 286866 [details]
Patch
r=me given that it works :)
Comment on attachment 286866 [details] Patch Clearing flags on attachment: 286866 Committed r204907: <http://trac.webkit.org/changeset/204907> All reviewed patches have been landed. Closing bug. |