Re: [Buildroot] [PATCH v2 1/4] package/wpebackend-rdk: new package

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

Hello Adrian,

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

> The wpebackend-rdk can be used as an alternative to wpebackend-fdo in
> some cases, and also supports a few additional configurations. For now
> only the Wayland-EGL and Broadcom RaspberryPi userland implementations
> can be enabled. The former is useful for testing, and the latter allows
> using WPE WebKit directly on top of Dispmanx in combination with the
> rpi-userland package in supported RaspberryPi models.
> 
> This only adds the new package, follow-up commits will un-hardcode the
> usage of wpebackend-fdo and allow selecting either which of both to
> build.
> 
> Signed-off-by: Adrian Perez de Castro <aperez@igalia.com>

I wanted to merge this, but unfortunately:

BR2_arm=y
BR2_cortex_a9=y
BR2_ARM_ENABLE_VFP=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_RPI_USERLAND=y
BR2_PACKAGE_WPEBACKEND_RDK=y
# BR2_TARGET_ROOTFS_TAR is not set

fails to build with:

In file included from /home/thomas/projets/buildroot/output/build/wpebackend-rdk-4.4.1/src/bcm-rpi/view-backend.cpp:30:
/home/thomas/projets/buildroot/output/build/wpebackend-rdk-4.4.1/src/input/Libinput/LibinputServer.h:37:10: fatal error: libudev.h: No such file or directory
   37 | #include <libudev.h>
      |          ^~~~~~~~~~~
compilation terminated.

as far as I can see, LibinputServer.h includes <libinput.h> and
<libudev.h>:

#ifndef KEY_INPUT_HANDLING_VIRTUAL
#include <libudev.h>
#include <libinput.h>
#else
#include <virtualinput.h>
#endif

whenever KEY_INPUT_HANDLING_VIRTUAL is *NOT* defined. And
KEY_INPUT_HANDLING_VIRTUAL is only defined when USE_VIRTUAL_KEYBOARD=ON.

I believe you should also properly use the USE_INPUT_UDEV and
USE_INPUT_WAYLAND options.

find_package(Libxkbcommon REQUIRED)

libxkbcommon is also a mandatory dependency. Even though it is
indirectly pulled in by libwpe, it should be expressed as a direct
dependency of wpebackend-rdk.

> ---
>  package/Config.in                          |  1 +
>  package/wpebackend-rdk/Config.in           | 57 ++++++++++++++++++++++
>  package/wpebackend-rdk/wpebackend-rdk.hash |  5 ++
>  package/wpebackend-rdk/wpebackend-rdk.mk   | 43 ++++++++++++++++
>  4 files changed, 106 insertions(+)

Please add yourself in the DEVELOPERS file for this package.


> diff --git a/package/wpebackend-rdk/Config.in b/package/wpebackend-rdk/Config.in
> new file mode 100644
> index 00000000000..6125911bec9
> --- /dev/null
> +++ b/package/wpebackend-rdk/Config.in
> @@ -0,0 +1,57 @@
> +comment "wpebackend-rdk needs a toolchain w/ C++, wchar, threads, dynamic library, and rpi-userland or an OpenGL-capable Wayland backend"
> +	depends on !(BR2_PACKAGE_RPI_USERLAND || BR2_PACKAGE_HAS_LIBEGL_WAYLAND) \
> +		|| !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR \
> +		|| !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS

Please split this into two comments, one for the toolchain things, one
for the rpi-userland || wayland EGL.

Also, the option BR2_PACKAGE_HAS_LIBEGL_WAYLAND is gone, see commit
4fad6b3c58abd20c533b67b85658bc66d015110b. You can use
BR2_PACKAGE_HAS_LIBEGL && BR2_PACKAGE_WAYLAND instead. This needs to be
fixed globally.

> +config BR2_PACKAGE_WPEBACKEND_RDK
> +	bool "wpebackend-rdk"
> +	depends on !BR2_STATIC_LIBS # wayland
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # wayland, libglib2
> +	depends on BR2_USE_WCHAR # libglib2
> +	depends on BR2_USE_MMU # libglib2
> +	depends on BR2_PACKAGE_RPI_USERLAND || BR2_PACKAGE_HAS_LIBEGL_WAYLAND
> +	select BR2_PACKAGE_LIBGLIB2
> +	select BR2_PACKAGE_LIBWPE
> +	help
> +	  WPEBackend interface supporting a number of configurations.

Would be nice to have a slightly extended explanation for the poor
souls like that are very confused by libwpe vs. wpebackend-fdo vs.
wpebackend-rdk vs. wpewebkit vs. webkitgtk.

> +if BR2_PACKAGE_WPEBACKEND_RDK
> +
> +choice
> +	prompt Implementation

Double quotes around "Implementation". Maybe it should be "Backend"
instead of "Implementation"?

> +	default BR2_PACKAGE_WPEBACKEND_RDK_IMPL_BRCM_RPI if BR2_PACKAGE_RPI_USERLAND
> +	default BR2_PACKAGE_WPEBACKEND_RDK_IMPL_WAYLAND_EGL if BR2_PACKAGE_HAS_LIBEGL_WAYLAND
> +
> +comment "brcm-rpi needs rpi-userland"
> +	depends on !BR2_PACKAGE_RPI_USERLAND || !BR2_arm

better expressed as:

	depends on BR2_arm
	depends on !BR2_PACKAGE_RPI_USERLAND

> +
> +config BR2_PACKAGE_WPEBACKEND_RDK_IMPL_BRCM_RPI
> +	bool "brcm-rpi"
> +	depends on BR2_arm
> +	depends on BR2_PACKAGE_RPI_USERLAND
> +	help
> +	  Build RaspberryPi support using Broadcom's rpi-userland.
> +
> +comment "wayland-egl needs an EGL capable Wayland implementation"
> +	depends on !BR2_PACKAGE_HAS_LIBEGL_WAYLAND
> +
> +config BR2_PACKAGE_WPEBACKEND_RDK_IMPL_WAYLAND_EGL
> +	bool "wayland-egl"
> +	depends on BR2_PACKAGE_HAS_LIBEGL_WAYLAND
> +	help
> +	  Build Wayland-EGL support.
> +
> +endchoice
> +
> +if BR2_PACKAGE_WPEBACKEND_RDK_IMPL_BRCM_RPI
> +
> +config BR2_PACKAGE_WPEBACKEND_RDK_LIBINPUT
> +	bool "libinput support"
> +	select BR2_PACKAGE_LIBINPUT
> +	help
> +	  Support keyboard and mouse input through libinput

Why is the keyboard/mouse support with libinput only available with the
brcm-rpi backend? Reading the CMakeLists.txt, I don't really see any
relationship.

Could you fix up those different issues and submit a v3 ?

Thanks a lot!

Thomas Petazzoni
-- 
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