diff mbox series

[2/4] package/asterisk: enable for non-glibc toolchains

Message ID 20181003131358.28002-2-bernd.kuhls@t-online.de
State Changes Requested
Headers show
Series [1/4] package/asterisk: bump version to 14.7.8 | expand

Commit Message

Bernd Kuhls Oct. 3, 2018, 1:13 p.m. UTC
Quoting Yann:
http://lists.busybox.net/pipermail/buildroot/2017-September/203004.html

"As a final stroke of genius, asterisk checks for the re-entrant variant
of res_ninit(), and concludes that all such functions are available,
including res_nsearch(). Uclibc-ng has the former but not the latter, so
the build fails. Since there is no cache variable for that check, we
can't pre-feed that result to configure, and fixing it is a bigger
endeavour.  So we make asterisk depend on glibc for now, until someone
is brave enough to fix it."

This hack was copied from Optware:
https://github.com/Optware/Optware-ng/blob/master/make/asterisk13.mk#L331

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/asterisk/Config.in   | 6 ++----
 package/asterisk/asterisk.mk | 4 ++++
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Arnout Vandecappelle Oct. 8, 2018, 5:19 p.m. UTC | #1
On 3/10/18 15:13, Bernd Kuhls wrote:
> Quoting Yann:
> http://lists.busybox.net/pipermail/buildroot/2017-September/203004.html
> 
> "As a final stroke of genius, asterisk checks for the re-entrant variant
> of res_ninit(), and concludes that all such functions are available,
> including res_nsearch(). Uclibc-ng has the former but not the latter, so
> the build fails. Since there is no cache variable for that check, we
> can't pre-feed that result to configure, and fixing it is a bigger
> endeavour.  So we make asterisk depend on glibc for now, until someone
> is brave enough to fix it."
> 
> This hack was copied from Optware:
> https://github.com/Optware/Optware-ng/blob/master/make/asterisk13.mk#L331
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/asterisk/Config.in   | 6 ++----
>  package/asterisk/asterisk.mk | 4 ++++
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/package/asterisk/Config.in b/package/asterisk/Config.in
> index 17ac22bfd4..2cf2369749 100644
> --- a/package/asterisk/Config.in
> +++ b/package/asterisk/Config.in
> @@ -1,7 +1,5 @@
>  config BR2_PACKAGE_ASTERISK
>  	bool "asterisk"
> -	# Uses glibc resolver function res_nsearch()
> -	depends on BR2_TOOLCHAIN_USES_GLIBC
>  	depends on BR2_INSTALL_LIBSTDCPP
>  	select BR2_PACKAGE_JANSSON
>  	select BR2_PACKAGE_LIBCURL
> @@ -22,5 +20,5 @@ config BR2_PACKAGE_ASTERISK
>  
>  	  http://www.asterisk.org/
>  
> -comment "asterisk needs a glibc toolchain w/ C++"
> -	depends on !BR2_TOOLCHAIN_USES_GLIBC || !BR2_INSTALL_LIBSTDCPP
> +comment "asterisk needs a toolchain w/ C++"
> +	depends on !BR2_INSTALL_LIBSTDCPP
> diff --git a/package/asterisk/asterisk.mk b/package/asterisk/asterisk.mk
> index ea779cc8f6..46fe4de947 100644
> --- a/package/asterisk/asterisk.mk
> +++ b/package/asterisk/asterisk.mk
> @@ -117,6 +117,10 @@ ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),y)
>  ASTERISK_CONF_OPTS += --with-execinfo
>  else
>  ASTERISK_CONF_OPTS += --without-execinfo
> +define ASTERISK_NO_RESINIT
> +	sed -i -e '/AC_DEFINE(\[HAVE_RES_NINIT\]/d' $(@D)/configure.ac
> +endef

 Meh, I'm dead against such sed-patching, and I'm also against conditional
"patching" of a package.

 Is it htat much of a problem to extend the autoconf test with a call to
res_nsearch?

 Regards,
 Arnout

> +ASTERISK_POST_PATCH_HOOKS += ASTERISK_NO_RESINIT
>  endif
>  
>  ifeq ($(BR2_PACKAGE_LIBGSM),y)
>
Yann E. MORIN Oct. 8, 2018, 7:45 p.m. UTC | #2
Arnout, Bernd, All,

