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

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

[Bernd Kuhls]

Upstream added optional support for execinfo

https://git.exim.org/exim.git/commitdiff/204a7a2c2e8601558905dc34c576a627045a9f21
https://git.exim.org/exim.git/commitdiff/48ea675fee2d5fee8d33c525e28727b69114cfce

in version 4.97 which was added to buildroot with commit
faec3ca30e358575f70a036879029f63f7da9b29

Fixes:
http://autobuild.buildroot.net/results/282/282882371e1d8c224c457bf65016f8abd11f8c45/

Signed-off-by: Bernd Kuhls <bernd@kuhls.net>
---
 package/exim/exim.mk | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/package/exim/exim.mk b/package/exim/exim.mk
index 23d888e6f2..30b0d78c94 100644
--- a/package/exim/exim.mk
+++ b/package/exim/exim.mk
@@ -104,6 +104,7 @@ define EXIM_CONFIGURE_TOOLCHAIN
 	$(call exim-config-add,HOSTCC,$(HOSTCC))
 	$(call exim-config-add,HOSTCFLAGS,$(HOSTCFLAGS))
 	$(EXIM_FIX_IP_OPTIONS_FOR_MUSL)
+	$(EXIM_EXTRALIBS)
 endef
 
 ifneq ($(call qstrip,$(BR2_PACKAGE_EXIM_CUSTOM_CONFIG_FILE)),)
@@ -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
+
 # We need the host version of macro_predef during the build, before
 # building it we need to prepare the makefile.
 define EXIM_BUILD_CMDS
@@ -136,7 +146,7 @@ define EXIM_BUILD_CMDS
 		CFLAGS="-std=c99 $(HOST_CFLAGS)" \
 		LFLAGS="-fPIC $(HOST_LDFLAGS)"
 	$(TARGET_MAKE_ENV) build=br $(MAKE) -C $(@D) $(EXIM_STATIC_FLAGS) \
-		CFLAGS="-std=c99 $(TARGET_CFLAGS)"
+		CFLAGS="-std=c99 $(TARGET_CFLAGS) $(EXIM_C_FLAGS)"
 endef
 
 # Need to replicate the LFLAGS in install, as exim still wants to build
-- 
2.39.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/2] package/exim: do not build perl-based utilities

[Bernd Kuhls]

Upstream added various utilities, for example exim_id_update
with commit https://github.com/Exim/exim/commit/46a36afae41f63de654269c8a0b7cf5852a85a14
which is a "Utility for one-time upgrage/downgrade between exim message-
id formats, around the 4.97 transition" which would add a dependency to
host-perl/perl so we build only the exim binary.

Signed-off-by: Bernd Kuhls <bernd@kuhls.net>
---
 package/exim/exim.mk | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/package/exim/exim.mk b/package/exim/exim.mk
index 30b0d78c94..1b3295697e 100644
--- a/package/exim/exim.mk
+++ b/package/exim/exim.mk
@@ -146,16 +146,15 @@ define EXIM_BUILD_CMDS
 		CFLAGS="-std=c99 $(HOST_CFLAGS)" \
 		LFLAGS="-fPIC $(HOST_LDFLAGS)"
 	$(TARGET_MAKE_ENV) build=br $(MAKE) -C $(@D) $(EXIM_STATIC_FLAGS) \
-		CFLAGS="-std=c99 $(TARGET_CFLAGS) $(EXIM_C_FLAGS)"
+		CFLAGS="-std=c99 $(TARGET_CFLAGS) $(EXIM_C_FLAGS)" exim
 endef
 
 # Need to replicate the LFLAGS in install, as exim still wants to build
 # something when installing...
 define EXIM_INSTALL_TARGET_CMDS
-	DESTDIR=$(TARGET_DIR) INSTALL_ARG="-no_chown -no_symlink" build=br \
-	  $(MAKE) -C $(@D) $(EXIM_STATIC_FLAGS) \
-		CFLAGS="-std=c99 $(TARGET_CFLAGS)" \
-		install
+	cd $(@D)/build-br; \
+	DESTDIR=$(TARGET_DIR) build=br \
+	../scripts/exim_install -no_chown -no_symlink exim
 	chmod u+s $(TARGET_DIR)/usr/sbin/exim
 endef
 
