Bug 184376

Summary: [WPE] Improve include hierarchy
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WPE WebKitAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, commit-queue, mcatanzaro, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Mac   
OS: Linux   
Bug Depends on:    
Bug Blocks: 178894    
Attachments:
Description Flags
Patch
none
Patch none

Description Michael Catanzaro 2018-04-06 17:48:37 PDT
Currently we have:

/usr/include/wpe-0.1/WPE/JavaScriptCore (see bug #184375)
/usr/include/wpe-0.1/WPE/jsc
/usr/include/wpe-0.1/WPE/wpe

And in the pkg-config:

Cflags: -I${includedir}/wpe-0.1 -I${includedir}/wpe-0.1/WPE

In contrast to WebKitGTK+:

/usr/include/webkit2gtk-4.0/JavaScriptCore
/usr/include/webkit2gtk-4.0/jsc (hopefully, haven't checked trunk :)
/usr/include/webkit2gtk-4.0/webkit2
/usr/include/webkit2gtk-4.0/webkitdom

Cflags: -I${includedir}/webkitgtk-4.0

It's desirable to eliminate the unnecessary WPE subdirectory. In theory, this could cause problems if WebKitGTK+ and WPE ever get pulled into the same include path, since the jsc headers could conflict, but such a case is guaranteed to explode at runtime due to symbol collisions, so this shouldn't ever be a problem in practice.
Comment 1 Michael Catanzaro 2018-04-06 19:22:10 PDT
(In reply to Michael Catanzaro from comment #0)
> In theory,
> this could cause problems if WebKitGTK+ and WPE ever get pulled into the
> same include path, since the jsc headers could conflict

Sorry, I wasn't thinking properly. There's no difference because wpe-0.1/WPE was already previously in the Cflags.
Comment 2 Michael Catanzaro 2018-04-06 19:26:33 PDT
Created attachment 337411 [details]
Patch
Comment 3 Michael Catanzaro 2018-04-06 19:28:44 PDT
This will break any applications including headers as:

#include <WPE/wpe/webkit.h>

rather than:

#include <wpe/webkit.h>

Now only the later style is permitted.
Comment 4 Michael Catanzaro 2018-04-07 18:29:38 PDT
This kinda-sorta conflicts with https://github.com/WebPlatformForEmbedded/WPEBackend/issues/10. It should be "fine" as in no build failures, but it's going to result in the headers being installed into the same place. We should still do this, but I further suggest switching WebKit from using /usr/include/wpe-0.1 to /usr/include/wpe-webkit-0.1 to further disambiguate.

Alternative schemes are possible, of course.
Comment 5 Michael Catanzaro 2018-04-09 07:13:16 PDT
Let's change the include path to use wpe-webkit-0.1 instead of wpe-0.1, so we can have separate API versions for WebKit and libwpe.
Comment 6 Michael Catanzaro 2018-04-09 18:24:11 PDT
Created attachment 337568 [details]
Patch
Comment 7 WebKit Commit Bot 2018-04-12 08:51:16 PDT
Comment on attachment 337568 [details]
Patch

Clearing flags on attachment: 337568

Committed r230576: <https://trac.webkit.org/changeset/230576>
Comment 8 WebKit Commit Bot 2018-04-12 08:51:17 PDT
All reviewed patches have been landed.  Closing bug.