After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 734327 - Build against webkit2gtk-4.0
Build against webkit2gtk-4.0
Status: RESOLVED FIXED
Product: gitg
Classification: Applications
Component: gitg
git master
Other Linux
: Normal normal
: ---
Assigned To: gitg-maint
gitg-maint
Depends on:
Blocks:
 
 
Reported: 2014-08-06 04:38 UTC by Michael Catanzaro
Modified: 2014-08-14 17:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Support both webkit2gtk-3.0 and webkit2gtk-4.0 (51.88 KB, patch)
2014-08-11 15:17 UTC, Michael Catanzaro
none Details | Review
Support both webkit2gtk-3.0 and webkit2gtk-4.0 (51.75 KB, patch)
2014-08-11 15:23 UTC, Michael Catanzaro
none Details | Review
Support both webkit2gtk-3.0 and webkit2gtk-4.0 (51.96 KB, patch)
2014-08-13 01:50 UTC, Michael Catanzaro
none Details | Review
Support both webkit2gtk-3.0 and webkit2gtk-4.0 (51.58 KB, patch)
2014-08-13 18:51 UTC, Michael Catanzaro
committed Details | Review

Description Michael Catanzaro 2014-08-06 04:38:53 UTC
gitg is broken in jhbuild because it builds against webkit2gtk-3.0, which has been superseded by webkit2gtk-4.0.

There's new Vala bindings available in the Vala repo. Fortunately the changes are pretty minimal....
Comment 1 jessevdk@gmail.com 2014-08-06 08:24:44 UTC
It would be great to have some help here. I want gitg to be able to compile against both 3.0 and 4.0, but I don't want to build webkit2gtk manually on my machine (I think it might even not have enough RAM to complete it). Would you be interested in providing a patch?
Comment 2 Michael Catanzaro 2014-08-11 05:43:36 UTC
I keep meaning to get around to this, sorry. :(
Comment 3 Michael Catanzaro 2014-08-11 15:17:24 UTC
Created attachment 283114 [details] [review]
Support both webkit2gtk-3.0 and webkit2gtk-4.0
Comment 4 Michael Catanzaro 2014-08-11 15:19:08 UTC
Review of attachment 283114 [details] [review]:

::: Makefile.am
@@ +123,2 @@
 %.typelib: %.gir
+	$(INTROSPECTION_COMPILER) $(INTROSPECTION_COMPILER_ARGS) --includedir=$(top_srcdir) --includedir=$(datadir)/webkitgtk-4.0/gir-1.0 -o $@ $<

Couldn't figure out any nicer way to make this work. I think the extra include path should not cause any problems for webkit2gtk-3.0 users.
Comment 5 Michael Catanzaro 2014-08-11 15:23:58 UTC
Created attachment 283115 [details] [review]
Support both webkit2gtk-3.0 and webkit2gtk-4.0

Removed some cruft I accidentally left in configure.ac
Comment 6 jessevdk@gmail.com 2014-08-11 15:48:33 UTC
Review of attachment 283115 [details] [review]:

Only minor nitpicks. Additionally, does this mean that we didn't use any of the API that changed?

::: Makefile.am
@@ +123,2 @@
 %.typelib: %.gir
+	$(INTROSPECTION_COMPILER) $(INTROSPECTION_COMPILER_ARGS) --includedir=$(top_srcdir) --includedir=$(datadir)/webkitgtk-4.0/gir-1.0 -o $@ $<

This is probably not harmful, but at the same time it is easy to just export additional introspection args from configure.ac. You can create a new var in configure.ac and set it to --includedir=... etc. in the case of webkit2gtk-4.0, AC_SUBST it and use that variable here.

::: configure.ac
@@ +275,3 @@
 	--pkg json-glib-1.0			\
 	--pkg libsoup-2.4			\
+	--pkg \$(WEBKIT_PKGCONFIG)		\

No need to not directly substitute in the pkg.
Comment 7 Michael Catanzaro 2014-08-13 01:02:38 UTC
(In reply to comment #6)
> Review of attachment 283115 [details] [review]:
> 
> Only minor nitpicks. Additionally, does this mean that we didn't use any of the
> API that changed?

Yup. There are very few differences, discussed at [1].

[1] http://blogs.igalia.com/carlosgc/2014/08/01/webkitgtk-2-5-1-good-bye-webkit1/
Comment 8 Michael Catanzaro 2014-08-13 01:50:55 UTC
Created attachment 283245 [details] [review]
Support both webkit2gtk-3.0 and webkit2gtk-4.0
Comment 9 jessevdk@gmail.com 2014-08-13 07:33:48 UTC
Review of attachment 283245 [details] [review]:

::: configure.ac
@@ +89,3 @@
+PKG_CHECK_EXISTS([webkit2gtk-4.0], [
+	WEBKIT_PKGCONFIG=webkit2gtk-4.0
+	WEBKIT_INTROSPECTION_COMPILER_ARGS='--includedir=\$(datadir)/webkitgtk-4.0/gir-1.0'

This doesn't work when installing gitg in a different prefix than webkit. The way to do this is to obtain the datadir for webkit from pkgconfig.
Comment 10 Michael Catanzaro 2014-08-13 15:44:22 UTC
(In reply to comment #9)
> This doesn't work when installing gitg in a different prefix than webkit. The
> way to do this is to obtain the datadir for webkit from pkgconfig.

Bleh, that variable doesn't exist in the pkgconfig.  Carlos, is it OK if I add it? I don't think there's a better way to determine where the gir is installed to?

I'm also confused why manually adding the includedir was not previously necessary....
Comment 11 jessevdk@gmail.com 2014-08-13 17:01:11 UTC
Relevant stubs:

_gir_get_pkgconfig_var(INTROSPECTION_INSTALL_GIRDIR "girdir" "--define-variable=datadir=${DATA_INSTALL_DIR}")

at https://github.com/WebKit/webkit/blob/18330d0fee077c9b8d35d0178d01f56e46d4e517/Source/cmake/FindGObjectIntrospection.cmake#L47. I'm not sure why datadir is explicitly overridden there. As a temporary solution, you can obtain the prefix variable from pkg-config and append /share to it to make the datadir. But I agree, it looks like this is an issue to be sorted out at the webkit level.

(In reply to comment #10)
> (In reply to comment #9)
> I'm also confused why manually adding the includedir was not previously
> necessary....

Because I guess before the gir file was installed in the system-wide girdir.
Comment 12 Carlos Garcia Campos 2014-08-13 17:08:11 UTC
(In reply to comment #11)
> Relevant stubs:
> 
> _gir_get_pkgconfig_var(INTROSPECTION_INSTALL_GIRDIR "girdir"
> "--define-variable=datadir=${DATA_INSTALL_DIR}")
> 
> at
> https://github.com/WebKit/webkit/blob/18330d0fee077c9b8d35d0178d01f56e46d4e517/Source/cmake/FindGObjectIntrospection.cmake#L47.
> I'm not sure why datadir is explicitly overridden there. As a temporary
> solution, you can obtain the prefix variable from pkg-config and append /share
> to it to make the datadir. But I agree, it looks like this is an issue to be
> sorted out at the webkit level.
> 
> (In reply to comment #10)
> > (In reply to comment #9)
> > I'm also confused why manually adding the includedir was not previously
> > necessary....
> 
> Because I guess before the gir file was installed in the system-wide girdir.

I think it's a bug in WebKit, we didn't want to change the path where the gir files are installed. It sounds like yet another regression of the cmake switch.
Comment 13 Carlos Garcia Campos 2014-08-13 17:10:49 UTC
(In reply to comment #11)
> Relevant stubs:
> 
> _gir_get_pkgconfig_var(INTROSPECTION_INSTALL_GIRDIR "girdir"
> "--define-variable=datadir=${DATA_INSTALL_DIR}")
> 
> at
> https://github.com/WebKit/webkit/blob/18330d0fee077c9b8d35d0178d01f56e46d4e517/Source/cmake/FindGObjectIntrospection.cmake#L47.
> I'm not sure why datadir is explicitly overridden there. As a temporary
> solution, you can obtain the prefix variable from pkg-config and append /share
> to it to make the datadir. But I agree, it looks like this is an issue to be
> sorted out at the webkit level.

Yes, definitely a bug, it was not our intention to override that variable. Please file a bug in WebKit and I'll make sure it's fixed for 2.5.3.
Comment 14 jessevdk@gmail.com 2014-08-13 18:14:13 UTC
@Michael: if you file it at the webkit tracker, could you please add a reference here?
Comment 15 Michael Catanzaro 2014-08-13 18:51:09 UTC
Created attachment 283318 [details] [review]
Support both webkit2gtk-3.0 and webkit2gtk-4.0

(Doesn't work, pending WebKit fix.)
Comment 16 Michael Catanzaro 2014-08-13 18:51:56 UTC
(In reply to comment #13) 
> Yes, definitely a bug, it was not our intention to override that variable.
> Please file a bug in WebKit and I'll make sure it's fixed for 2.5.3.

Oh lovely, I'm the one who broke this, in
https://bugs.webkit.org/show_bug.cgi?id=135288

I think we should just revert that commit.  The use case was kind of bogus;
Martin was trying to help me build WebKit without jhbuild and install it into
my jhbuild install prefix, but I'm pretty sure that will never realistically
work. If I had been using jhbuild to build WebKit then the pkgconfig would have
found the right location automatically.

(In reply to comment #12)
> > Because I guess before the gir file was installed in the system-wide girdir.
> 
> I think it's a bug in WebKit, we didn't want to change the path where the gir
> files are installed. It sounds like yet another regression of the cmake switch.

Actually yes, that's true, they used to be installed in the system-wide girdir.
 I thought I had checked that and concluded otherwise.... Anyway, if we fix
that in WebKit, then we don't need to mess with include paths in gitg.
Comment 17 jessevdk@gmail.com 2014-08-13 20:59:45 UTC
If you want to keep installing in a separate prefix work well, you can do what we do in libgit2-glib, which is simply installing the gir in $(datadir)/gir-1.0 (note, not pkgdatadir). This has worked out well for us, although it would theoretically be possible that some distro's would change the gir installation dir. In practice it's not an issue.
Comment 18 Michael Catanzaro 2014-08-14 14:07:19 UTC
(In reply to comment #17)
> If you want to keep installing in a separate prefix work well, you can do what
> we do in libgit2-glib, which is simply installing the gir in $(datadir)/gir-1.0
> (note, not pkgdatadir). This has worked out well for us, although it would
> theoretically be possible that some distro's would change the gir installation
> dir. In practice it's not an issue.

Carlos has pushed a patch that does exactly that.
Comment 19 jessevdk@gmail.com 2014-08-14 16:20:47 UTC
OK, I guess it's fine to push the gitg patch.
Comment 20 Michael Catanzaro 2014-08-14 17:11:01 UTC
Attachment 283318 [details] pushed as a1cdc51 - Support both webkit2gtk-3.0 and webkit2gtk-4.0