| Message ID | 20240627193335.4069574-3-aperez@igalia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
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
Hello, On Fri, 02 Aug 2024 23:06:44 +0200 Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote: > 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. Oops O:-) > 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. You're right! > > 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 With the additional BR2_PACKAGE_COG_NEEDS_WPEBACKEND_FDO configuration symbol I was trying to avoid adding wpebackend-fdo more than once to COG_DEPENDENCIES. If it's fine to have it there (as it seems to be the case) then yes, I am aboard for all the simplifications. > Could you rework a v3 according to those suggestions? Yes, will re-spin this into a v3 at some point. Thanks for all the recommendations! Cheers, —Adrián
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 + 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 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 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 + 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 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 + $(eval $(meson-package))
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. 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.