Bug 219835 - [CMake][WPE] Remove unconditional WebCore dependency to WPEBackend-FDO
Summary: [CMake][WPE] Remove unconditional WebCore dependency to WPEBackend-FDO
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CMake (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-13 08:52 PST by Philippe Normand
Modified: 2020-12-15 02:14 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.81 KB, patch)
2020-12-13 08:53 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (1.89 KB, patch)
2020-12-14 10:48 PST, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2020-12-13 08:52:11 PST
This dependency should be enabled only when client-side video or audio rendering is enabled.
Comment 1 Philippe Normand 2020-12-13 08:53:46 PST
Created attachment 416117 [details]
Patch
Comment 2 Pablo Saavedra 2020-12-14 10:29:19 PST
Depending on the toolchain is composed (for example, I'm thinking about in SDK generated by the Yocto meta-webkit layout what includes the libWPE as a system library) we could  get such build instruction for an object:

```
arm-poky-linux-gnueabi-g++   -march=armv7-a -mthumb -mfpu=neon -mfloat-abi=hard -fstack-protector-strong  -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi --sysroot=/home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi  -DBUILDING_WITH_CMAKE=1
...
...
-isystem /home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi/usr/include/wpe-fdo-1.0
...
-isystem /home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi/usr/include/wpe-1.0
...
-c DerivedSources/WebCore/unified-sources/UnifiedSource-3c72abbe-40.cpp
```

This lead in a in a conflict during the headers lookup due to a collision in the "version.h" files name included in both libraries:
 

In file included from /home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi/usr/include/wpe-1.0/wpe/wpe.h:43,
                 from ../../Source/WebCore/platform/libwpe/PlatformKeyboardEventLibWPE.cpp:32,
                 from DerivedSources/WebCore/unified-sources/UnifiedSource-3c72abbe-39.cpp:8:
/home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi/usr/include/wpe-fdo-1.0/wpe/version.h:27:2: error: #error "Only <wpe/fdo.h> can be included directly."
   27 | #error "Only <wpe/fdo.h> can be included directly."


The wpe-1.0/wpe/wpe.h is looking for the version.h header from wpe-1.0 but it finds the wpe-fdo-1.0/wpe/version.h because the WPEBackend-FDO lib is defined before as a system lib before than the libWPE library.


On way to avoid this it could be just ensure the right order if the -isystem flags by prepending (better say: appending before) the LibWPE system lib: `list(APPEND WebCore_LIBRARIES ${WPEBACKEND_FDO_LIBRARIES})` before adding the WPEbackend-fdo system libraries. Like here:

```
Index: Source/WebCore/PlatformWPE.cmake
===================================================================
--- Source/WebCore/PlatformWPE.cmake	(revision 270767)
+++ Source/WebCore/PlatformWPE.cmake	(working copy)
@@ -60,7 +60,6 @@
     ${GLIB_LIBRARIES}
     ${LIBTASN1_LIBRARIES}
     ${UPOWERGLIB_LIBRARIES}
-    ${WPEBACKEND_FDO_LIBRARIES}
 )
 
 list(APPEND WebCore_SYSTEM_INCLUDE_DIRECTORIES
@@ -69,10 +68,15 @@
     ${GLIB_INCLUDE_DIRS}
     ${LIBTASN1_INCLUDE_DIRS}
     ${UPOWERGLIB_INCLUDE_DIRS}
-    ${WPEBACKEND_FDO_INCLUDE_DIRS}
 )
 
+if (USE_WPE_VIDEO_PLANE_DISPLAY_DMABUF OR USE_WPEBACKEND_FDO_AUDIO_EXTENSION)
+    list(APPEND WebCore_LIBRARIES ${WPEBACKEND_FDO_LIBRARIES})
+    list(APPEND WebCore_SYSTEM_INCLUDE_DIRECTORIES ${WPE_INCLUDE_DIRS})
+    list(APPEND WebCore_SYSTEM_INCLUDE_DIRECTORIES ${WPEBACKEND_FDO_INCLUDE_DIRS})
+endif ()
+
 if (USE_OPENXR)
     list(APPEND WebCore_LIBRARIES ${OPENXR_LIBRARIES})
     list(APPEND WebCore_SYSTEM_INCLUDE_DIRECTORIES ${OPENXR_INCLUDE_DIRS})
-endif ()
\ No newline at end of file
+endif ()
```

WDYT?
Comment 3 Philippe Normand 2020-12-14 10:48:15 PST
Created attachment 416171 [details]
Patch
Comment 4 Pablo Saavedra 2020-12-14 12:03:01 PST
Comment on attachment 416171 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416171&action=review

LGTM

> Source/WebCore/PlatformWPE.cmake:75
> +    list(APPEND WebCore_SYSTEM_INCLUDE_DIRECTORIES ${WPE_INCLUDE_DIRS})

thanks
Comment 5 EWS 2020-12-15 02:14:47 PST
Committed r270826: <https://trac.webkit.org/changeset/270826>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416171 [details].