[Buildroot] [PATCH] package/qt5/qt5webkit: fix generated artifacts

[Giulio Benetti]

Hi Thomas,

On 22/09/22 23:45, Thomas Ballasi wrote:
> Generated artifacts of the installation process were wrongly located,
> causing packages using qt5webkit (qt-webkit-kiosk and python-pyqt5) to
> fail at build time. The changes aims at fixing this issue.

"The changes aims at fixing this issue." should be:
"Let's add a patch that:"

and then you list the points below using the present verb

> There were three main issues occuring during the build:
> 
> 1. *.pri files were wrongly located in the host's and target's sysroot
>     directores while buildroot implements its own mkspecs directory.
>     By setting KDE_INSTALL_USE_QT_SYS_PATHS on, mkspecs modules are now
>     being installed in host's data dir (ECM_MKSPECS_INSTALL_DIR is set
>     accordingly @ line 102 in Source/cmake/ECMGeneratePriFile.cmake).
>     This also required to prevent using the CMake package's default
>     DATADIR variable as it enforces to install under the sysroot
>     directory.
> 
> 2. *.pri files' content had hardcoded include and library paths which
>     has been corrected by setting on KDE_INSTALL_USE_QT_SYS_PATHS as
>     their content is written according to this value (see line 514 and
>     739 in file Source/WebKit/PlatformQt.cmake).
> 
> 3. *.h files were located a directory below where supposed (inside qt5/
>     directory). This was caused by using DATADIR which assumed include
>     files were to be located in sysroot/usr/include/.
>     Disabling this variable by removing it from build options leads to a
>     correct behavior.
> 
> Regression happened when qt5webkit started using cmake-package at commit
> df0b0fe6919c0d0f3750f439a3cfa765232bd569.

What is the upstream status of this patch? Can you point here the URL
of the pending patch?

> More info @ https://bugs.buildroot.org/show_bug.cgi?id=14606

Here ^^^ it should be:
Fixes:
https://bugs.buildroot.org/show_bug.cgi?id=14606

> Signed-off-by: Thomas Ballasi <thomas.ballasi at savoirfairelinux.com>
> ---
>   ...-set-KDE_INSTALL_USE_QT_SYS_PATHS-on.patch | 42 +++++++++++++++++++
>   package/qt5/qt5webkit/qt5webkit.mk            |  7 ++++
>   2 files changed, 49 insertions(+)
>   create mode 100644 package/qt5/qt5webkit/0007-cmake-set-KDE_INSTALL_USE_QT_SYS_PATHS-on.patch
> 
> diff --git a/package/qt5/qt5webkit/0007-cmake-set-KDE_INSTALL_USE_QT_SYS_PATHS-on.patch b/package/qt5/qt5webkit/0007-cmake-set-KDE_INSTALL_USE_QT_SYS_PATHS-on.patch
> new file mode 100644
> index 0000000000..b65eb305b4
> --- /dev/null
> +++ b/package/qt5/qt5webkit/0007-cmake-set-KDE_INSTALL_USE_QT_SYS_PATHS-on.patch
> @@ -0,0 +1,42 @@
> +From f4950219005b487c18d41ce4e6bc11c4b0e3a20d Mon Sep 17 00:00:00 2001
> +From: Thomas Ballasi <thomas.ballasi at savoirfairelinux.com>
> +Date: Wed, 21 Sep 2022 14:46:40 -0400
> +Subject: [PATCH] cmake: set KDE_INSTALL_USE_QT_SYS_PATHS on
> +
> +This variable is used to save .pri files to directories relative to the
> +host (output/host/mkspecs/modules/) rather than relative to the target
> +itself, which is unwanted behavior.
> +
> +The changes also enables .pri files not to hardcode include and library
> +paths and to use $$QT_MODULE_INCLUDE_BASE and $$QT_MODULE_LIB_BASE.
> +
> +Signed-off-by: Thomas Ballasi <thomas.ballasi at savoirfairelinux.com>

Also here please add:
'[Upstream status: URL of this pending patch]

> +---
> + Source/cmake/OptionsQt.cmake | 8 +-------
> + 1 file changed, 1 insertion(+), 7 deletions(-)
> +
> +diff --git a/Source/cmake/OptionsQt.cmake b/Source/cmake/OptionsQt.cmake
> +index 1ee60b777106..607c69bd38fe 100644
> +--- a/Source/cmake/OptionsQt.cmake
> ++++ b/Source/cmake/OptionsQt.cmake
> +@@ -998,16 +998,10 @@ feature_summary(WHAT ALL FATAL_ON_MISSING_REQUIRED_PACKAGES)
> + include(ECMQueryQmake)
> +
> + query_qmake(qt_install_prefix_dir QT_INSTALL_PREFIX)
> +-if (CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)
> +-    set(CMAKE_INSTALL_PREFIX "${qt_install_prefix_dir}" CACHE PATH "Install path prefix, prepended onto install directories." FORCE)
> +-endif ()
> ++set(CMAKE_INSTALL_PREFIX "${qt_install_prefix_dir}")
> +
> + include(KDEInstallDirs)
> +
> +-if (NOT qt_install_prefix_dir STREQUAL "${CMAKE_INSTALL_PREFIX}")
> +-    set(KDE_INSTALL_USE_QT_SYS_PATHS OFF)
> +-endif ()
> +-
> + # We split all installed files into 2 components: Code and Data. This is different from
> + # traditional approach with Runtime and Devel, but we need it to fix concurrent installation of
> + # debug and release builds in qmake-based build
> +--
> +2.25.1
> +
> diff --git a/package/qt5/qt5webkit/qt5webkit.mk b/package/qt5/qt5webkit/qt5webkit.mk
> index 6912359674..607c022568 100644
> --- a/package/qt5/qt5webkit/qt5webkit.mk
> +++ b/package/qt5/qt5webkit/qt5webkit.mk
> @@ -57,4 +57,11 @@ QT5WEBKIT_CONF_OPTS += \
>   	-DSHARED_CORE=ON \
>   	-DUSE_LIBHYPHEN=OFF
>   
> +QT5WEBKIT_INSTALL_STAGING_OPTS = install/fast
> +
> +define QT5WEBKIT_INSTALL_TARGET_CMDS
> +	$(TARGET_MAKE_ENV) $(BR2_CMAKE) --install $(QT5WEBKIT_BUILDDIR) \
> +		--prefix $(TARGET_DIR)/usr
> +endef
> +
>   $(eval $(cmake-package))

The patch works correctly, so with commit log improve and the local 
patch with Upstream status pointed:
Reviewed-by: Giulio Benetti <giulio.benetti at benettiengineering.com>

Thanks for contributing!

Best regards
-- 
Giulio Benetti
CEO/CTO at Benetti Engineering sas