Re: [Buildroot] [PATCH v2 2/4] package/cog: depend on wpebackend-fdo only if needed

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

Hello,

On Thu, 27 Jun 2024 22:33:30 +0300
Adrian Perez de Castro <aperez@igalia.com> wrote:

> Make the Cog headless platform plug-in selectable, allowing to configure
> the build without any plug-in at all. When all plug-ins are disabled,
> Cog does not require wpebackend-fdo at build time, and it is still
> able to use its built-in "fallback" support to load other WPE backends
> like wpebackend-rdk.

This seems to makes sense to me, but I completely fail to understand
the complicated implementation you are proposing below.

There is in fact something already super wrong in package/cog: it has
wpebackend-fdo in COG_DEPENDENCIES, but it doesn't select it. It works
because it depends on wpewebkit, which itself selects wpebackend-fdo,
but that's really wrong. This should be fixed in a first patch.

> Signed-off-by: Adrian Perez de Castro <aperez@igalia.com>
> ---
>  package/cog/Config.in | 28 ++++++++++++++++++++++++++--
>  package/cog/cog.mk    | 13 ++++++++++---
>  2 files changed, 36 insertions(+), 5 deletions(-)
> 
> ---
> v1 -> v2:
> - Unchanged.
> 
> diff --git a/package/cog/Config.in b/package/cog/Config.in
> index d2a910f9b89..d706b045b15 100644
> --- a/package/cog/Config.in
> +++ b/package/cog/Config.in
> @@ -19,6 +19,10 @@ config BR2_PACKAGE_COG
>  
>  if BR2_PACKAGE_COG
>  
> +config BR2_PACKAGE_COG_NEEDS_WPEBACKEND_FDO
> +	bool
> +	default n

I believe this is not needed.

>  config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
>  	string "home uri"
>  	default "https://wpewebkit.org"
> @@ -30,6 +34,8 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
>  config BR2_PACKAGE_COG_PLATFORM_FDO
>  	bool "Wayland backend"
>  	default y
> +	depends on BR2_PACKAGE_WPEBACKEND_FDO
> +	select BR2_PACKAGE_COG_NEEDS_WPEBACKEND_FDO

If the Wayland backend needs wpebackend-fdo, just select it:

	select BR2_PACKAGE_WPEBACKEND_FDO

>  	select BR2_PACKAGE_LIBXKBCOMMON
>  	select BR2_PACKAGE_WAYLAND_PROTOCOLS
>  	help
> @@ -43,6 +49,8 @@ config BR2_PACKAGE_COG_PLATFORM_DRM
>  	depends on BR2_PACKAGE_HAS_LIBGBM
>  	depends on BR2_PACKAGE_LIBGBM_HAS_FEATURE_DMA_BUF
>  	depends on BR2_PACKAGE_LIBGBM_HAS_FEATURE_FORMAT_MODIFIER_PLANE_COUNT
> +	depends on BR2_PACKAGE_WPEBACKEND_FDO
> +	select BR2_PACKAGE_COG_NEEDS_WPEBACKEND_FDO

If the DRM backend needs wpebackend-fdo, just select it:

	select BR2_PACKAGE_WPEBACKEND_FDO

>  	select BR2_PACKAGE_LIBDRM
>  	select BR2_PACKAGE_LIBINPUT
>  	help
> @@ -50,16 +58,32 @@ config BR2_PACKAGE_COG_PLATFORM_DRM
>  	  with video drivers that support kernel mode-setting (KMS)
>  	  via the DRM user-space API.
>  
> +config BR2_PACKAGE_COG_PLATFORM_HEADLESS
> +	bool "Headless backend"
> +	depends on BR2_PACKAGE_WPEBACKEND_FDO
> +	select BR2_PACKAGE_COG_NEEDS_WPEBACKEND_FDO

Same, just select wpebackend-fdo.

