Bug 184376 - [WPE] Improve include hierarchy
Summary: [WPE] Improve include hierarchy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks: 178894
  Show dependency treegraph
 
Reported: 2018-04-06 17:48 PDT by Michael Catanzaro
Modified: 2018-04-12 08:51 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.22 KB, patch)
2018-04-06 19:26 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (3.22 KB, patch)
2018-04-09 18:24 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.