Re: [Buildroot] [PATCH 1/2] package/byacc: add new host package

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

Hello Thomas,

Thanks for your contribution. The commit title should be just
"package/byacc: new package". See below for more comments.

On Fri, 15 Apr 2022 18:52:39 +0200
Thomas Perrot <thomas.perrot@bootlin.com> wrote:

> This package provides a Berkeley LALR Yacc parser generator that has been use
> by i.MX Code Signing tool.
> 
> Signed-off-by: Thomas Perrot <thomas.perrot@bootlin.com>
> ---
>  package/Config.in.host       |  1 +
>  package/byacc/Config.in.host | 10 ++++++++++
>  package/byacc/byacc.hash     |  3 +++
>  package/byacc/byacc.mk       | 16 ++++++++++++++++
>  4 files changed, 30 insertions(+)

You forgot to add an entry in the DEVELOPERS file for this new package.

>  create mode 100644 package/byacc/Config.in.host
>  create mode 100644 package/byacc/byacc.hash
>  create mode 100644 package/byacc/byacc.mk
> 
> diff --git a/package/Config.in.host b/package/Config.in.host
> index cd1c34b5a3d9..69c985842d1f 100644
> --- a/package/Config.in.host
> +++ b/package/Config.in.host
> @@ -7,6 +7,7 @@ menu "Host utilities"
>  	source "package/babeltrace2/Config.in.host"
>  	source "package/bmap-tools/Config.in.host"
>  	source "package/btrfs-progs/Config.in.host"
> +	source "package/byacc/Config.in.host"

This is not needed. This tool is merely a build dependency of imx-cst,
so we don't need a visible entry in menuconfig. Look at package/bison/:
it doesn't have a visible menuconfig entry.


> diff --git a/package/byacc/byacc.hash b/package/byacc/byacc.hash
> new file mode 100644
> index 000000000000..d2ad441df39f
> --- /dev/null
> +++ b/package/byacc/byacc.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  42c1805cc529314e6a76326fe1b33e80c70862a44b01474da362e2f7db2d749c  byacc-20220128.tgz
> +sha256  654a231fe9cd4d36e580214962fa777a1df0771d6889cfc74d19a3d414f14711  LICENSE
> diff --git a/package/byacc/byacc.mk b/package/byacc/byacc.mk
> new file mode 100644
> index 000000000000..e14c4d9c7914
> --- /dev/null
> +++ b/package/byacc/byacc.mk
> @@ -0,0 +1,16 @@
> +################################################################################
> +#
> +# byacc
> +#
> +################################################################################
> +
> +BYACC_SITE = https://invisible-mirror.net/archives/byacc
> +BYACC_SITE_METHOD = https

Specifying the site method here is not needed.

> +BYACC_VERSION = 20220128
> +BYACC_SOURCE = byacc-$(BYACC_VERSION).tgz
> +BYACC_LICENSE = PD

It seems like we normally use "Public Domain" and not "PD".

> +BYACC_LICENSE_FILES = LICENSE
> +
> +HOST_BYACC_CONF_OPTS += --program-transform-name='s,^,b,'

+= not needed, a = is sufficient here. However, a comment above this
line would be appreciated, as it's not obvious why you're doing this.

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