Skip to contentSkip to conversation

icecast: new package

PATCH Superseded Thomas Petazzoni 2 participants 4 messages Jul 17, 2013 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Thomas Petazzoni

author
#1 lore raw

PATCH v2 1/1 icecast: new package

Dear Ezequiel Garcia,

Quoted 29 lines · Ezequiel Garcia

+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
Quoted 3 lines

++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?

Review

Quoted 6 lines
(quoted diff) +6 -0
+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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#2 lore raw

PATCH v2 1/1 icecast: new package

Quoted 41 lines · Thomas Petazzoni

Dear Ezequiel Garcia,

Quoted 29 lines · Ezequiel Garcia

+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
Quoted 3 lines

++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?

Quoted 9 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+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?

Quoted 5 lines

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.

Signature · 3 lines
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
#3 lore raw

PATCH v2 1/1 icecast: new package

Quoted 52 lines · Ezequiel Garcia
Quoted 41 lines · Thomas Petazzoni

Dear Ezequiel Garcia,

Quoted 29 lines · Ezequiel Garcia

+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
Quoted 3 lines

++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?

Quoted 9 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+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?

Quoted 2 lines

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?

Signature · 3 lines
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
author
#4 lore raw

PATCH v2 1/1 icecast: new package

In reply to Ezequiel Garcia

Dear Ezequiel Garcia,

Quoted 4 lines · Ezequiel Garcia

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

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