> +	help
> +	  Enable the headless platform backend.
> +
>  config BR2_PACKAGE_COG_USE_SYSTEM_DBUS
>  	bool "expose system D-Bus control interface"
>  	help
>  	  Expose remote control interface on system bus
>  
> -comment "DRM platform needs EGL and GBM"
> +comment "Headless platform needs wpebackend-fdo"
> +	depends on \
> +		!BR2_PACKAGE_WPEBACKEND_FDO
> +
> +comment "DRM platform needs EGL, GBM, wpebackend-fdo"
>  	depends on \
>  		!BR2_PACKAGE_HAS_LIBEGL || \
>  		!BR2_PACKAGE_HAS_LIBGBM || \
>  		!BR2_PACKAGE_LIBGBM_HAS_FEATURE_DMA_BUF || \
> -		!BR2_PACKAGE_LIBGBM_HAS_FEATURE_FORMAT_MODIFIER_PLANE_COUNT
> +		!BR2_PACKAGE_LIBGBM_HAS_FEATURE_FORMAT_MODIFIER_PLANE_COUNT || \
> +		!BR2_PACKAGE_WPEBACKEND_FDO
> +
> +comment "Wayland platform needs wpebackend-fdo"
> +	depends on \
> +		!BR2_PACKAGE_WPEBACKEND_FDO

Drop all those changes.

>  endif
> diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> index 7f680bb7005..166f095118b 100644
> --- a/package/cog/cog.mk
> +++ b/package/cog/cog.mk
> @@ -8,7 +8,7 @@ COG_VERSION = 0.18.4
>  COG_SITE = https://wpewebkit.org/releases
>  COG_SOURCE = cog-$(COG_VERSION).tar.xz
>  COG_INSTALL_STAGING = YES
> -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo wayland
> +COG_DEPENDENCIES = dbus wpewebkit wayland
>  COG_LICENSE = MIT
>  COG_LICENSE_FILES = COPYING
>  COG_CONF_OPTS = \
> @@ -19,8 +19,6 @@ COG_CONF_OPTS = \
>  	-Dcog_home_uri='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))' \
>  	-Dplatforms='$(subst $(space),$(comma),$(strip $(COG_PLATFORMS_LIST)))'
>  
> -COG_PLATFORMS_LIST = headless
> -
>  ifeq ($(BR2_PACKAGE_WESTON),y)
>  COG_CONF_OPTS += -Dwayland_weston_direct_display=true
>  COG_DEPENDENCIES += weston
> @@ -28,6 +26,11 @@ else
>  COG_CONF_OPTS += -Dwayland_weston_direct_display=false
>  endif
>  
> +ifeq ($(BR2_PACKAGE_COG_PLATFORM_HEADLESS),y)
> +COG_PLATFORMS_LIST += headless
> +COG_DEPENDENCIES += wpebackend-fdo
> +endif
> +
>  ifeq ($(BR2_PACKAGE_COG_PLATFORM_FDO),y)
>  COG_PLATFORMS_LIST += wayland
>  COG_DEPENDENCIES += libxkbcommon wayland-protocols
> @@ -48,4 +51,8 @@ ifeq ($(BR2_PACKAGE_LIBMANETTE),y)
>  COG_DEPENDENCIES += libmanette
>  endif
>  
> +ifeq ($(BR2_PACKAGE_COG_NEEDS_WPEBACKEND_FDO),y)
> +COG_DEPENDENCIES += wpebackend-fdo
> +endif

Drop this, and instead do:

ifeq ($(BR2_PACKAGE_COG_PLATFORM_HEADLESS),y)
COG_PLATFORMS_LIST += headless
COG_DEPENDENCIES += wpebackend-fdo
endif

ifeq ($(BR2_PACKAGE_COG_PLATFORM_FDO),y)
COG_PLATFORMS_LIST += wayland
COG_DEPENDENCIES += libxkbcommon wayland-protocols wpebackend-fdo
endif

ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
COG_PLATFORMS_LIST += drm
COG_DEPENDENCIES += libdrm libinput libgbm libegl udev wpebackend-fdo
endif

Could you rework a v3 according to those suggestions?

Thanks a lot!

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