On 2018-10-08 19:19 +0200, Arnout Vandecappelle spake thusly:
> On 3/10/18 15:13, Bernd Kuhls wrote:
[--SNIP--]
> > diff --git a/package/asterisk/asterisk.mk b/package/asterisk/asterisk.mk
> > index ea779cc8f6..46fe4de947 100644
> > --- a/package/asterisk/asterisk.mk
> > +++ b/package/asterisk/asterisk.mk
> > @@ -117,6 +117,10 @@ ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),y)
> >  ASTERISK_CONF_OPTS += --with-execinfo
> >  else
> >  ASTERISK_CONF_OPTS += --without-execinfo
> > +define ASTERISK_NO_RESINIT
> > +	sed -i -e '/AC_DEFINE(\[HAVE_RES_NINIT\]/d' $(@D)/configure.ac
> > +endef
> 
>  Meh, I'm dead against such sed-patching, and I'm also against conditional
> "patching" of a package.

Agreed.

>  Is it htat much of a problem to extend the autoconf test with a call to
> res_nsearch?

It should not, no. It should be possible to change (in master), configure.ac
line 1391, from:

    [int foo = res_ninit(NULL);]

... to something like:

    [
        int foo;
        foo = res_ninit(NULL);
        foo = res_nsearch(NULL, NULL, 0, 0, NULL, 0);
    ]

... and potentially adding all such functions.

Since this is an AC_LINK_IFELSE(), we don't care what values we pas to
the the functions, as long as they have the proper types.

Regards,
Yann E. MORIN.

>  Regards,
>  Arnout
> 
> > +ASTERISK_POST_PATCH_HOOKS += ASTERISK_NO_RESINIT
> >  endif
> >  
> >  ifeq ($(BR2_PACKAGE_LIBGSM),y)
> > 
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
Thomas Petazzoni Oct. 9, 2018, 1:09 p.m. UTC | #3
Hello,

On Wed,  3 Oct 2018 15:13:56 +0200, Bernd Kuhls wrote:
> Quoting Yann:
> http://lists.busybox.net/pipermail/buildroot/2017-September/203004.html
> 
> "As a final stroke of genius, asterisk checks for the re-entrant variant
> of res_ninit(), and concludes that all such functions are available,
> including res_nsearch(). Uclibc-ng has the former but not the latter, so
> the build fails. Since there is no cache variable for that check, we
> can't pre-feed that result to configure, and fixing it is a bigger
> endeavour.  So we make asterisk depend on glibc for now, until someone
> is brave enough to fix it."
> 
> This hack was copied from Optware:
> https://github.com/Optware/Optware-ng/blob/master/make/asterisk13.mk#L331
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/asterisk/Config.in   | 6 ++----
>  package/asterisk/asterisk.mk | 4 ++++
>  2 files changed, 6 insertions(+), 4 deletions(-)

I agree with Arnout here, we want to fix this with a proper autoconf
check. I've marked this patch as Changes Requested. Could you rework it?

Thanks a lot!

Thomas
diff mbox series

Patch

diff --git a/package/asterisk/Config.in b/package/asterisk/Config.in
index 17ac22bfd4..2cf2369749 100644
--- a/package/asterisk/Config.in
+++ b/package/asterisk/Config.in
@@ -1,7 +1,5 @@ 
 config BR2_PACKAGE_ASTERISK
 	bool "asterisk"
-	# Uses glibc resolver function res_nsearch()
-	depends on BR2_TOOLCHAIN_USES_GLIBC
 	depends on BR2_INSTALL_LIBSTDCPP
 	select BR2_PACKAGE_JANSSON
 	select BR2_PACKAGE_LIBCURL
@@ -22,5 +20,5 @@  config BR2_PACKAGE_ASTERISK
 
 	  http://www.asterisk.org/
 
-comment "asterisk needs a glibc toolchain w/ C++"
-	depends on !BR2_TOOLCHAIN_USES_GLIBC || !BR2_INSTALL_LIBSTDCPP
+comment "asterisk needs a toolchain w/ C++"
+	depends on !BR2_INSTALL_LIBSTDCPP
diff --git a/package/asterisk/asterisk.mk b/package/asterisk/asterisk.mk
index ea779cc8f6..46fe4de947 100644
--- a/package/asterisk/asterisk.mk
+++ b/package/asterisk/asterisk.mk
@@ -117,6 +117,10 @@  ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),y)
 ASTERISK_CONF_OPTS += --with-execinfo
 else
 ASTERISK_CONF_OPTS += --without-execinfo
+define ASTERISK_NO_RESINIT
+	sed -i -e '/AC_DEFINE(\[HAVE_RES_NINIT\]/d' $(@D)/configure.ac
+endef
+ASTERISK_POST_PATCH_HOOKS += ASTERISK_NO_RESINIT
 endif
 
 ifeq ($(BR2_PACKAGE_LIBGSM),y)