WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
178100
[WPE] Header cairo.h is used in GLib API headers but cannot be found
https://bugs.webkit.org/show_bug.cgi?id=178100
Summary
[WPE] Header cairo.h is used in GLib API headers but cannot be found
Adrian Perez
Reported
2017-10-09 16:52:31 PDT
When a program tries to use the GLib-based API for WPE (see
bug #178090
for a patch to make it installable), trying to use the <wpe/webkit.h> header will result in the compilation error shown below, due to “cairo.h” not being in the headers search path. --- In file included from /home/aperez/devel/wpe/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/include/wpe-0.1/WPE/wpe/webkit.h:46:0, from /home/aperez/devel/wpe/buildroot/output/build/dinghy-custom/dinghy.c:8: /home/aperez/devel/wpe/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/include/wpe-0.1/WPE/wpe/WebKitFaviconDatabase.h:27:19: fatal error: cairo.h: No such file or directory #include <cairo.h> ^
Attachments
Patch
(1.40 KB, patch)
2017-10-09 17:00 PDT
,
Adrian Perez
no flags
Details
Formatted Diff
Diff
Patch
(1.83 KB, patch)
2017-10-10 05:21 PDT
,
Adrian Perez
no flags
Details
Formatted Diff
Diff
Patch
(3.00 KB, patch)
2017-10-10 11:34 PDT
,
Adrian Perez
no flags
Details
Formatted Diff
Diff
Patch
(4.09 KB, patch)
2017-10-10 13:08 PDT
,
Adrian Perez
no flags
Details
Formatted Diff
Diff
Patch
(1.33 KB, patch)
2017-10-10 13:54 PDT
,
Adrian Perez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Adrian Perez
Comment 1
2017-10-09 16:54:01 PDT
I think we can fix this by adding “cairo” to the “Requires.private” field of the .pc generated to be used by pkg-config. I'll try that out and submit a patch.
Adrian Perez
Comment 2
2017-10-09 17:00:14 PDT
Created
attachment 323245
[details]
Patch
Zan Dobersek
Comment 3
2017-10-10 04:37:02 PDT
Can we instead just use a forward declaration of cairo_surface_t, and drop the cairo.h include?
Adrian Perez
Comment 4
2017-10-10 04:48:54 PDT
(In reply to Zan Dobersek from
comment #3
)
> Can we instead just use a forward declaration of cairo_surface_t, and drop > the cairo.h include?
Very likely! I'll give that a try and update the patch accordingly once I am sure it works. :-)
Adrian Perez
Comment 5
2017-10-10 05:21:54 PDT
Created
attachment 323299
[details]
Patch
Adrian Perez
Comment 6
2017-10-10 05:24:52 PDT
(In reply to Adrian Perez from
comment #4
)
> (In reply to Zan Dobersek from
comment #3
) > > Can we instead just use a forward declaration of cairo_surface_t, and drop > > the cairo.h include? > > Very likely! I'll give that a try and update the patch accordingly > once I am sure it works. :-)
Worked perfectly, the new version of the patch adds only the forward declaration and removes the #include \o/
Adrian Perez
Comment 7
2017-10-10 05:26:26 PDT
Ouch, we'll have to add “#include <cairo.h>” to the .cpp files which make use of “WebKitFaviconDatabase.h”, as Good Guy EWS™ has found out.
Adrian Perez
Comment 8
2017-10-10 11:34:17 PDT
Created
attachment 323326
[details]
Patch
Build Bot
Comment 9
2017-10-10 11:35:19 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See
http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Zan Dobersek
Comment 10
2017-10-10 11:42:18 PDT
Comment on
attachment 323326
[details]
Patch Thanks, I appreciate the change.
WebKit Commit Bot
Comment 11
2017-10-10 12:09:54 PDT
Comment on
attachment 323326
[details]
Patch Clearing flags on attachment: 323326 Committed
r223136
: <
http://trac.webkit.org/changeset/223136
>
WebKit Commit Bot
Comment 12
2017-10-10 12:09:56 PDT
All reviewed patches have been landed. Closing bug.
Michael Catanzaro
Comment 13
2017-10-10 12:39:08 PDT
But you need to update the GTK header as well. We have to keep these headers completely in sync. Only exception should be symbols that do not exist in one port or the other.
Michael Catanzaro
Comment 14
2017-10-10 12:41:10 PDT
Reverted
r223136
for reason: Forgot to update GTK API header Committed
r223138
: <
http://trac.webkit.org/changeset/223138
>
Michael Catanzaro
Comment 15
2017-10-10 12:42:45 PDT
(This was probably overkill for a rollout, but it's easier for backporting to land one fixed patch instead of an incremental fixup.)
Adrian Perez
Comment 16
2017-10-10 12:52:38 PDT
(In reply to Michael Catanzaro from
comment #13
)
> But you need to update the GTK header as well. We have to keep these headers > completely in sync. Only exception should be symbols that do not exist in > one port or the other.
I'll prepare a new version of the patch that covers both WPE and GTK+.
Adrian Perez
Comment 17
2017-10-10 13:08:37 PDT
Created
attachment 323336
[details]
Patch
Michael Catanzaro
Comment 18
2017-10-10 13:48:22 PDT
You shouldn't have to #include a new header to use any types from our API. Zan's instincts to avoid unneeded forward declarations are good usually, but not in public headers IMO. Thanks to gtk-doc for catching this problem on the GTK EWS. :P
Adrian Perez
Comment 19
2017-10-10 13:54:13 PDT
Created
attachment 323347
[details]
Patch
Michael Catanzaro
Comment 20
2017-10-10 14:26:44 PDT
Comment on
attachment 323347
[details]
Patch Perfect!
WebKit Commit Bot
Comment 21
2017-10-10 14:32:56 PDT
Comment on
attachment 323347
[details]
Patch Clearing flags on attachment: 323347 Committed
r223146
: <
http://trac.webkit.org/changeset/223146
>
WebKit Commit Bot
Comment 22
2017-10-10 14:32:58 PDT
All reviewed patches have been landed. Closing bug.
Michael Catanzaro
Comment 23
2017-10-11 05:42:11 PDT
Reverted
r223146
for reason: Better to not expose cairo in the WPE API Committed
r223174
: <
http://trac.webkit.org/changeset/223174
>
Michael Catanzaro
Comment 24
2017-10-11 05:42:34 PDT
We'll address this in
bug #178157
instead.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug