[Buildroot] [PATCH v2] icecast: new package

[Buildroot] [PATCH v2] icecast: new package

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Wed, 17 Jul 2013 07:06:47 -0300, Ezequiel Garcia wrote:

> +diff -Naur icecast-2.3.3/m4/xiph_curl.m4 icecast-2.3.3-patched/m4/xiph_curl.m4
> +--- icecast-2.3.3/m4/xiph_curl.m4	2012-06-11 13:45:26.000000000 -0300
> ++++ icecast-2.3.3-patched/m4/xiph_curl.m4	2013-07-17 06:58:02.000000000 -0300
> +@@ -48,28 +48,10 @@
> + dnl
> + dnl Now check if the installed libcurl is sufficiently new.
> + dnl
> +-AC_CHECK_HEADERS([curl/curl.h],, curl_ok="no") 
> + AC_MSG_CHECKING(for libcurl)
> +-if test "$curl_ok" = "yes"
> +-then
> +-    AC_RUN_IFELSE(AC_LANG_SOURCE([
> +-#include <curl/curl.h>
> +-int main()
> +-{
> +-    return 0;
> +-}
> +-]),,[curl_ok="no"])
> +-fi
> +-if test "$curl_ok" = "yes"; then
> +-    AC_MSG_RESULT(yes)
> +-    AC_DEFINE(HAVE_CURL, 1, [Define if you have libcurl.])
> +-    ifelse([$1], , :, [$1])     
> +-else
> +-    AC_MSG_RESULT(no)
> +-    CURL_LIBS=""
> +-    CURL_CFLAGS=""
> +-    ifelse([$2], , :, [$2])
> +-fi
> ++AC_MSG_RESULT(yes)
> ++AC_DEFINE(HAVE_CURL, 1, [Define if you have libcurl.])
> ++ifelse([$1], , :, [$1])     

So here you're completely removing the curl check and assume that it is
always present?


> +ifeq ($(BR2_PACKAGE_LIBCURL),y)
> +ICECAST_DEPENDENCIES += libcurl
> +ICECAST_CONF_OPT += --with-curl-config=$(STAGING_DIR)/usr/bin/curl-config
> +else
> +ICECAST_CONF_OPT += --without-curl
> +endif

and here curl is still optional?

Regarding the xiph_curl.m4 check, you could turn it into an AC_TRY_LINK
test, like is done for Speex, Theora and so on. Or maybe even keep only
the AC_CHECK_HEADERS() as an indication of whether curl is available or
not.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2] icecast: new package

[Ezequiel Garcia]

On Wed, Jul 17, 2013 at 12:19:05PM +0200, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Wed, 17 Jul 2013 07:06:47 -0300, Ezequiel Garcia wrote:
> 
> > +diff -Naur icecast-2.3.3/m4/xiph_curl.m4 icecast-2.3.3-patched/m4/xiph_curl.m4
> > +--- icecast-2.3.3/m4/xiph_curl.m4	2012-06-11 13:45:26.000000000 -0300
> > ++++ icecast-2.3.3-patched/m4/xiph_curl.m4	2013-07-17 06:58:02.000000000 -0300
> > +@@ -48,28 +48,10 @@
> > + dnl
> > + dnl Now check if the installed libcurl is sufficiently new.
> > + dnl
> > +-AC_CHECK_HEADERS([curl/curl.h],, curl_ok="no") 
> > + AC_MSG_CHECKING(for libcurl)
> > +-if test "$curl_ok" = "yes"
> > +-then
> > +-    AC_RUN_IFELSE(AC_LANG_SOURCE([
> > +-#include <curl/curl.h>
> > +-int main()
> > +-{
> > +-    return 0;
> > +-}
> > +-]),,[curl_ok="no"])
> > +-fi
> > +-if test "$curl_ok" = "yes"; then
> > +-    AC_MSG_RESULT(yes)
> > +-    AC_DEFINE(HAVE_CURL, 1, [Define if you have libcurl.])
> > +-    ifelse([$1], , :, [$1])     
> > +-else
> > +-    AC_MSG_RESULT(no)
> > +-    CURL_LIBS=""
> > +-    CURL_CFLAGS=""
> > +-    ifelse([$2], , :, [$2])
> > +-fi
> > ++AC_MSG_RESULT(yes)
> > ++AC_DEFINE(HAVE_CURL, 1, [Define if you have libcurl.])
> > ++ifelse([$1], , :, [$1])     
> 
> So here you're completely removing the curl check and assume that it is
> always present?
> 
> 
> > +ifeq ($(BR2_PACKAGE_LIBCURL),y)
> > +ICECAST_DEPENDENCIES += libcurl
> > +ICECAST_CONF_OPT += --with-curl-config=$(STAGING_DIR)/usr/bin/curl-config
> > +else
> > +ICECAST_CONF_OPT += --without-curl
> > +endif
> 
> and here curl is still optional?
> 

Doh, silly uh?

> Regarding the xiph_curl.m4 check, you could turn it into an AC_TRY_LINK
> test, like is done for Speex, Theora and so on. Or maybe even keep only
> the AC_CHECK_HEADERS() as an indication of whether curl is available or
> not.
> 

Mmm... okay, I'll try AC_TRY_LINK. The AC_CHECK_HEADERS *need* to be removed
because they produce a very silly error:

as_fn_error $? "cannot run test program while cross compiling"

And this "as_fn_error" makes the configure exit with error. This happens
even when curl is installed and available, and is the reason of this
ugly patch.

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[Buildroot] [PATCH v2] icecast: new package

[Ezequiel Garcia]

On Wed, Jul 17, 2013 at 07:25:11AM -0300, Ezequiel Garcia wrote:
> On Wed, Jul 17, 2013 at 12:19:05PM +0200, Thomas Petazzoni wrote:
> > Dear Ezequiel Garcia,
> > 
> > On Wed, 17 Jul 2013 07:06:47 -0300, Ezequiel Garcia wrote:
> > 
> > > +diff -Naur icecast-2.3.3/m4/xiph_curl.m4 icecast-2.3.3-patched/m4/xiph_curl.m4
> > > +--- icecast-2.3.3/m4/xiph_curl.m4	2012-06-11 13:45:26.000000000 -0300
> > > ++++ icecast-2.3.3-patched/m4/xiph_curl.m4	2013-07-17 06:58:02.000000000 -0300
> > > +@@ -48,28 +48,10 @@
> > > + dnl
> > > + dnl Now check if the installed libcurl is sufficiently new.
> > > + dnl
> > > +-AC_CHECK_HEADERS([curl/curl.h],, curl_ok="no") 
> > > + AC_MSG_CHECKING(for libcurl)
> > > +-if test "$curl_ok" = "yes"
> > > +-then
> > > +-    AC_RUN_IFELSE(AC_LANG_SOURCE([
> > > +-#include <curl/curl.h>
> > > +-int main()
> > > +-{
> > > +-    return 0;
> > > +-}
> > > +-]),,[curl_ok="no"])
> > > +-fi
> > > +-if test "$curl_ok" = "yes"; then
> > > +-    AC_MSG_RESULT(yes)
> > > +-    AC_DEFINE(HAVE_CURL, 1, [Define if you have libcurl.])
> > > +-    ifelse([$1], , :, [$1])     
> > > +-else
> > > +-    AC_MSG_RESULT(no)
> > > +-    CURL_LIBS=""
> > > +-    CURL_CFLAGS=""
> > > +-    ifelse([$2], , :, [$2])
> > > +-fi
> > > ++AC_MSG_RESULT(yes)
> > > ++AC_DEFINE(HAVE_CURL, 1, [Define if you have libcurl.])
> > > ++ifelse([$1], , :, [$1])     
> > 
> > So here you're completely removing the curl check and assume that it is
> > always present?
> > 
> > 
> > > +ifeq ($(BR2_PACKAGE_LIBCURL),y)
> > > +ICECAST_DEPENDENCIES += libcurl
> > > +ICECAST_CONF_OPT += --with-curl-config=$(STAGING_DIR)/usr/bin/curl-config
> > > +else
> > > +ICECAST_CONF_OPT += --without-curl
> > > +endif
> > 
> > and here curl is still optional?
> > 
> 
> Doh, silly uh?
> 

Or maybe not so silly.

As far as I can see, this curl m4 file will be used when --with-curl is
present (or more accurately when --without-curl is absent).
When --without-curl is absent, then BR should guarantee that curl is
correctly installed, and the patch merely removes the package configure
double-check.

Or at least that's what my tests say.

Does this sound sane?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[Buildroot] [PATCH v2] icecast: new package

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Wed, 17 Jul 2013 07:25:11 -0300, Ezequiel Garcia wrote:

> Mmm... okay, I'll try AC_TRY_LINK. The AC_CHECK_HEADERS *need* to be removed
> because they produce a very silly error:
> 
> as_fn_error $? "cannot run test program while cross compiling"

Nah, that doesn't make sense. An AC_CHECK_HEADERS check should work
just fine in cross-compilation mode. The test that is showing this
error is the AC_RUN_IFELSE(), which obviously cannot work in
cross-compilation mode.

From the generated configure script included in the icecast 2.3.3
tarball (fat comments added by me).

##############################################
# This part is the AC_CHECK_HEADERS for curl.h
##############################################

curl_ok="yes"

xt_curl_CPPFLAGS="$CPPFLAGS"
xt_curl_LIBS="$LIBS"
CPPFLAGS="$CPPFLAGS $CURL_CFLAGS"
LIBS="$CURL_LIBS $LIBS"
for ac_header in curl/curl.h
do :
  ac_fn_c_check_header_mongrel "$LINENO" "curl/curl.h" "ac_cv_header_curl_curl_h" "$ac_includes_default"
if test "x$ac_cv_header_curl_curl_h" = xyes; then :
  cat >>confdefs.h <<_ACEOF
#define HAVE_CURL_CURL_H 1
_ACEOF

else
  curl_ok="no"
fi

##############################################
# This part is the AC_RUN_IFELSE
##############################################

{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libcurl" >&5
$as_echo_n "checking for libcurl... " >&6; }
if test "$curl_ok" = "yes"
then
    if test "$cross_compiling" = yes; then :
  { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
as_fn_error $? "cannot run test program while cross compiling
See \`config.log' for more details" "$LINENO" 5; }
else
  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h.  */

#include <curl/curl.h>
int main()
{
    return 0;
}

_ACEOF
if ac_fn_c_try_run "$LINENO"; then :

else
  curl_ok="no"
fi
rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
  conftest.$ac_objext conftest.beam conftest.$ac_ext
fi

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com