Bug 114908 - [EFL] Remove unnecessary pkgs in EFL jhbuild
Summary: [EFL] Remove unnecessary pkgs in EFL jhbuild
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-21 00:38 PDT by Gyuyoung Kim
Modified: 2013-05-08 21:56 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.77 KB, patch)
2013-04-21 00:47 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (2.87 KB, patch)
2013-05-07 20:34 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2013-04-21 00:38:47 PDT
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.
Comment 1 Gyuyoung Kim 2013-04-21 00:47:41 PDT
Created attachment 198957 [details]
Patch
Comment 2 Laszlo Gombos 2013-04-22 18:52:17 PDT
(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 ?
Comment 3 Raphael Kubo da Costa (:rakuco) 2013-04-23 06:12:50 PDT
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.
Comment 4 Gyuyoung Kim 2013-04-24 17:54:32 PDT
(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
Comment 5 Gyuyoung Kim 2013-04-24 21:32:07 PDT
(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.
Comment 6 Raphael Kubo da Costa (:rakuco) 2013-04-25 09:40:38 PDT
(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.
Comment 7 Gyuyoung Kim 2013-04-26 00:52:57 PDT
(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.
Comment 8 Gyuyoung Kim 2013-04-29 09:10:37 PDT
Laszlo, it looks there is no objection to land this patch. What do you think about it ?
Comment 9 Laszlo Gombos 2013-05-07 18:48:20 PDT
+Thiago for feedback on libseccomp.
Comment 10 Gyuyoung Kim 2013-05-07 20:34:41 PDT
Created attachment 201024 [details]
Patch
Comment 11 Gyuyoung Kim 2013-05-07 20:35:58 PDT
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.
Comment 12 Thiago Marcos P. Santos 2013-05-08 01:24:06 PDT
(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.
Comment 13 Gyuyoung Kim 2013-05-08 01:54:48 PDT
(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
Comment 14 Thiago Marcos P. Santos 2013-05-08 04:11:51 PDT
(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.
Comment 15 Gyuyoung Kim 2013-05-08 17:42:04 PDT
(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.
Comment 16 Laszlo Gombos 2013-05-08 19:39:40 PDT
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.
Comment 17 Gyuyoung Kim 2013-05-08 21:50:19 PDT
Comment on attachment 201024 [details]
Patch

Clearing flags on attachment: 201024

Committed r149790: <http://trac.webkit.org/changeset/149790>
Comment 18 Gyuyoung Kim 2013-05-08 21:50:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 Gyuyoung Kim 2013-05-08 21:56:59 PDT
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