Bug 114908 – [EFL] Remove unnecessary pkgs in EFL jhbuild

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

[Gyuyoung Kim]

As far as I understand, main role of jhbuild is for layout test. However, it looks that EFL jhbuild supports some pkgs which don't influence on the result of layout test. For example, p11-kit, libgpg-error, libgcrypt and libseccomp. We can install them by *apt-get install*. I don't prefer to take a long build time when using --update-efl. I hope to keep essential pkgs in jhbuild.

[Gyuyoung Kim]

Created attachment 198957 [details]
Patch

[Laszlo Gombos]

(In reply to comment #0)
> As far as I understand, main role of jhbuild is for layout test. However, it looks that EFL jhbuild supports some pkgs which don't influence on the result of layout test. For example, p11-kit, libgpg-error, libgcrypt and libseccomp.

Are you sure that e.g. the latest libsoup that is being built with jhbuild builds and tested using an older version of libgcrypt that comes with your Linux distribution ?

[Raphael Kubo da Costa (:rakuco)]

In general, jhbuild was used both to make the layout test results equal across different systems (by using the same version of pixman and cairo, for example) and also to ease the development setup for contributors (we don't really need to ship the EFL, for example).

That said, making the list of packages installed smaller is always a good thing.

As Laszlo said, please check if at least the libgcrypt stuff is not needed by libsoup.

[Gyuyoung Kim]

(In reply to comment #3)
> In general, jhbuild was used both to make the layout test results equal across different systems (by using the same version of pixman and cairo, for example) and also to ease the development setup for contributors (we don't really need to ship the EFL, for example).
> 
> That said, making the list of packages installed smaller is always a good thing.
> 
> As Laszlo said, please check if at least the libgcrypt stuff is not needed by libsoup.

Thank you for your agreement. To my quick search, libsoup doesn't use crypto library directly, but it uses dependent library(gnutls) which uses *nettle* library for crypto, not libgcrypt. The libgcrypt is used by *eet* library for WebKit EFL. I think we can move it to the dependencies section in http://trac.webkit.org/wiki/EFLWebKit. Or, it would be good if we support a script to install all dependencies pkgs as GTK port. http://trac.webkit.org/browser/trunk/Tools/gtk/install-dependencies

[Gyuyoung Kim]

(In reply to comment #4)

> Thank you for your agreement. To my quick search, libsoup doesn't use crypto library directly, but it uses dependent library(gnutls) which uses *nettle* library for crypto, not libgcrypt. 

Oops, gnutls can use libgcrypt. Even the gnutls pkg has a dependency libgcrypt pkg on Ubuntu 13.04. It looks dependency chain is as below,

Dependency chain : libsoup -> glib-networking -> gnutls -> libgcrypt.

If we want to install libgcrypt via jhbuild, it looks we also need to install gnutls via jhbuild. However, I still think we don't need to install libgcypt via jhbuild.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #5)
> If we want to install libgcrypt via jhbuild, it looks we also need to install gnutls via jhbuild.

Why is that?

> However, I still think we don't need to install libgcypt via jhbuild.

We usually try to support the latest Ubuntu LTS release so that we can depend on old enough stuff (and ship newer versions of dependencies in jhbuild when needed). As long as things do not break for Ubuntu 12.04 by removing libgcrypt and/or gnutls from the build, I'm fine with it.

[Gyuyoung Kim]

(In reply to comment #6)

>  As long as things do not break for Ubuntu 12.04 by removing libgcrypt and/or gnutls from the build, I'm fine with it.

There is no build problem on my 12.04 PC locally. It looks this patch will not influence on 12.04 at least.

[Gyuyoung Kim]

Laszlo, it looks there is no objection to land this patch. What do you think about it ?

[Laszlo Gombos]

+Thiago for feedback on libseccomp.

[Gyuyoung Kim]

Created attachment 201024 [details]
Patch

[Gyuyoung Kim]

Laszlo, there are build breaks when we don't use libseccomp on jhbuild. So, I don't remove it in this patch. Thank you for your point it out.

[Thiago Marcos P. Santos]

(In reply to comment #11)
> Laszlo, there are build breaks when we don't use libseccomp on jhbuild. So, I don't remove it in this patch. Thank you for your point it out.

Weird. It should build without libseccomp...

BTW, I should have added it to "jhbuild-optional.modules" instead.

[Gyuyoung Kim]

(In reply to comment #12)
> (In reply to comment #11)
> > Laszlo, there are build breaks when we don't use libseccomp on jhbuild. So, I don't remove it in this patch. Thank you for your point it out.
> 
> Weird. It should build without libseccomp...

When using --seccomp-filters, there were below breaks.

[ 92%] /home/gyuyoung/webkit/WebKit/Source/WebKit2/Shared/linux/SeccompFilters/SeccompFilters.cpp: In constructor ‘WebKit::SeccompFilters::SeccompFilters(WebKit::SeccompFilters::Action)’:
/home/gyuyoung/webkit/WebKit/Source/WebKit2/Shared/linux/SeccompFilters/SeccompFilters.cpp:49:26: error: invalid conversion from ‘int’ to ‘WebKit::SeccompFilters::HANDLE {aka void*}’ [-fpermissive]
/home/gyuyoung/webkit/WebKit/Source/WebKit2/Shared/linux/SeccompFilters/SeccompFilters.cpp: In destructor ‘virtual WebKit::SeccompFilters::~SeccompFilters()’:
/home/gyuyoung/webkit/WebKit/Source/WebKit2/Shared/linux/SeccompFilters/SeccompFilters.cpp:57:30: error: too many arguments to function ‘void seccomp_release()’
/usr/include/seccomp.h:184:6: note: declared here

[Thiago Marcos P. Santos]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Laszlo, there are build breaks when we don't use libseccomp on jhbuild. So, I don't remove it in this patch. Thank you for your point it out.
> > 
> > Weird. It should build without libseccomp...
> 
> When using --seccomp-filters, there were below breaks.
> 
> [ 92%] /home/gyuyoung/webkit/WebKit/Source/WebKit2/Shared/linux/SeccompFilters/SeccompFilters.cpp: In constructor ‘WebKit::SeccompFilters::SeccompFilters(WebKit::SeccompFilters::Action)’:
> /home/gyuyoung/webkit/WebKit/Source/WebKit2/Shared/linux/SeccompFilters/SeccompFilters.cpp:49:26: error: invalid conversion from ‘int’ to ‘WebKit::SeccompFilters::HANDLE {aka void*}’ [-fpermissive]
> /home/gyuyoung/webkit/WebKit/Source/WebKit2/Shared/linux/SeccompFilters/SeccompFilters.cpp: In destructor ‘virtual WebKit::SeccompFilters::~SeccompFilters()’:
> /home/gyuyoung/webkit/WebKit/Source/WebKit2/Shared/linux/SeccompFilters/SeccompFilters.cpp:57:30: error: too many arguments to function ‘void seccomp_release()’
> /usr/include/seccomp.h:184:6: note: declared here

Ah, in this case yes. But maybe we should only add it to jhbuild when it is enabled by default.

[Gyuyoung Kim]

(In reply to comment #14)
 
> Ah, in this case yes. But maybe we should only add it to jhbuild when it is enabled by default.

ok, if you wanna add it when it is enabled by default, I don't have big opinion about it. I request previous patch again.

[Laszlo Gombos]

Comment on attachment 201024 [details]
Patch

lgtm.

I do not think jhbuild should have knowledge of what webkit features are being turned on/off. The one and only jhbuild config should support all possible webkit build configurations.

[Gyuyoung Kim]

Comment on attachment 201024 [details]
Patch

Clearing flags on attachment: 201024

Committed r149790: <http://trac.webkit.org/changeset/149790>

[Gyuyoung Kim]

All reviewed patches have been landed.  Closing bug.

[Gyuyoung Kim]

Those dependency pkgs are added to "Dependencies" field in WebKit EFL wiki.
(https://trac.webkit.org/wiki/EFLWebKit)

 * libp11-kit-dev
 * libgpg-error-dev
 * libgcrypt11-dev