Re: [Buildroot] [PATCH v7 1/2] package/libtalloc: new package

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

Hello Matt,

On Mon, 12 Apr 2021 11:40:10 -0500
Matt Weber <matthew.weber@rockwellcollins.com> wrote:

> From: David GOUARIN <dgouarin@gmail.com>
> 
> talloc is a hierarchical, reference counted memory pool system with destructors.
> It is the core memory allocator used in Samba.
> 
> Signed-off-by: David GOUARIN <dgouarin@gmail.com>
> Signed-off-by: Kalpesh Panchal <kalpesh.panchal@rockwellcollins.com>
> Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>

I have finally applied this patch, with those changes. See below.

> diff --git a/package/libtalloc/Config.in b/package/libtalloc/Config.in
> new file mode 100644
> index 0000000000..bb671128fb
> --- /dev/null
> +++ b/package/libtalloc/Config.in
> @@ -0,0 +1,10 @@
> +config BR2_PACKAGE_LIBTALLOC
> +	bool "libtalloc"
> +	depends on BR2_USE_MMU
> +	depends on !BR2_STATIC_LIBS

Forgot Config.in comment. In fact, I forgot it myself as well, and
added it as a follow-up commit.

> diff --git a/package/libtalloc/libtalloc.mk b/package/libtalloc/libtalloc.mk
> new file mode 100644
> index 0000000000..598a8d8114
> --- /dev/null
> +++ b/package/libtalloc/libtalloc.mk
> @@ -0,0 +1,54 @@
> +################################################################################
> +#
> +# libtalloc
> +#
> +################################################################################
> +
> +LIBTALLOC_VERSION = 2.3.1
> +LIBTALLOC_SOURCE = talloc-$(LIBTALLOC_VERSION).tar.gz
> +LIBTALLOC_SITE = https://www.samba.org/ftp/talloc
> +LIBTALLOC_LICENSE = GPL-3.0+

According to OpenEmbedded, the license really is LGPL-3.0+, GPL-3.0+.
There are some parts under GPL, some under LGPL, but it's clear what is
under which license.

Also, what bothers me a bit is how can freeradius be under the GPL-2.0
is it links against libtalloc that has bits under GPL-3.0+ ? Shouldn't
freeradius then automatically be under GPL-3.0+ as well ?

> +LIBTALLOC_LICENSE_FILES = talloc.h pytalloc.h
> +LIBTALLOC_INSTALL_STAGING = YES
> +
> +LIBTALLOC_CONF_OPTS += --cross-compile \
> +		--cross-answers=$(@D)/cache.txt \
> +		--hostcc=gcc \
> +		--with-libiconv=$(STAGING_DIR)/usr # (see below)
> +
> +# --with-libiconv=
> +# waf will search for libiconv by default in /usr/local. Because of a bug in some waf
> +# python script, /usr/local is then used in many subsequent and unrelated checks, which
> +# ultimately causes a failure when BR2_COMPILER_PARANOID_UNSAFE_PATH is set.
> +# However no need to set libiconv as a dependency of libtalloc since it's optional.

I've moved the comment above the CONF_OPTS definition, we usually have
comments before the code, not after.

> +
> +ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
> +LIBTALLOC_CFLAGS += `$(PKG_CONFIG_HOST_BINARY) --cflags libtirpc`
> +LIBTALLOC_LDFLAGS += `$(PKG_CONFIG_HOST_BINARY) --libs libtirpc`
> +LIBTALLOC_DEPENDENCIES += libtirpc host-pkgconf
> +endif
> +
> +ifeq ($(BR2_PACKAGE_PYTHON3),y)
> +LIBTALLOC_PYTHON = \
> +	PYTHON="$(HOST_DIR)/bin/python3" \
> +	PYTHON_CONFIG="$(STAGING_DIR)/usr/bin/python3-config"
> +LIBTALLOC_DEPENDENCIES += host-python3 python3
> +LIBTALLOC_CONF_ENV += \
> +	$(LIBTALLOC_PYTHON)

This LIBTALLOC_PYTHON variable was not really needed, you could just
add PYTHON and PYTHON_CONFIG directly in LIBTALLOC_CONF_ENV.

Applied with those changes!

Thanks,

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