Re: [Buildroot] [PATCH 1/2] package/exim: fix build with libexecinfo

[Luca Ceresoli via buildroot <buildroot@buildroot.org>]

Hi Yann,

On Fri, 29 Dec 2023 22:56:26 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Bernd, Luca, All,
> 
> On 2023-12-28 09:51 +0100, Luca Ceresoli via buildroot spake thusly:
> > On Wed, 27 Dec 2023 19:17:55 +0100
> > Bernd Kuhls <bernd@kuhls.net> wrote:  
> > > Upstream added optional support for execinfo  
> [--SNIP--]
> > > @@ -126,6 +127,15 @@ ifeq ($(BR2_STATIC_LIBS),y)
> > >  EXIM_STATIC_FLAGS = LFLAGS="-pthread --static"
> > >  endif
> > >  
> > > +ifeq ($(BR2_PACKAGE_LIBEXECINFO),y)
> > > +EXIM_DEPENDENCIES += libexecinfo
> > > +define EXIM_EXTRALIBS
> > > +$(call exim-config-add,EXTRALIBS,-lexecinfo)
> > > +endef
> > > +else
> > > +EXIM_C_FLAGS = -DNO_EXECINFO
> > > +endif  
> > 
> > I think this logic is not entirely correct. As I read it:
> > 
> >  * on non-glibc systems it automatically enables the new exim
> >    stack dump feature if libexecinfo is enabled
> >  * on glibc systems, which always have the backtrace() and related
> >    functions without additional libraries, we never enable this feature
> > 
> > For consistency, the else branch should just be removed to avoid
> > setting NO_EXECINFO, thus enabling the stack dump feature whenever it
> > is possible.  
> 
> If I understand correctly, your proposal would not work either: for a
> non-glibc config that does not have libexecinfo eabled, we do want to
> define NO_EXECINFO.

Exactly!

> Instead, the logic should be somthong like:
> 
>     if glibc:
>         do nothing, execinfo is available
>     elif libexecinfo enabled:
>         add dependency and extra-lib
>     else:
>         define NO_EXECINFO
> 
> Since libexecinfo embedds the fact that it is also non-glibc, we can
> rewrite the condition as:
> 
>     if libexecinfo:
>         add dependency and extra-lib
>     elif non-glibc;
>         define NO_EXECINFO
> 
> So, the 'else' clause only needs to be changed into:
>     else ifneq ($(BR2_TOOLCHAIN_USES_GLIBC),y)

Correct, thanks for suggesting this fix.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot