Re: [Buildroot] [PATCH V2] package/firmware-utils: add package

[Thomas Petazzoni <thomas.petazzoni@bootlin.com>]

Hello Rafał,

On Mon,  9 Aug 2021 16:41:44 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This package provides a set of tools originally developed for the
> OpenWrt project. They allow working with various firmware formats that
> are required for flashing new images on embedded devices.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

It mostly looks good, and I almost applied, but I do have a few
questions/comments.

> diff --git a/package/Config.in b/package/Config.in
> index 046c04e994..6657811a8c 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -212,6 +212,7 @@ menu "Filesystem and flash utilities"
>  	source "package/exfat-utils/Config.in"
>  	source "package/exfatprogs/Config.in"
>  	source "package/f2fs-tools/Config.in"
> +	source "package/firmware-utils/Config.in"

Could you explain why a target variant of this package is needed? What
are the use cases for having those tools installed on the target?

> diff --git a/package/firmware-utils/Config.in b/package/firmware-utils/Config.in
> new file mode 100644
> index 0000000000..da765ec1b4
> --- /dev/null
> +++ b/package/firmware-utils/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_FIRMWARE_UTILS
> +	bool "firmware-utils"

You need:

	select BR2_PACKAGE_OPENSSL
	select BR2_PACKAGE_ZLIB

also, could you:

 (1) Verify that it builds with the basic OpenSSL options, i.e after
     disabling all openssl sub-options ?

 (2) Verify that it builds with libressl ?

> diff --git a/package/firmware-utils/Config.in.host b/package/firmware-utils/Config.in.host
> new file mode 100644
> index 0000000000..cdc7b6e775
> --- /dev/null
> +++ b/package/firmware-utils/Config.in.host
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_HOST_FIRMWARE_UTILS
> +	bool "host firmware-utils"

Note: do *not* add any "select" here.

> diff --git a/package/firmware-utils/firmware-utils.mk b/package/firmware-utils/firmware-utils.mk
> new file mode 100644
> index 0000000000..5a35d1b898
> --- /dev/null
> +++ b/package/firmware-utils/firmware-utils.mk
> @@ -0,0 +1,35 @@
> +################################################################################
> +#
> +# firmware-utils
> +#
> +################################################################################
> +
> +FIRMWARE_UTILS_VERSION = e3853149fb18dfa55f10c10c9aaf99c329747365
> +FIRMWARE_UTILS_SITE = git://git.openwrt.org/project/firmware-utils.git
> +FIRMWARE_UTILS_LICENSE = \
> +	BSD-2-Clause (tplink-safeloader), \
> +	BSD-3-Clause (seama), \
> +	GPL-2.0 (add_header, bcmalgo, buffalo-enc, buffalo-lib, buffalo-tag, \
> +		 buffalo-tftp, dgfirmware, dns313-header, edimax_fw_header, \
> +		 fix-u-media-header, hcsmakeimage, mkbrncmdline, mkbrnimg, \
> +		 mkbuffaloimg, mkcameofw, mkcasfw, mkdapimg, mkdapimg2, \
> +		 mkdhpimg, mkdniimg, mkhilinkfw, mkmerakifw-old, mkmerakifw, \
> +		 mkplanexfw, mkporayfw, mkrasimage, mkrtn56uimg, mksenaofw, \
> +		 mksercommfw, mktitanimg, mktplinkfw-lib, mktplinkfw, \
> +		 mktplinkfw2, mkwrggimg, mkwrgimg, mkzcfw, mkzynfw, \
> +		 osbridge-crc, pc1crypt, srec2bin, trx2edips, uimage_padhdr, \
> +		 wrt400n, zyimage, zytrx), \
> +	GPL-2.0+ (addpattern, asustrx, bcm4908asus, bcm4908kernel, dgn3500sum, \
> +		  encode_crc, jcgimage, lzma2eva, makeamitbin, mkchkimg, \
> +		  mkcsysimg, mkdlinkfw-lib, mkdlinkfw, mkedimaximg, mkfwimage, \
> +		  mkfwimage2, mkheader_gemtek, mkmylofw, motorola-bin, \
> +		  nec-enc, oseama, otrx, ptgen, sign_dlink_ru, spw303v, trx, \
> +		  xorimage, zyxbcm), \
> +	GPL-2.0 OR GPL-3.0 (nand_ecc), \
> +	GPL-2.0+ OR MIT (lxlfw), \
> +	UNKNOWN (imagetag, trx2usr)

> +FIRMWARE_UTILS_DEPENDENCIES = libopenssl zlib

Use "openssl" instead of "libopenssl".

> +HOST_FIRMWARE_UTILS_DEPENDENCIES = host-libopenssl host-zlib

Use "host-openssl" instead of "host-libopenssl".

Thanks a lot!

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