Re: [Buildroot] [PATCH v2] package/bmap-writer: add initial package

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

Hello,

On Thu,  2 Jan 2025 14:33:58 +0100
Ayoub Zaki <ayoub.zaki@embetrix.com> wrote:

> Signed-off-by: Ayoub Zaki <ayoub.zaki@embetrix.com>

Thanks for your patch. I applied it, but I had to fix a number of
things. See below my different comments.

> diff --git a/DEVELOPERS b/DEVELOPERS
> index 381bc9f490..2e66e3d8a7 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -3447,3 +3447,6 @@ F:	package/quazip/
>  F:	package/shapelib/
>  F:	package/simple-mail/
>  F:	package/tinc/
> +
> +N:	Ayoub Zaki <ayoub.zaki@embetrix.com>
> +F:	package/bmap-writer/

This file is alphabetically sorted, but you had added this entry at the
end of the file, which isn't good.

> diff --git a/package/bmap-writer/Config.in b/package/bmap-writer/Config.in
> new file mode 100644
> index 0000000000..d65eda918c
> --- /dev/null
> +++ b/package/bmap-writer/Config.in
> @@ -0,0 +1,17 @@
> +config BR2_PACKAGE_BMAP_WRITER
> +	bool "bmap-writer"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_USE_WCHAR
> +	select BR2_PACKAGE_LIBARCHIVE
> +	select BR2_PACKAGE_LIBXML2
> +	help
> +	  bmap-writer is a command-line utility designed to efficiently write
> +	  disk images to storage devices using block mapping (BMAP). It serves
> +	  as a lightweight alternative to the Yocto BMAP tool, specifically for
> +	  embedded systems. Unlike the Yocto BMAP tool, bmap-writer is C++ based,
> +	  does not require Python, and focuses solely on writing an image.

These lines were too long. This is reported by "make check-package", so
I rewrapped them.

> +	  https://github.com/embetrix/bmap-writer
> +
> +comment "bmap-writer needs a toolchain w/ C++, wchar"
> +	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP
> diff --git a/package/bmap-writer/Config.in.host b/package/bmap-writer/Config.in.host
> new file mode 100644
> index 0000000000..d5bfd62f0a
> --- /dev/null
> +++ b/package/bmap-writer/Config.in.host
> @@ -0,0 +1,10 @@
> +config BR2_PACKAGE_HOST_BMAP_WRITER
> +	bool "host bmap-writer"
> +	help
> +	  bmap-writer is a command-line utility designed to efficiently write
> +	  disk images to storage devices using block mapping (BMAP). It serves
> +	  as a lightweight alternative to the Yocto BMAP tool, specifically for
> +	  embedded systems. Unlike the Yocto BMAP tool, bmap-writer is C++ based,
> +	  does not require Python, and focuses solely on writing an image.

Same: lines too long.

Also, this file was not included anywhere, so it was useless. I added
the necessary include logic in package/Config.in.host.

> diff --git a/package/bmap-writer/bmap-writer.mk b/package/bmap-writer/bmap-writer.mk
> new file mode 100644
> index 0000000000..bbe4eb1078
> --- /dev/null
> +++ b/package/bmap-writer/bmap-writer.mk
> @@ -0,0 +1,13 @@
> +################################################################################
> +#
> +# bmap-writer
> +#
> +################################################################################
> +
> +BMAP_WRITER_VERSION = 0.0.3
> +BMAP_WRITER_SITE = $(call github,embetrix,bmap-writer,$(BMAP_WRITER_VERSION))
> +BMAP_WRITER_LICENSE = GPL-3.0
> +BMAP_WRITER_LICENSE_FILES = LICENSE
> +BMAP_WRITER_DEPENDENCIES = host-pkgconf libarchive libxml2

Missing

HOST_BMAP_WRITER_DEPENDENCIES

> +
> +$(eval $(cmake-package))

Missing

$(eval $(host-cmake-package))

(which means you really didn't test the host version of the package).

I've applied after fixing those different issues. 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