Re: [Buildroot] [PATCH 2/2] package/qt6/qt6base: allow selecting the host Qt Print Support module

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello,

On Wed,  4 Sep 2024 14:20:38 +0200
Roy Kollen Svendsen <roykollensvendsen@gmail.com> wrote:

> According to configure.cmake printsupport requires widgets:
> 
> qt_feature("printsupport" PRIVATE
>     LABEL "Qt PrintSupport"
>     CONDITION QT_FEATURE_widgets
>     SECTION "Module"
>     PURPOSE "Provides the Qt PrintSupport module."
> )
> 
> So we let print support automatically select widgets support.
> 
> Signed-off-by: Roy Kollen Svendsen <roykollensvendsen@gmail.com>

This should be PATCH 1/2 in the series, because your patch adding
BR2_PACKAGE_HOST_QT6TOOLS_LINGUIST_TOOLS uses this new
BR2_PACKAGE_HOST_QT6BASE_PRINTSUPPORT option.

> ---
>  package/qt6/qt6base/Config.in.host | 11 +++++++++++
>  package/qt6/qt6base/qt6base.mk     | 16 ++++++++++++++--
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/package/qt6/qt6base/Config.in.host b/package/qt6/qt6base/Config.in.host
> index 46fb668817..8f0bc1f070 100644
> --- a/package/qt6/qt6base/Config.in.host
> +++ b/package/qt6/qt6base/Config.in.host
> @@ -8,6 +8,13 @@ config BR2_PACKAGE_HOST_QT6BASE_GUI
>  config BR2_PACKAGE_HOST_QT6BASE_NETWORK
>  	bool
>  
> +# Select this if you need host qt6 tools that require the Qt Print Support
> +# module
> +# (e.g. Qt Tools Linguist Tools).

I don't think that this really makes sense. We don't really care about
how is using this (in the future, it may be other things). We care
about what the option does... but what it does is actually pretty
obvious from the option name. I think the only comment is to clarify
that yes, widget support is needed for the print support, which isn't
really obvious.

> +config BR2_PACKAGE_HOST_QT6BASE_PRINTSUPPORT
> +	bool
> +	select BR2_PACKAGE_HOST_QT6BASE_WIDGETS
> +
>  # Select this if you need host qt6 tools that require the Qt::Test module
>  # (e.g. qmltestrunner).
>  config BR2_PACKAGE_HOST_QT6BASE_TEST
> @@ -17,3 +24,7 @@ config BR2_PACKAGE_HOST_QT6BASE_TEST
>  # (e.g. qhelpgenerator).
>  config BR2_PACKAGE_HOST_QT6BASE_SQL
>  	bool
> +
> +# The Qt Widgets module is required by the Qt Print Support module

And this comment would become useless.

> +config BR2_PACKAGE_HOST_QT6BASE_WIDGETS
> +	bool

Isn't widget support dependent on GUI support?

> diff --git a/package/qt6/qt6base/qt6base.mk b/package/qt6/qt6base/qt6base.mk
> index 4c0392f92d..271f464115 100644
> --- a/package/qt6/qt6base/qt6base.mk
> +++ b/package/qt6/qt6base/qt6base.mk
> @@ -113,10 +113,8 @@ HOST_QT6BASE_CONF_OPTS += \
>  	-DFEATURE_png=OFF \
>  	-DFEATURE_gif=OFF \
>  	-DFEATURE_jpeg=OFF \
> -	-DFEATURE_printsupport=OFF \
>  	-DFEATURE_kms=OFF \
>  	-DFEATURE_fontconfig=OFF \
> -	-DFEATURE_widgets=OFF \
>  	-DFEATURE_libinput=OFF \
>  	-DFEATURE_tslib=OFF \
>  	-DFEATURE_eglfs=OFF
> @@ -131,6 +129,13 @@ else
>  HOST_QT6BASE_CONF_OPTS += -DFEATURE_network=OFF
>  endif
>  
> +# The Qt Print Support module is required for building Qt Tools Linguist Tools

Not needed, it could be other things.

> +ifeq ($(BR2_PACKAGE_HOST_QT6BASE_PRINTSUPPORT),y)
> +HOST_QT6BASE_CONF_OPTS += -DFEATURE_printsupport=ON
> +else
> +HOST_QT6BASE_CONF_OPTS += -DFEATURE_printsupport=OFF
> +endif
> +
>  # We need host qt6base with Sql support for host-qt6tools to generate the
>  # qhelpgenerator host tool. qt6tools will fail to build if qhelpgenerator is not
>  # available.
> @@ -150,6 +155,13 @@ else
>  HOST_QT6BASE_CONF_OPTS += -DFEATURE_testlib=OFF
>  endif
>  
> +# The Qt Widgets module is required by the Qt Print Support module

Also not very relevant comment IMO.

> +ifeq ($(BR2_PACKAGE_HOST_QT6BASE_WIDGETS),y)
> +HOST_QT6BASE_CONF_OPTS += -DFEATURE_widgets=ON
> +else
> +HOST_QT6BASE_CONF_OPTS += -DFEATURE_widgets=OFF
> +endif

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot