signalwire / freeswitch

FreeSWITCH is a Software Defined Telecom Stack enabling the digital transformation from proprietary telecom switches to a versatile software implementation that runs on any commodity hardware. From a Raspberry PI to a multi-core server, FreeSWITCH can unlock the telecommunications potential of any device.
https://freeswitch.com/#getting-started
Other
3.5k stars 1.4k forks source link

build failure in buildroot #681

Closed hthiery closed 4 years ago

hthiery commented 4 years ago

When building freeswtich in buildroot I see the following build failure.

Making all in s2check
make[8]: Entering directory '/home/buildroot/autobuild/instance-0/output-1/build/freeswitch-1.10.3/libs/sofia-sip/s2check'
     COMPILE s2tcase.o
     COMPILE s2base.o
     COMPILE s2sip.o
     COMPILE s2_localinfo.o
s2tcase.c: In function 's2_tcase_add_test':
s2tcase.c:60:27: warning: passing argument 2 of '_tcase_add_test' from incompatible pointer type [-Wincompatible-pointer-types]
       _tcase_add_test(tc, tf, name, signo, 0, start, end);
                           ^~
In file included from s2tcase.c:35:
/home/buildroot/autobuild/instance-0/output-1/host/arm-buildroot-linux-musleabihf/sysroot/usr/include/check.h:331:69: note: expected 'const TTest *' {aka 'const struct TTest *'} but argument is of type 'TFun' {aka 'void (*)(int)'}
 CK_DLL_EXP void CK_EXPORT _tcase_add_test(TCase * tc, const TTest * ttest,
                                                       ~~~~~~~~~~~~~~^~~~~
s2tcase.c:60:31: warning: passing argument 3 of '_tcase_add_test' makes integer from pointer without a cast [-Wint-conversion]
       _tcase_add_test(tc, tf, name, signo, 0, start, end);
                               ^~~~
In file included from s2tcase.c:35:
/home/buildroot/autobuild/instance-0/output-1/host/arm-buildroot-linux-musleabihf/sysroot/usr/include/check.h:332:47: note: expected 'int' but argument is of type 'const char *'
                                           int _signal, int allowed_exit_value,
                                           ~~~~^~~~~~~
s2tcase.c:60:7: error: too many arguments to function '_tcase_add_test'
       _tcase_add_test(tc, tf, name, signo, 0, start, end);
       ^~~~~~~~~~~~~~~
In file included from s2tcase.c:35:_tcase_add_test(tc, tf, name, signo, 0, start, end);
/home/buildroot/autobuild/instance-0/output-1/host/arm-buildroot-linux-musleabihf/sysroot/usr/include/check.h:331:27: note: declared here
 CK_DLL_EXP void CK_EXPORT _tcase_add_test(TCase * tc, const TTest * ttest,
                           ^~~~~~~~~~~~~~~
mjerris commented 4 years ago

this doesn't appear to be an issue with buildroot. Looks to be an issue with you have a version of the check test framework installed but it is the wrong version. you can either uninstall that lib or find the right version it needs. we are looking at maybe moving this into tree because this library keeps changing api in non backwards compat ways.

hthiery commented 4 years ago

ah ok .. but as far as I can see the test should be disabled with "--disable-unit-tests".

I supposed that the test code should not be compiled?

hthiery commented 4 years ago

Oh. I see that "--disable-unit-tests" is used for the libvpx. Do you know if there is the possibilty to disable the tests for freeswitch?

micmac1 commented 4 years ago

Hi @hthiery and @mjerris

I ran into the same issue when upgrading FS to 1.10.3 on OpenWrt. I patched the tests out again, see this commit.

To me it looks like commit 2dced93 enabled some extra tests ("tests s2check utils") that were there previously but not enabled. And now that these are enabled we get compiler errors. Probably because the tests haven't been updated for a while.

At least that's how it looks to me, but I've been wrong before. What Michael said about incompatible version of check test framework could well be. I couldn't get the test to fail on my own computer's buildroot. But on openwrt build bots the checks were failing. I'm not even aware that I have a test check framework installed on my box, though :/

Kind regards, Seb

micmac1 commented 4 years ago

@mjerris could you please elaborate what you mean with "check test framework"? Maybe post a link?

hthiery commented 4 years ago

HI,

I did the follwing workaround just for trial to build without the failure:

--- a/libs/sofia-sip/configure.ac
+++ b/libs/sofia-sip/configure.ac
@@ -246,12 +246,12 @@ SAC_SOFIA_SU
 SAC_OPENSSL
 SAC_TPORT

-dnl Check is used for testing
-PKG_CHECK_MODULES(CHECK, check >= 0.9.4, have_check="yes", have_check="no")
-AM_CONDITIONAL(HAVE_CHECK, test x"$have_check" = "xyes")
-if test x"$have_check" = "xyes"; then
-   AC_DEFINE([HAVE_CHECK], 1, [Define to 1 if check library is available])
-fi
+#dnl Check is used for testing
+#PKG_CHECK_MODULES(CHECK, check >= 0.9.4, have_check="yes", have_check="no")
+#AM_CONDITIONAL(HAVE_CHECK, test x"$have_check" = "xyes")
+#if test x"$have_check" = "xyes"; then
+#   AC_DEFINE([HAVE_CHECK], 1, [Define to 1 if check library is available])
+#fi
 AC_CHECK_HEADERS([fnmatch.h])
micmac1 commented 4 years ago

HI,

I did the follwing workaround just for trial to build without the failure:

Thank you Heiko!

libcheck is indeed available in OpenWrt as well. The current version is 0.14.0.

hthiery commented 4 years ago

HI,

I did the follwing workaround just for trial to build without the failure:

--- a/libs/sofia-sip/configure.ac
+++ b/libs/sofia-sip/configure.ac
@@ -246,12 +246,12 @@ SAC_SOFIA_SU
 SAC_OPENSSL
 SAC_TPORT

-dnl Check is used for testing
-PKG_CHECK_MODULES(CHECK, check >= 0.9.4, have_check="yes", have_check="no")
-AM_CONDITIONAL(HAVE_CHECK, test x"$have_check" = "xyes")
-if test x"$have_check" = "xyes"; then
-   AC_DEFINE([HAVE_CHECK], 1, [Define to 1 if check library is available])
-fi
+#dnl Check is used for testing
+#PKG_CHECK_MODULES(CHECK, check >= 0.9.4, have_check="yes", have_check="no")
+#AM_CONDITIONAL(HAVE_CHECK, test x"$have_check" = "xyes")
+#if test x"$have_check" = "xyes"; then
+#   AC_DEFINE([HAVE_CHECK], 1, [Define to 1 if check library is available])
+#fi
 AC_CHECK_HEADERS([fnmatch.h])

Unfortunatly this workaround does not work ;-/

micmac1 commented 4 years ago

Unfortunatly this workaround does not work ;-/

It should. Without HAVE_CHECK the s2blabla.c is a no-op. Do you run autoreconf in buildroot?

hthiery commented 4 years ago

It should. Without HAVE_CHECK the s2blabla.c is a no-op. Do you run autoreconf in buildroot?

Yes. I rechecked and found with only preventing the define of HAVE_CHECK it works.

I'm not sure what makes the difference to the previous change. But it was another error.

--- a/libs/sofia-sip/configure.ac
+++ b/libs/sofia-sip/configure.ac
@@ -249,9 +249,9 @@ SAC_TPORT
 dnl Check is used for testing
 PKG_CHECK_MODULES(CHECK, check >= 0.9.4, have_check="yes", have_check="no")
 AM_CONDITIONAL(HAVE_CHECK, test x"$have_check" = "xyes")
-if test x"$have_check" = "xyes"; then
-   AC_DEFINE([HAVE_CHECK], 1, [Define to 1 if check library is available])
-fi
+#if test x"$have_check" = "xyes"; then
+#   AC_DEFINE([HAVE_CHECK], 1, [Define to 1 if check library is available])
+#fi
 AC_CHECK_HEADERS([fnmatch.h])
hthiery commented 4 years ago

@mjerris: do you have a better solution for that what I proposed? Or do you know if your proposal to move the lib into the tree will be implemented?

bkuhls commented 4 years ago

Reverting only the changes in s2tcase.c fixes the problem seen in buildroot for me: https://github.com/signalwire/freeswitch/commit/2dced93d94ce4be86c57451ee842afbf435749aa#diff-f47ff3b577412018f6faa1db68dae9c9

yann-morin-1998 commented 4 years ago

The root cause is that the check API changed in commit libcheck/check@3987c1d906ee68 where the number of parameters decreased from 7 to 6, when it lost the const char * fname (third) parameter.

This change is present in check version 0.13.0 onward, relased 2019-10-21. Note that the previous relase, 0.12.0 was from 2017-10-20, more than two years before...

So, as time passes, distros will get updated with a newer check version, and the problem will occur on newer systems.

bkuhls commented 4 years ago

The issue was fixed in 1.10.4 because libs/sofia-sip/ was removed: https://github.com/signalwire/freeswitch/pull/759

micmac1 commented 4 years ago

The issue was fixed in 1.10.4 because libs/sofia-sip/ was removed: #759

Yes, but you still need libsofia. It's now an "external" dep. Get it from https://github.com/freeswitch. Patch still required :)

micmac1 commented 4 years ago

Oh, just realized buildroot already packages sofia-sip 1.12.11 for janus-gateway. Not sure if the freeswitch "fork" is a drop-in replacement. On OpenWrt we don't have a package for sofia-sip yet, so no issue for us.

I'm wondering the same thing about the freeswitch spandsp fork. For now I've reintegrated the fork into freeswitch. Pull request currently open here.

mjerris commented 4 years ago

this sofia-sip we pulled into its own lib should be considered upstream for sofia. It is not maintained anywhere else but us. Spandsp is being maintained by the author. So neither are really forks. sofia should be a backwards compatible superset. Configure args to disable tests would be fine, patches to make the tests work with multiple versions of libcheck would be better. those patches can be submitted on github for those lib repos