Re: [Buildroot] [PATCH 4/7] package/alfred: new package

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

Hello José,

On Fri, 10 Dec 2021 16:12:06 +0200
José Pekkarinen <jose.pekkarinen@unikie.com> wrote:

> This  patch will provide the initial
> package for alfred project.
> 
> Signed-off-by: José Pekkarinen <jose.pekkarinen@unikie.com>

I have applied your patch, but after doing a number of changes. See below.

> diff --git a/package/alfred/Config.in b/package/alfred/Config.in
> new file mode 100644
> index 0000000000..5f9c6f2a6e
> --- /dev/null
> +++ b/package/alfred/Config.in
> @@ -0,0 +1,20 @@
> +config BR2_PACKAGE_ALFRED
> +	bool "alfred"
> +	depends on !BR2_STATIC_LIBS # dlopen()
> +	select BR2_PACKAGE_LIBNL

You have forgotten to account for the threads dependency of libnl,
which must be replicated here;

> +	select BR2_PACKAGE_LIBCAP
> +	select BR2_PACKAGE_GPSD

Neither libcap and gpsd are mandatory dependencies (see the README file
from the alfred code base for details). So I dropped these entirely. If
you want to have support for libcap and gpsd, you need to handle them
in the .mk file like this:

ifeq ($(BR2_PACKAGE_LIBCAP),y)
... do something ...
else
... do something else ...
endif

> +comment "alfred needs a toolchain with dynamic library support"
> +	depends on BR2_STATIC_LIBS

Modified to account for the threads dependency.

> diff --git a/package/alfred/alfred.hash b/package/alfred/alfred.hash
> new file mode 100644
> index 0000000000..a2dd0116d7
> --- /dev/null
> +++ b/package/alfred/alfred.hash
> @@ -0,0 +1,4 @@
> +# From https://downloads.open-mesh.org/batman/stable/sources/alfred

As pointed out by Giulio, this didn't make sense, as it doesn't point
to a file that contains the sha256, and actually no file there contains
the sha256. So instead I've added the sha1 from the file provided by
upstream, and put the sha256 hash under the "Locally computed" comment.

> +sha256  4c79b6c45de4bcc8cbfe64cba9a0f8b4ef304ca84c194622f2bfa41e01e2cb95  alfred-2021.4.tar.gz
> +# Locally computed
> +sha256  cecbf53d1148e13256ac29f8b900655b7fc8dc12d59939a95bc2323ea1747025  LICENSES/preferred/GPL-2.0
> diff --git a/package/alfred/alfred.mk b/package/alfred/alfred.mk
> new file mode 100644
> index 0000000000..4e38ddad92
> --- /dev/null
> +++ b/package/alfred/alfred.mk
> @@ -0,0 +1,25 @@
> +################################################################################
> +#
> +# alfred
> +#
> +################################################################################
> +
> +ALFRED_VERSION = 2021.4
> +ALFRED_SITE = https://downloads.open-mesh.org/batman/stable/sources/alfred
> +ALFRED_LICENSE = GPL-2

used GPL-2.0 here, which is the correct SPDX code for the GPLv2.

> +ALFRED_LICENSE_FILES = LICENSES/preferred/GPL-2.0
> +ALFRED_DEPENDENCIES = libnl libcap gpsd

Dropped libcap and gpsd from here.

> +
> +define ALFRED_BUILD_CMDS
> +	(cd $(@D); \
> +	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/$(d) CC="$(TARGET_CC)")

Several things were wrong:

 - cd $(@D) was not needed, since you're doing -C $(@D)

 - the $(d) was wrong, as $(d) is an empty variable

 - doing that within a subshell is not necessary

 - passing $(TARGET_CONFIGURE_OPTS) is preferred over passing just CC.

Applied with those changes, 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