Igalia / meta-webkit

Yocto / OpenEmbedded layer for WebKit based engines and browsers
MIT License
127 stars 70 forks source link

wpewebkit: Avoid Qt dependencies if qtwpe is disabled #317

Closed aperezdc closed 3 years ago

aperezdc commented 3 years ago

Avoid pulling additional dependencies declared by cmake_qt5.bbclass when qt5-layer is present but the qtwpe option is disabled.

psaavedra commented 3 years ago

There is an error here:

| -- Configuring done
| CMake Error at Source/WebKit/PlatformWPE.cmake:461 (add_library):
|   Target "qtwpe" links to target "Qt5::Core" but the target was not found.
|   Perhaps a find_package() call is missing for an IMPORTED target, or an
|   ALIAS target is missing?
| Call Stack (most recent call first):
|   Source/cmake/WebKitMacros.cmake:71 (include)
|   Source/WebKit/CMakeLists.txt:401 (WEBKIT_INCLUDE_CONFIG_FILES_IF_EXISTS)
| 
| 
aperezdc commented 3 years ago

Dang, I just realized that the inherit … is parsed as part of the build recipe, before the distro/local configuration files have the chance to set PACKAGECONFIG, so when the line gets parsed PACKAGECONFIG has its default value (and nothing else). We would need a different approach.

aperezdc commented 3 years ago

Okay, I have a way that works, even though it's not particularly pretty.

aperezdc commented 3 years ago

Okay, I have a way that works, even though it's not particularly pretty.

But it didn't... trying now a different approach which completely avoids cmake_qt5.bbclass 😏

psaavedra commented 3 years ago

Okay, I have a way that works, even though it's not particularly pretty.

But it didn't... trying now a different approach which completely avoids cmake_qt5.bbclass smirk

That implies a duplicity of the same variables in poky and meta-webkit. I could foresee some issues in the future if those variables are modified in the future.

If we are interested in control if WPE must be compiled with or without QT5 independently of if the meta-qt layer is added as layers then we can control this using a global variable WPE_ENABLE_QT5:

diff --git a/.gitlab-ci/template/include/local-wpe-qt.conf b/.gitlab-ci/template/include/local-wpe-qt.conf
index 50b817c..9c53258 100644
--- a/.gitlab-ci/template/include/local-wpe-qt.conf
+++ b/.gitlab-ci/template/include/local-wpe-qt.conf
@@ -1,4 +1,4 @@
-PACKAGECONFIG_append_pn-wpewebkit = " qtwpe"
+WPE_ENABLE_QT5 = "yes"

 IMAGE_INSTALL_append = " wpewebkit-qtwpe-qml-plugin qt-wpe-simple-browser"

diff --git a/recipes-browser/wpewebkit/wpewebkit.inc b/recipes-browser/wpewebkit/wpewebkit.inc
index af96b62..7bb2dc7 100644
--- a/recipes-browser/wpewebkit/wpewebkit.inc
+++ b/recipes-browser/wpewebkit/wpewebkit.inc
@@ -13,12 +13,13 @@ DEPENDS = " \
 "

 inherit cmake pkgconfig perlnative python3native
-inherit ${@'cmake_qt5' if 'qt5-layer' in d.getVar('BBFILE_COLLECTIONS').split() else ''}
+inherit ${@'cmake_qt5' if bb.utils.to_boolean(d.getVar("WPE_ENABLE_QT5"), False) else ''}

 export WK_USE_CCACHE="NO"

 PACKAGECONFIG ??= "fetchapi gold indexeddb jit mediasource video webaudio webcrypto woff2 gst_gl remote-inspector openjpeg unified-builds service-worker \
                    ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'systemd', '' ,d)} \
+                  ${@'qtwpe' if bb.utils.to_boolean(d.getVar("WPE_ENABLE_QT5"), False) else ''} \
                   "

 PACKAGECONFIG[reduce-size] = "-DCMAKE_BUILD_TYPE=MinSizeRel,-DCMAKE_BUILD_TYPE=Release,,"
@@ -128,6 +129,8 @@ RRECOMMENDS_${PN} += " \
     shared-mime-info \
     ttf-bitstream-vera \
     ${PN}-web-inspector-plugin \
-    ${PN}-qtwpe-qml-plugin \
     ${@bb.utils.contains('PACKAGECONFIG', 'video', 'gstreamer1.0-plugins-base-meta gstreamer1.0-plugins-good-meta gstreamer1.0-plugins-bad-meta', '', d)} \
 "
+
+RRECOMMENDS_${PN}_append = "${@'wpewebkit-qtwpe-qml-plugin qt-wpe-simple-browser' if bb.utils.to_boolean(d.getVar('WPE_ENABLE_QT5'), False) else ''}"
+
aperezdc commented 3 years ago

@psaavedra I agree that there is no good solution here, unless we could really split building the WPE Qt library into its own real package, instead of having it built as part of the wpewebkit one. While your approach would also work, adding a global makes it impossible to toggle the feature by setting PACKAGECONFIG, and potentially break that expectation that users of meta-webkit would have.

I think the risk of having issues in the future when we pick variables from qmake5_paths.bbclass will be very low (the file has not changed much in the past), and hopefully we can have a separate package for the Qt API—I have filed bug #232228 to track the changes needed in WebKit.

psaavedra commented 3 years ago

@psaavedra I agree that there is no good solution here, unless we could really split building the WPE Qt library into its own real package, instead of having it built as part of the wpewebkit one. While your approach would also work, adding a global makes it impossible to toggle the feature by setting PACKAGECONFIG, and potentially break that expectation that users of meta-webkit would have.

I think the risk of having issues in the future when we pick variables from qmake5_paths.bbclass will be very low (the file has not changed much in the past), and hopefully we can have a separate package for the Qt API—I have filed bug #232228 to track the changes needed in WebKit.

Yes. I'm agree there is not an ideal solution for this. Let's proceed adding these changes and then wait for a rework addin the new wpewebkit-qt package.