Re: [Buildroot] [PATCH v3] package/mhz: add new package

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Robert Marko <robimarko@gmail.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH v3] package/mhz: add new package
Date: Tue, 1 Aug 2023 22:49:23 +0200	[thread overview]
Message-ID: <20230801224923.35b6ac96@windsurf> (raw)
In-Reply-To: <20230731170504.2769638-1-robimarko@gmail.com>

Hello Robert,

On Mon, 31 Jul 2023 19:05:04 +0200
Robert Marko <robimarko@gmail.com> wrote:

> mhz is a tool to mathematically calculate the real running CPU frequency
> and as such has proved as invaluable tool for developing CPUFreq and
> similar features in the kernel.
> 
> Its source finally got a license recently so it can be packaged.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>

I've applied your change, after making a number of tweaks. Many of them
had already been pointed out to you in the review of the previous
versions, but you did not take them into account.

>  package/Config.in     |  1 +
>  package/mhz/Config.in |  6 ++++++
>  package/mhz/mhz.hash  |  3 +++
>  package/mhz/mhz.mk    | 21 +++++++++++++++++++++
>  4 files changed, 31 insertions(+)

Entry in the DEVELOPERS file was missing.

> diff --git a/package/mhz/mhz.hash b/package/mhz/mhz.hash
> new file mode 100644
> index 0000000000..eb66c10d63
> --- /dev/null
> +++ b/package/mhz/mhz.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  1e07bb0d455d63fd33fb87a24f65a803c7c2d920fde4e5b5378b8c1b07de3b47  mhz-11aac2399780a1f7ea9f007b14af0464797d5cf1-br1.tar.gz
> +sha256  9449d373cad921620bb8f1e4f7b39f957e263163ded159c1efc8de27f8997099  LICENSE
> \ No newline at end of file

Missing newline here. This is reported by "make check-package", which
verifies basic coding style details.

> diff --git a/package/mhz/mhz.mk b/package/mhz/mhz.mk
> new file mode 100644
> index 0000000000..36b3754968
> --- /dev/null
> +++ b/package/mhz/mhz.mk
> @@ -0,0 +1,21 @@
> +################################################################################
> +#
> +# mhz
> +#
> +################################################################################
> +
> +MHZ_VERSION = 11aac2399780a1f7ea9f007b14af0464797d5cf1
> +MHZ_SITE = https://github.com/wtarreau/mhz.git
> +MHZ_SITE_METHOD = git

Switched to using the "github" helper to do an HTTP download instead.

> +MHZ_LICENSE = MIT/X11

The license is just MIT. X11 is a slightly difference license, and
MIT/X11 is not a valid SPDX identifier.

> +MHZ_LICENSE_FILES = LICENSE
> +
> +define MHZ_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D)
> +endef
> +
> +define MHZ_INSTALL_TARGET_CMDS
> +	$(INSTALL) $(@D)/mhz $(TARGET_DIR)/usr/bin/mhz

Missing -D -m 0755 options.

As said: I fixed those small details when applying.

Thanks for your contribution!

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