-- 
2.39.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Luca Ceresoli via buildroot]

Hello Bernd,

On Wed, 27 Dec 2023 19:17:55 +0100
Bernd Kuhls <bernd@kuhls.net> wrote:

> Upstream added optional support for execinfo
> 
> https://git.exim.org/exim.git/commitdiff/204a7a2c2e8601558905dc34c576a627045a9f21
> https://git.exim.org/exim.git/commitdiff/48ea675fee2d5fee8d33c525e28727b69114cfce
> 
> in version 4.97 which was added to buildroot with commit
> faec3ca30e358575f70a036879029f63f7da9b29
> 
> Fixes:
> http://autobuild.buildroot.net/results/282/282882371e1d8c224c457bf65016f8abd11f8c45/

Thanks for taking care of the build failures!

> @@ -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.

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

Re: [Buildroot] [PATCH 2/2] package/exim: do not build perl-based utilities

[Luca Ceresoli via buildroot]

Hello Bernd,

[+Cc Yann]

On Wed, 27 Dec 2023 19:17:56 +0100
Bernd Kuhls <bernd@kuhls.net> wrote:

> Upstream added various utilities, for example exim_id_update
> with commit https://github.com/Exim/exim/commit/46a36afae41f63de654269c8a0b7cf5852a85a14
> which is a "Utility for one-time upgrage/downgrade between exim message-
> id formats, around the 4.97 transition" which would add a dependency to
> host-perl/perl so we build only the exim binary.
> 
> Signed-off-by: Bernd Kuhls <bernd@kuhls.net>
> ---
>  package/exim/exim.mk | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/package/exim/exim.mk b/package/exim/exim.mk
> index 30b0d78c94..1b3295697e 100644
> --- a/package/exim/exim.mk
> +++ b/package/exim/exim.mk
> @@ -146,16 +146,15 @@ define EXIM_BUILD_CMDS
>  		CFLAGS="-std=c99 $(HOST_CFLAGS)" \
>  		LFLAGS="-fPIC $(HOST_LDFLAGS)"
>  	$(TARGET_MAKE_ENV) build=br $(MAKE) -C $(@D) $(EXIM_STATIC_FLAGS) \
> -		CFLAGS="-std=c99 $(TARGET_CFLAGS) $(EXIM_C_FLAGS)"
> +		CFLAGS="-std=c99 $(TARGET_CFLAGS) $(EXIM_C_FLAGS)" exim
>  endef
>  
>  # Need to replicate the LFLAGS in install, as exim still wants to build
>  # something when installing...
>  define EXIM_INSTALL_TARGET_CMDS
> -	DESTDIR=$(TARGET_DIR) INSTALL_ARG="-no_chown -no_symlink" build=br \
> -	  $(MAKE) -C $(@D) $(EXIM_STATIC_FLAGS) \
> -		CFLAGS="-std=c99 $(TARGET_CFLAGS)" \
> -		install
> +	cd $(@D)/build-br; \
> +	DESTDIR=$(TARGET_DIR) build=br \
> +	../scripts/exim_install -no_chown -no_symlink exim
>  	chmod u+s $(TARGET_DIR)/usr/sbin/exim
>  endef

I like this complete rewrite of the install commands, which get rid of
the utterly complex installation step currently in place. However I'd
have mentioned this in the commit log, including a mention of the
checks done to ensure this is still installing whatever is
actually needed.

Also, the comment about the LFLAGS, added by Yann in [0], should be
removed as it it does not apply anymore.

[0]
https://git.busybox.net/buildroot/commit/?id=bf22cea53fa2af48bde3010673d3babee1ae6873

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

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

[Yann E. MORIN]

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.

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)

Unless I missed something less obvious...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Luca Ceresoli via buildroot]

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