Bug 158141 – [EFL][GTK] Layout Test doesn't run on Ubuntu 16.04

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Gyuyoung Kim]

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

[Gyuyoung Kim]

Created attachment 279933 [details]
Patch

[Gyuyoung Kim]

Hmm, all tests come to crash after bumpping php version from 5.0 to 7.0 :(

[Csaba Osztrogonác]

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?

[Antonio Gomes]

So past this change, is WebKitEFL be easily buildable in < Ubuntu 16.04?

[Carlos Alberto Lopez Perez]

(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.

[Csaba Osztrogonác]

(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.

[Gyuyoung Kim]

Created attachment 280895 [details]
Patch

[Gyuyoung Kim]

(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?

[Gyuyoung Kim]

(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.

[Carlos Alberto Lopez Perez]

(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.

[Csaba Osztrogonác]

(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

[Michael Catanzaro]

(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.

[Gyuyoung Kim]

Created attachment 281259 [details]
Patch

[Gyuyoung Kim]

(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.

[Gyuyoung Kim]

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.

[Carlos Alberto Lopez Perez]

Comment on attachment 281259 [details]
Patch

Works ok on Debian stable. Thanks!

[Michael Catanzaro]

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. ;)

[Gyuyoung Kim]

Created attachment 281443 [details]
Patch

[Gyuyoung Kim]

(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.

[Michael Catanzaro]

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.

[Carlos Alberto Lopez Perez]

(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

[Gyuyoung Kim]

Created attachment 281459 [details]
Patch

[Michael Catanzaro]

(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.

[Gyuyoung Kim]

Created attachment 281517 [details]
Patch

[Gyuyoung Kim]

(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.

[Michael Catanzaro]

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

[Gyuyoung Kim]

(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.

[WebKit Commit Bot]

Comment on attachment 281517 [details]
Patch

Clearing flags on attachment: 281517

Committed r202158: <http://trac.webkit.org/changeset/202158>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

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

[Michael Catanzaro]

Created attachment 282464 [details]
Patch

[Michael Catanzaro]

(Untested, but looks simple enough.)

[Carlos Alberto Lopez Perez]

(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

[Gyuyoung Kim]

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'

[Michael Catanzaro]

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.

[Carlos Alberto Lopez Perez]

(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.

[Michael Catanzaro]

OK then... r=me if you want to submit a patch?

[Carlos Alberto Lopez Perez]

Committed r202808: <http://trac.webkit.org/changeset/202808>