| Summary: | [EFL][GTK] Layout Test doesn't run on Ubuntu 16.04 | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||||||
| Component: | WebKit EFL | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | achristensen, bugs-noreply, cgarcia, clopez, commit-queue, glenn, lucas.de.marchi, mcatanzaro, ossy, tonikitoo | ||||||||||||||||
| Priority: | P2 | ||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||
| Bug Blocks: | 157013 | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
Created attachment 279933 [details]
Patch
Hmm, all tests come to crash after bumpping php version from 5.0 to 7.0 :( Comment on attachment 279933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279933&action=review > LayoutTests/http/conf/debian-httpd-2.4.conf:30 > -LoadModule php5_module modules/libphp5.so > +LoadModule php7_module modules/libphp7.0.so what about GTK port? So past this change, is WebKitEFL be easily buildable in < Ubuntu 16.04? (In reply to comment #3) > what about GTK port? No idea. Someone running Ubuntu 16.04 would have to test that. I'm a Debian user. (In reply to comment #5) > (In reply to comment #3) > > what about GTK port? > > No idea. Someone running Ubuntu 16.04 would have to test that. I'm a Debian > user. AFAIK debian-httpd-2.4.conf is shared accross ports, so after this change all ports would use PHP 7 instead of PHP 5. But Tools/gtk/install-dependencies wasn't updated by this patch. Created attachment 280895 [details]
Patch
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #3) > > > what about GTK port? > > > > No idea. Someone running Ubuntu 16.04 would have to test that. I'm a Debian > > user. > > AFAIK debian-httpd-2.4.conf is shared accross ports, so after this change all > ports would use PHP 7 instead of PHP 5. But Tools/gtk/install-dependencies > wasn't updated by this patch. I upload new patch which includes GTK port. But I fail to run layout test using this patch on GTK locally. Could someone run layout test on GTK with this patch? (In reply to comment #4) > So past this change, is WebKitEFL be easily buildable in < Ubuntu 16.04? Build is fine on < Ubuntu 16.04. But layout test can't be run on < Ubuntu 16.04. (In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #3) > > > what about GTK port? > > > > No idea. Someone running Ubuntu 16.04 would have to test that. I'm a Debian > > user. > > AFAIK debian-httpd-2.4.conf is shared accross ports, so after this change all > ports would use PHP 7 instead of PHP 5. But Tools/gtk/install-dependencies > wasn't updated by this patch. The GTK bots are all running Debian 8 Jessie (stable), which still has php5. I'm also running Debian stable and I think quite a few other developers. I have tested it and it broke the layout tests on my machine and will also break all the GTK bots. Traceback (most recent call last): File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 77, in main run_details = run(port, options, args, stderr) File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 409, in run run_details = manager.run(args) File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 204, in run int(self._options.child_processes), retrying=False) File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 263, in _run_tests return self._runner.run_tests(self._expectations, test_inputs, tests_to_skip, num_workers, needs_http, needs_websockets, needs_web_platform_test_server, retrying) File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 111, in run_tests self.start_servers() File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 193, in start_servers self._port.start_http_server() File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/port/base.py", line 918, in start_http_server server.start() File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/layout_tests/servers/http_server_base.py", line 79, in start self._stop_running_server() File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server.py", line 208, in _stop_running_server raise http_server_base.ServerError('Failed to stop %s: %s' % (self._name, err)) ServerError: Failed to stop httpd: apache2: Syntax error on line 30 of /home/clopez/webkit/webkit/layout-test-results/httpd.conf: Cannot load modules/libphp7.0.so into server: /usr/lib/apache2/modules/libphp7.0.so: cannot open shared object file: No such file or directory I wonder why Ubuntu has decided to stop shipping php5??? Debian still ships php5, even on unstable/sid. In any case, we can't land this as is. A Solution has to be found to keep machines still running with php5 working. Perhaps one idea is to make the layout tests tools detect which version of php has the system and generate one apache config file (php5) or other (php7). Another idea is to create a debian-httpd-php7.conf file or something like that. I see that on LayoutTests/http/conf/ there are several configs for debian, fedora, etc. And I guess depending on the system one or other is chosen. (In reply to comment #9) > (In reply to comment #4) > > So past this change, is WebKitEFL be easily buildable in < Ubuntu 16.04? > > Build is fine on < Ubuntu 16.04. But layout test can't be run on < Ubuntu > 16.04. 15.10 is the only one supported (by Canonical) Ubuntu version < 16.04, but it will reach its end of life in July. So bumping to 16.04 is not an option, but a necessity - https://wiki.ubuntu.com/Releases (In reply to comment #10) > I wonder why Ubuntu has decided to stop shipping php5??? Debian still ships > php5, even on unstable/sid. FWIW Fedora no longer ships PHP 5 either, we only have PHP 7. > In any case, we can't land this as is. A Solution has to be found to keep > machines still running with php5 working. Yup. > Another idea is to create a debian-httpd-php7.conf file or something > like that. This would probably be easier/better as then you don't have to worry about looking up the layout test config from the build directory, or what happens when the user upgrades to a newer distro. Look in Tools/Scripts/webkitpy/port/base.py _apache_config_file_name_for_platform, do a 'dpkg -s' there and change the Debian config file based on the results. Created attachment 281259 [details]
Patch
(In reply to comment #12) > (In reply to comment #10) > > I wonder why Ubuntu has decided to stop shipping php5??? Debian still ships > > php5, even on unstable/sid. > > FWIW Fedora no longer ships PHP 5 either, we only have PHP 7. > > > In any case, we can't land this as is. A Solution has to be found to keep > > machines still running with php5 working. > > Yup. > > > Another idea is to create a debian-httpd-php7.conf file or something > > like that. > > This would probably be easier/better as then you don't have to worry about > looking up the layout test config from the build directory, or what happens > when the user upgrades to a newer distro. Look in > Tools/Scripts/webkitpy/port/base.py _apache_config_file_name_for_platform, > do a 'dpkg -s' there and change the Debian config file based on the results. I just upload a WIP patch based on you guys suggestions. Let me request review as soon as patch is ready. Comment on attachment 281259 [details]
Patch
I verified this patch works both on Ubuntu 16.04 and < 16.04 locally. But I'm not sure if this patch is best to detect php version in Ubuntu. If someone knows better way to detect php version in Ubuntu, please let me know. If not, I'd like to land this patch as it is.
Comment on attachment 281259 [details]
Patch
Works ok on Debian stable. Thanks!
Comment on attachment 281259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281259&action=review > Tools/ChangeLog:3 > + [EFL] LayoutTest doesn't run on Ubuntu 16.04 Well it's actually not EFL-specific. > Tools/Scripts/webkitpy/port/base.py:1133 > + def _is_php_version_7(self): The function name should reflect that this is Debian-specific... _is_debian_php_version_7. > Tools/Scripts/webkitpy/port/base.py:1152 > + def _php_version(self): _debian_php_version > Tools/efl/install-dependencies:123 > + if [[ $ubuntu_version == *Ubuntu\ 16.04* ]]; then Eh, /etc/issue is used to customize the initial prompt on virtual terminals. It might mention the OS version by default, but sysadmins will often change it.... You should use /etc/os-release instead, and write it in a way that's not guaranteed to break in Ubuntu 16.10. And it should install the -7.0 package in Debian 9 as well. Anyway, I think you don't actually need to fix any of this, because you should be able to simply install the libapache2-mod-php virtual package and it should do the right thing. ;) Created attachment 281443 [details]
Patch
(In reply to comment #17) > Comment on attachment 281259 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=281259&action=review > > > Tools/ChangeLog:3 > > + [EFL] LayoutTest doesn't run on Ubuntu 16.04 > > Well it's actually not EFL-specific. > > > Tools/Scripts/webkitpy/port/base.py:1133 > > + def _is_php_version_7(self): > > The function name should reflect that this is Debian-specific... > _is_debian_php_version_7. > > > Tools/Scripts/webkitpy/port/base.py:1152 > > + def _php_version(self): > > _debian_php_version > Fixed all. Thanks. > > Tools/efl/install-dependencies:123 > > + if [[ $ubuntu_version == *Ubuntu\ 16.04* ]]; then > Eh, /etc/issue is used to customize the initial prompt on virtual terminals. > It might mention the OS version by default, but sysadmins will often change > it.... You should use /etc/os-release instead, and write it in a way that's > not guaranteed to break in Ubuntu 16.10. And it should install the -7.0 > package in Debian 9 as well. > > Anyway, I think you don't actually need to fix any of this, because you > should be able to simply install the libapache2-mod-php virtual package and > it should do the right thing. ;) Now I understand I'm able to keep to use "/etc/issue" to detect Ubuntu version because I simply need to install libapache2-mod-php package. If not, plz let me know again. Comment on attachment 281443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281443&action=review Yeah, you should be able to simply get rid of the installPHPWithApt function because I think you don't actually need to choose between installing libapache2-mod-php7.0 and libapache2-mod-php5; you can just install libapache2-mod-php. > Tools/efl/install-dependencies:122 > + ubuntu_version=`cat /etc/issue` To be clear: you can't use /etc/issue to detect distro version. Admins will modify this file to change agetty's prompt, and then your check won't work. I'm surprised it works at all; on Fedora we don't hardcode the distro version here, we use \S, which agetty expands to the distro name/version. (In reply to comment #20) > Comment on attachment 281443 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=281443&action=review > > Yeah, you should be able to simply get rid of the installPHPWithApt function > because I think you don't actually need to choose between installing > libapache2-mod-php7.0 and libapache2-mod-php5; you can just install > libapache2-mod-php. > No, you can't. libapache2-mod-php is not available on debian stable Created attachment 281459 [details]
Patch
(In reply to comment #21) > No, you can't. > > libapache2-mod-php is not available on debian stable Sigh... then we need installPHPWithApt after all, but it needs to check /etc/os-release, not /etc/issue. Created attachment 281517 [details]
Patch
(In reply to comment #23) > (In reply to comment #21) > > No, you can't. > > > > libapache2-mod-php is not available on debian stable > > Sigh... then we need installPHPWithApt after all, but it needs to check > /etc/os-release, not /etc/issue. Ok, Fixed. Comment on attachment 281517 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281517&action=review > Tools/gtk/install-dependencies:439 > + if [[ $ubuntu_version == *VERSION_ID=\"16.04* ]]; then Can you write this in a way that will work on future versions of Ubuntu and Debian? What you really want is something like this: if Ubuntu: if version < 16.04: install PHP 5 else install PHP 7 else if Debian: if version < 9: install PHP 5 else install PHP 7 (In reply to comment #26) > Comment on attachment 281517 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=281517&action=review > > > Tools/gtk/install-dependencies:439 > > + if [[ $ubuntu_version == *VERSION_ID=\"16.04* ]]; then > > Can you write this in a way that will work on future versions of Ubuntu and > Debian? What you really want is something like this: > > if Ubuntu: > if version < 16.04: install PHP 5 > else install PHP 7 > else if Debian: > if version < 9: install PHP 5 > else install PHP 7 If gtk folk need to support debian, I'm willing to support it. But unfortunately I only have Ubuntu now. I can upload a following patch after preparing Debian box. But if you can make a patch for debian, please upload it from your side. I will help to review it. Comment on attachment 281517 [details] Patch Clearing flags on attachment: 281517 Committed r202158: <http://trac.webkit.org/changeset/202158> All reviewed patches have been landed. Closing bug. Reopening since this code only works on Ubuntu 16.04; it's going to be broken in e.g. Ubuntu 16.10. I shouldn't have given r+. I think we have an easy solution, though. We can simply: apt-get install libapache2-mod-php7.0 || apt-get install libapache2-mod-php5 Created attachment 282464 [details]
Patch
(Untested, but looks simple enough.) (In reply to comment #32) > (Untested, but looks simple enough.) It works, but consider this alternative version that avoids extra calls to apt-get and don't prints unnecessary warnings on stdout. diff --git a/Tools/gtk/install-dependencies b/Tools/gtk/install-dependencies index eaab85e..afc9835 100755 --- a/Tools/gtk/install-dependencies +++ b/Tools/gtk/install-dependencies @@ -199,9 +199,14 @@ function installDependenciesWithApt { git-svn \ subversion" + if apt-cache show libapache2-mod-php7.0 >/dev/null; then + packages="$packages libapache2-mod-php7.0" + else + packages="$packages libapache2-mod-php5" + fi + apt-get install $packages - installPHPWithApt } function installDependenciesWithPacman { @@ -455,16 +460,5 @@ function installDependenciesWithDnf { dnf install $packages } -function installPHPWithApt { - if [ -f "/etc/os-release" ]; then - ubuntu_version=`grep VERSION_ID /etc/os-release` - if [[ $ubuntu_version == *VERSION_ID=\"16.04* ]]; then - apt-get install libapache2-mod-php7.0 - else - apt-get install libapache2-mod-php5 - fi - fi -} - checkInstaller I don't think this is critical issue that below log error message is printed during the dependencies installation. However it would be good if we can avoid below message is not printed as Carlos said. Reading state information... Done E: Unable to locate package libapache2-mod-php7.0 E: Couldn't find any package by regex 'libapache2-mod-php7.0' Good point about the warnings. My concern with apt-cache: it doesn't work the first time you use it, right? That works for you because you previously built a database for use by apt-cache, right? So we need to build the apt-cache database if it has not been done already. (In reply to comment #35) > Good point about the warnings. > > My concern with apt-cache: it doesn't work the first time you use it, right? > That works for you because you previously built a database for use by > apt-cache, right? So we need to build the apt-cache database if it has not > been done already. Its the same for apt-get. Both commands share the same database. If apt-cache fails to find the package, then apt-get is going to fail also. OK then... r=me if you want to submit a patch? Committed r202808: <http://trac.webkit.org/changeset/202808> |
It looks we need to bump php version. ServerError raised: Failed to start httpd: apache2: Syntax error on line 30 of /home/gyuyoung/webkit/WebKit/layout-test-results/httpd.conf: Cannot load modules/libphp5.so into server: /usr/lib/apache2/modules/libphp5.so: cannot open shared object file: No such file or directory Traceback (most recent call last): File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 77, in main run_details = run(port, options, args, stderr) File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 409, in run run_details = manager.run(args) File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 210, in run int(self._options.child_processes), retrying=False) File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 269, in _run_tests return self._runner.run_tests(self._expectations, test_inputs, tests_to_skip, num_workers, needs_http, needs_websockets, needs_web_platform_test_server, retrying) File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 111, in run_tests self.start_servers() File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 193, in start_servers self._port.start_http_server() File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/port/base.py", line 918, in start_http_server server.start() File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/servers/http_server_base.py", line 89, in start self._pid = self._spawn_process() File "/home/gyuyoung/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server.py", line 184, in _spawn_process raise http_server_base.ServerError('Failed to start %s: %s' % (self._name, err)) ServerError: Failed to start httpd: apache2: Syntax error on line 30 of /home/gyuyoung/webkit/WebKit/layout-test-results/httpd.conf: Cannot load modules/libphp5.so into server: /usr/lib/apache2/modules/libphp5.so: cannot open shared object file: No such file or directory