Bug 236938 – Python3 for lighttpd

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Pascal Abresch]

Python3 support should be enabled for lighttpd, the Haiku port currently uses lighttpd to run the tests.

[Pascal Abresch]

Created attachment 452689 [details]
Patch

[Jonathan Bedard]

Comment on attachment 452689 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=452689&action=review

> Tools/Scripts/webkitpy/layout_tests/servers/http_server.py:114
> +                 '               ".php"  => "%s"\n'

Worth noting that we won’t be adding PHP tests, and all existing tests have been replaced with Python. You should be able to remove PHP entirely, should you desire.

[Pascal Abresch]

Good day,
I don't mind much either way, I can submit a patch that replaces php instead.

[Adrien Destugues]

Yes, replacing php seems better and makes it clearer that it is not needed anymore.

[Pascal Abresch]

Created attachment 453304 [details]
Patch

[Darin Adler]

Comment on attachment 453304 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=453304&action=review

> Tools/Scripts/webkitpy/layout_tests/servers/http_server.py:-115
> -                 '               ".php"  => "%s" )\n\n') %
> -                                     self._port_obj._path_to_lighttpd_php())

Change log doesn’t mention why we’re removing ".php". Maybe a good idea, but why? Also, why not remove the _path_to_lighttpd_php function too?

> Tools/Scripts/webkitpy/layout_tests/servers/lighttpd.conf:41
> -static-file.exclude-extensions = ( ".php", ".pl", ".cgi" )
> +static-file.exclude-extensions = ( ".py", ".pl", ".cgi" )

Same question: What’s the rationale for removing ".php"? Also, not great to leave the mention of it in the comment on the line above.

[Darin Adler]

Comment on attachment 453304 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=453304&action=review

>> Tools/Scripts/webkitpy/layout_tests/servers/http_server.py:-115
>> -                                     self._port_obj._path_to_lighttpd_php())
> 
> Change log doesn’t mention why we’re removing ".php". Maybe a good idea, but why? Also, why not remove the _path_to_lighttpd_php function too?

I see Jonathan Bedard advising you to remove php, so all you need to do is to mention this in the change log. And I do think we should remove the _path_to_lighttpd_php function and any other php-only code.

[Pascal Abresch]

Created attachment 453308 [details]
Patch

[Pascal Abresch]

I've updated the patch, it should be clearer now.

[Darin Adler]

Comment on attachment 453308 [details]
Patch

r=me, but as usual, best to wait for EWS results before we set commit-queue+

[Pascal Abresch]

I don't think any port but haiku uses lighttpd for tests currently, so it may be broken even if EWS passes. I don't currently have a reference to test against for this. (The haiku port already patches this file differently so not a straightforward replacement to test with)

[Darin Adler]

OK, we’ll just use EWS to verify that this change, which should not affect the ports in EWS, does not affect them.

[Radar WebKit Bug Importer]

<rdar://problem/89525957>

[Pascal Abresch]

EWS passed, it should be ready to commit now.

[Jonathan Bedard]

(In reply to Pascal Abresch from comment #14)
> EWS passed, it should be ready to commit now.

Ok! Sending through commit-queue now.

[EWS]

Committed r290599 (247875@main): <https://commits.webkit.org/247875@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453308 [details].