Bug 160814 - [GTK] Install script lacks gstreamer related dependencies
Summary: [GTK] Install script lacks gstreamer related dependencies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-12 12:59 PDT by Carlos Alberto Lopez Perez
Modified: 2016-08-16 07:54 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.41 KB, patch)
2016-08-12 13:11 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2016-08-12 12:59:58 PDT
Trying to build webkitgtk+ on a debian testing chroot after running the install dependencies script (but without building the jhbuild) results on this:

-- Checking for module 'gstreamer-1.0 >= 1.0.3'
--   
-- Checking for module 'gstreamer-base-1.0 >= 1.0.3'
--   
-- Checking for module 'gstreamer-app-1.0 >= 1.0.3'
--   
-- Checking for module 'gstreamer-audio-1.0 >= 1.0.3'
--   
-- Checking for module 'gstreamer-fft-1.0 >= 1.0.3'
--   
-- Checking for module 'gstreamer-gl-1.0>=1.8.0'
--   
-- Checking for module 'gstreamer-mpegts-1.0>=1.4.0'
--   
-- Checking for module 'gstreamer-pbutils-1.0 >= 1.0.3'
--   
-- Checking for module 'gstreamer-tag-1.0 >= 1.0.3'
--   
-- Checking for module 'gstreamer-video-1.0 >= 1.0.3'
--   
-- Found GStreamer: GSTREAMER_INCLUDE_DIRS;GSTREAMER_LIBRARIES;GSTREAMER_VERSION;GSTREAMER_BASE_INCLUDE_DIRS;GSTREAMER_BASE_LIBRARIES;GSTREAMER_APP_INCLUDE_DIRS;GSTREAMER_APP_LIBRARIES;GSTREAMER_PBUTILS_INCLUDE_DIRS;GSTREAMER_PBUTILS_LIBRARIES;GSTREAMER_VIDEO_INCLUDE_DIRS;GSTREAMER_VIDEO_LIBRARIES;GSTREAMER_MPEGTS_INCLUDE_DIRS;GSTREAMER_MPEGTS_LIBRARIES;GSTREAMER_TAG_INCLUDE_DIRS;GSTREAMER_TAG_LIBRARIES;GSTREAMER_GL_INCLUDE_DIRS;GSTREAMER_GL_LIBRARIES;GSTREAMER_AUDIO_INCLUDE_DIRS;GSTREAMER_AUDIO_LIBRARIES;GSTREAMER_FFT_INCLUDE_DIRS;GSTREAMER_FFT_LIBRARIES (Required is at least version "1.0.3") 
CMake Error at Source/cmake/OptionsGTK.cmake:339 (message):
  WebAudio requires the audio and fft GStreamer libraries.  Please check your
  gst-plugins-base installation.
Call Stack (most recent call first):
  Source/cmake/WebKitCommon.cmake:47 (include)
  CMakeLists.txt:126 (include)
