Re: [Buildroot] [PATCH 1/1] package/libtevent: add libtevent package

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

Hello Ayrton,

Thanks a lot for your contribution! It looks pretty good, but of course
I do have a small number of comments.

First a very minor one: the commit title should be:

	package/libtevent: new package

On Mon, 21 Oct 2024 11:46:27 +0000
Ayrton Leyssens <aleyssens@idtech.be> wrote:

> This patch adds support for libtevent as a standolane library.
> Libtevent is part of samba, but one might need it for other packages without the need for the whole samba eg. Certmonger.

The commit log should be wrapped at ~80 columns.

> Signed-off-by: Ayrton aleyssens@idtech.be<mailto:aleyssens@idtech.be>

Could you fixup your Signed-off-by line to be:

	Signed-off-by: Ayrton Leyssens <aleyssens@idtech.be>

 ?

> ---
> package/Config.in                             |   1 +
> ...mba-add-disable-stack-protector-opti.patch | 116 ++++++++++++++++++
> package/libtevent/Config.in                   |  16 +++
> package/libtevent/libtevent.hash              |   2 +
> package/libtevent/libtevent.mk                |  58 +++++++++
> 5 files changed, 193 insertions(+)

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

> diff --git a/package/libtevent/0001-buildtools-wafsamba-add-disable-stack-protector-opti.patch b/package/libtevent/0001-buildtools-wafsamba-add-disable-stack-protector-opti.patch
> new file mode 100644
> index 0000000000..839479a3fa
> --- /dev/null
> +++ b/package/libtevent/0001-buildtools-wafsamba-add-disable-stack-protector-opti.patch
> @@ -0,0 +1,116 @@
> +From 5885ed8e6db7648e6842d9811aace7edc4e8aba7 Mon Sep 17 00:00:00 2001
> +From: Fabrice Fontaine fontaine.fabrice@gmail.com<mailto:fontaine.fabrice@gmail.com>
> +Date: Wed, 20 Apr 2022 11:16:52 +0200
> +Subject: [PATCH] buildtools/wafsamba: add --disable-stack-protector option
> +
> +Allow the user to disable stack-protector through
> +--disable-stack-protector to avoid the following build failure with
> +libtalloc on embedded toolchains which don't support stack-protector:
> +
> +/home/autobuild/autobuild/instance-5/output-1/host/lib/gcc/i686-buildroot-linux-musl/9.4.0/../../../../i686-buildroot-linux-musl/bin/ld: talloc.c.5.o: in function `_vasprintf_tc':
> +talloc.c:(.text+0x427d): undefined reference to `__stack_chk_fail_local'
> +
> +This build failure is raised since
> +https://gitlab.com/ffontaine/samba/-/commit/38e97f8b52e85bdfcf2d74a4fb3c848fa46ba371
> +because stack-protector is enabled on libtalloc despite the fact that
> +libssp is not available:
> +
> +Checking if compiler accepts -fstack-protector-strong                                           : yes
> +
> +Fixes:
> + - http://autobuild.buildroot.org/results/e221bde25c7622db99761d0adcd56663296beb15
> +
> +Signed-off-by: Fabrice Fontaine fontaine.fabrice@gmail.com<mailto:fontaine.fabrice@gmail.com>
> +[Upstream status:
> +]

Convert this to the new Upstream: tag, as such:

Upstream: https://gitlab.com/samba-team/samba/-/merge_requests/2493

also, please add your Signed-off-by: line in addition to the one from
Fabrice Fontaine.


> diff --git a/package/libtevent/libtevent.hash b/package/libtevent/libtevent.hash
> new file mode 100644
> index 0000000000..990998e6d1
> --- /dev/null
> +++ b/package/libtevent/libtevent.hash
> @@ -0,0 +1,2 @@
> +# Locally calculated
> +sha256  362971e0f32dc1905f6fe4736319c4b8348c22dc85aa6c3f690a28efe548029e  tevent-0.16.1.tar.gz

Please add the hash of the license file (tevent.h).

> diff --git a/package/libtevent/libtevent.mk b/package/libtevent/libtevent.mk
> new file mode 100644
> index 0000000000..636a6abbe2
> --- /dev/null
> +++ b/package/libtevent/libtevent.mk
> @@ -0,0 +1,58 @@
> +################################################################################
> +#
> +# libtevent
> +#
> +################################################################################

This whole file looks exactly identical to libtalloc. Is everything
exactly identical, and needed for libtevent as well, or did you just
copy/paste? :-)

Final comment: since this libtevent package is needed for the
certmonger package, please send both patches together in a patch series
(libtevent should be PATCH 1/2, and certmonger should be PATCH 2/2).

Thanks a lot!

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