Comment 1 Carlos Alberto Lopez Perez 2016-08-12 13:11:17 PDT
Created attachment 285937 [details]
Patch
Comment 2 Philippe Normand 2016-08-13 01:12:19 PDT
I thought the install-dependencies script was meant to be used in combination with JHBuild, meaning that it shouldn't install libs built by JHBuild already...
Comment 3 Carlos Alberto Lopez Perez 2016-08-13 08:10:48 PDT
(In reply to comment #2)
> I thought the install-dependencies script was meant to be used in
> combination with JHBuild, meaning that it shouldn't install libs built by
> JHBuild already...

I also thought this, but then I saw listed several dependencies that we build on the JHBuild, like atk, gtk+, cairo ...

So, then I thought that it actually made sense to also included the dependencies to build WebKitGTK+ without using the JHBuild (which is something that you want to do sometimes).

this makes sense to you also?
Comment 4 Carlos Garcia Campos 2016-08-13 09:25:06 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > I thought the install-dependencies script was meant to be used in
> > combination with JHBuild, meaning that it shouldn't install libs built by
> > JHBuild already...
> 
> I also thought this, but then I saw listed several dependencies that we
> build on the JHBuild, like atk, gtk+, cairo ...
> 
> So, then I thought that it actually made sense to also included the
> dependencies to build WebKitGTK+ without using the JHBuild (which is
> something that you want to do sometimes).
> 
> this makes sense to you also?

yes
Comment 5 Carlos Alberto Lopez Perez 2016-08-13 13:32:25 PDT
Comment on attachment 285937 [details]
Patch

Clearing flags on attachment: 285937

Committed r204448: <http://trac.webkit.org/changeset/204448>
Comment 6 Carlos Alberto Lopez Perez 2016-08-13 13:32:33 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Konstantin Tokarev 2016-08-13 13:39:57 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > I thought the install-dependencies script was meant to be used in
> > combination with JHBuild, meaning that it shouldn't install libs built by
> > JHBuild already...
> 
> I also thought this, but then I saw listed several dependencies that we
> build on the JHBuild, like atk, gtk+, cairo ...

I even was cleaning up such cases, following advise of Michael Catanzaro, see https://bugs.webkit.org/show_bug.cgi?id=159628
Comment 8 Carlos Alberto Lopez Perez 2016-08-13 15:08:02 PDT
(In reply to comment #7)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > I thought the install-dependencies script was meant to be used in
> > > combination with JHBuild, meaning that it shouldn't install libs built by
> > > JHBuild already...
> > 
> > I also thought this, but then I saw listed several dependencies that we
> > build on the JHBuild, like atk, gtk+, cairo ...
> 
> I even was cleaning up such cases, following advise of Michael Catanzaro,
> see https://bugs.webkit.org/show_bug.cgi?id=159628

Well... I guess we need to add some comment on the script clarifying which kind of dependency is ok or not.

Personally, I see value on having listed the dependencies that allow to build WebKitGTK+ without using the internal jhbuild.


Perhaps an idea is to add a --nojhbuild parameter that installs the dependencies which are only needed when no using the jhbuild ?
Comment 9 Michael Catanzaro 2016-08-15 01:52:03 PDT
(In reply to comment #2)
> I thought the install-dependencies script was meant to be used in
> combination with JHBuild, meaning that it shouldn't install libs built by
> JHBuild already...

I agree; why should it install extra stuff?

FYI: I'm planning to delete this script and replace it with jhbuild sysdeps.
Comment 10 Konstantin Tokarev 2016-08-15 02:39:39 PDT
As for jhbuild sysdeps: AFAIU installation of system packages would require running jhbuild as root, but we should run main jhbuild as root for obvious reasons. Also, jhbuild itself is installed as a part of update-webkit-libs-jhbuild which is normally run after install-dependencies.
Comment 11 Michael Catanzaro 2016-08-15 03:56:21 PDT
(In reply to comment #10)
> As for jhbuild sysdeps: AFAIU installation of system packages would require
> running jhbuild as root

That's a Debian-specific problem. PackageKit allows package installation without authentication by default, and PackageKit is the default, cross-platform jhbuild sysdeps backend. (And it uses polkit, so it works without any root account.) But on Debian, jhbuild defaults to a Debian-specific sysdeps backend that uses apt-cache; I'm not completely sure why, but I think it's because Debian's PackageKit backend doesn't support searching for files? It's slower and does not use polkit, so we really do have to run it as root, but that is not any problem because the existing install-dependencies script already requires root. So the change is actually that root will no longer be required on other distros.

> but we should run main jhbuild as root for obvious
> reasons.

Of course not, only the sysdeps --install portion would need to be run as root (I guess with sudo), and *only* on Debian/Ubuntu. (We would need to check /etc/os-release before deciding to use sudo.) Actually building stuff should obviously not be done as root.

> Also, jhbuild itself is installed as a part of
> update-webkit-libs-jhbuild which is normally run after install-dependencies.

We would have to change that. The order would be:

 * Install jhbuild
 * jhbuild sysdeps --install
 * jhbuild build
Comment 12 Carlos Alberto Lopez Perez 2016-08-16 06:58:04 PDT
(In reply to comment #9)
> (In reply to comment #2)
> > I thought the install-dependencies script was meant to be used in
> > combination with JHBuild, meaning that it shouldn't install libs built by
> > JHBuild already...
> 
> I agree; why should it install extra stuff?
> 

Because sometimes I want to build WebKit without the JHBuild???? And sometimes I want to build it on distros that are not my usual distro (like Fedora). And I find it handy to just execute an script that installs all the deps.

> FYI: I'm planning to delete this script and replace it with jhbuild sysdeps.

In this case, rather than deleting it, please rename it to install-dependencies-nojhbuild
Comment 13 Konstantin Tokarev 2016-08-16 07:04:12 PDT
1. Package lists for these 2 cases are going to be different

2. Sysdeps of jhbuild can be used for "non-jhbuild" set up as well, it's not very productive to maintain parallel package lists for different distros.
Comment 14 Konstantin Tokarev 2016-08-16 07:04:51 PDT
(though it will have to be different jhbuild sysdeps module)
Comment 15 Michael Catanzaro 2016-08-16 07:24:22 PDT
You could just run 'jhbuild sysdeps --install' to install the dependencies, you don't need to build the full jhbuild environment if you don't want to. The point of switching to jhbuild sysdeps is to be able to remove this script, so we don't have to maintain three different sets of dependencies for each distro, there's no value in doing it otherwise.
Comment 16 Carlos Alberto Lopez Perez 2016-08-16 07:39:45 PDT
(In reply to comment #15)
> You could just run 'jhbuild sysdeps --install' to install the dependencies,
> you don't need to build the full jhbuild environment if you don't want to.
> The point of switching to jhbuild sysdeps is to be able to remove this
> script, so we don't have to maintain three different sets of dependencies
> for each distro, there's no value in doing it otherwise.

That won't work.

For example, I want the cairo-devel from the system in order to be able to built WebKit without the JHBuild.

And jhbuild sysdeps --install should not install Cairo because we want to build our own very specific version of Cairo in order to have reliable test results for the layout tests.
Comment 17 Konstantin Tokarev 2016-08-16 07:40:59 PDT
Solution is separate modulesets for 2 use cases.
Comment 18 Konstantin Tokarev 2016-08-16 07:54:12 PDT
(In reply to comment #11)
> That's a Debian-specific problem. PackageKit allows package installation
> without authentication by default, and PackageKit is the default,
> cross-platform jhbuild sysdeps backend. (And it uses polkit, so it works
> without any root account.) But on Debian, jhbuild defaults to a
> Debian-specific sysdeps backend that uses apt-cache; I'm not completely sure
> why, but I think it's because Debian's PackageKit backend doesn't support
> searching for files?

FWIW, apt-cache does not require root access, and pkexec could be used to run apt-get