[1/3] package/mosquitto: allow to build as static lib
diff mbox series

Message ID 20190718100636.27290-2-titouan.christophe@railnova.eu
State Changes Requested
Headers show
Series
  • Update package mosquitto
Related show

Commit Message

Titouan Christophe July 18, 2019, 10:06 a.m. UTC
Signed-off-by: Titouan Christophe <titouan.christophe@railnova.eu>
---
 package/mosquitto/Config.in    | 4 ----
 package/mosquitto/mosquitto.mk | 6 ++++++
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Peter Korsgaard Aug. 1, 2019, 10:23 a.m. UTC | #1
>>>>> "Titouan" == Titouan Christophe <titouan.christophe@railnova.eu> writes:

 > Signed-off-by: Titouan Christophe <titouan.christophe@railnova.eu>
 > ---
 >  package/mosquitto/Config.in    | 4 ----
 >  package/mosquitto/mosquitto.mk | 6 ++++++
 >  2 files changed, 6 insertions(+), 4 deletions(-)

 > diff --git a/package/mosquitto/Config.in b/package/mosquitto/Config.in
 > index 11b6d7891b..7135e86e69 100644
 > --- a/package/mosquitto/Config.in
 > +++ b/package/mosquitto/Config.in
 > @@ -1,6 +1,5 @@
 >  config BR2_PACKAGE_MOSQUITTO
 >  	bool "mosquitto"
 > -	depends on !BR2_STATIC_LIBS # builds .so
 >  	help
 >  	  Mosquitto is an open source message broker that implements
 >  	  the MQ Telemetry Transport protocol versions 3.1 and
 > @@ -22,6 +21,3 @@ config BR2_PACKAGE_MOSQUITTO_BROKER
 
 >  comment "mosquitto broker needs a system with MMU"
 >  	depends on BR2_PACKAGE_MOSQUTTO && !BR2_USE_MMU
 > -
 > -comment "mosquitto needs a toolchain w/ dynamic library"
 > -	depends on BR2_STATIC_LIBS
 > diff --git a/package/mosquitto/mosquitto.mk b/package/mosquitto/mosquitto.mk
 > index 51c0abd0ba..ed72af754a 100644
 > --- a/package/mosquitto/mosquitto.mk
 > +++ b/package/mosquitto/mosquitto.mk
 > @@ -17,6 +17,12 @@ MOSQUITTO_MAKE_OPTS = \
 >  	WITH_WRAP=no \
 >  	WITH_DOCS=no
 
 > +ifeq ($(BR2_STATIC_LIBS),y)
 > +MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=yes WITH_SHARED_LIBRARIES=no
 > +else
 > +MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no WITH_SHARED_LIBRARIES=yes
 > +endif

We actually have 3 variants:

- static only (BR2_STATIC_LIBS)
- shared only (BR2_SHARED_LIBS)
- shared and static (BR2_SHARED_STATIC_LIBS)

So I reworked it to take that into consideration and committed, thanks.
Peter Korsgaard Aug. 1, 2019, 10:41 a.m. UTC | #2
>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

Hi,

 >> +ifeq ($(BR2_STATIC_LIBS),y)
 >> +MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=yes WITH_SHARED_LIBRARIES=no
 >> +else
 >> +MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no WITH_SHARED_LIBRARIES=yes
 >> +endif

 > We actually have 3 variants:

 > - static only (BR2_STATIC_LIBS)
 > - shared only (BR2_SHARED_LIBS)
 > - shared and static (BR2_SHARED_STATIC_LIBS)

 > So I reworked it to take that into consideration and committed, thanks.

Sorry, I will have to take that back. I did a test compilation with a
completely static (BR2_STATIC_LIBS) setup, which failed:

In file included from security.c:25:
lib_load.h:23:11: fatal error: dlfcn.h: No such file or directory
 # include <dlfcn.h>
           ^~~~~~~~~
compilation terminated.

The loadable plugin logic in security.c seems to be built
unconditionally, so that cannot work in a completely static setup.

Care to bring up this issue upstream?

In the mean time I have marked your patch as changes requested. For
reference, the changes I did in mosquitto.mk look like this:

feq ($(BR2_SHARED_LIBS),y)
MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no
else
MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=yes
endif

ifeq ($(BR2_STATIC_LIBS),y)
MOSQUITTO_MAKE_OPTS += WITH_SHARED_LIBRARIES=no
else
MOSQUITTO_MAKE_OPTS += WITH_SHARED_LIBRARIES=yes
endif
Titouan Christophe Aug. 1, 2019, 5:09 p.m. UTC | #3
Hello Peter,

Thank you for reviewing this series !

On 8/1/19 12:41 PM, Peter Korsgaard wrote:
>>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:
> 
> Hi,
> 
>   >> +ifeq ($(BR2_STATIC_LIBS),y)
>   >> +MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=yes WITH_SHARED_LIBRARIES=no
>   >> +else
>   >> +MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no WITH_SHARED_LIBRARIES=yes
>   >> +endif
> 
>   > We actually have 3 variants:
> 
>   > - static only (BR2_STATIC_LIBS)
>   > - shared only (BR2_SHARED_LIBS)
>   > - shared and static (BR2_SHARED_STATIC_LIBS)
> 
>   > So I reworked it to take that into consideration and committed, thanks.
> 
> Sorry, I will have to take that back. I did a test compilation with a
> completely static (BR2_STATIC_LIBS) setup, which failed:

Could you share the defconfig you have used to obtain that result ?

When I run test-pkg with BR2_PACKAGE_MOSQUITTO=y (and 
BR2_PACKAGE_MOSQUITTO_BROKER defaults to y), I see: 
"""br-arm-full-static [5/6]: OK""".

> 
> In file included from security.c:25:
> lib_load.h:23:11: fatal error: dlfcn.h: No such file or directory
>   # include <dlfcn.h>
>             ^~~~~~~~~
> compilation terminated.
> 
> The loadable plugin logic in security.c seems to be built
> unconditionally, so that cannot work in a completely static setup.
> 
> Care to bring up this issue upstream?

As soon as I understand the conditions to reproduce, I will do so.

> 
> In the mean time I have marked your patch as changes requested. For
> reference, the changes I did in mosquitto.mk look like this:
> 
> feq ($(BR2_SHARED_LIBS),y)
> MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no
> else
> MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=yes
> endif
> 
> ifeq ($(BR2_STATIC_LIBS),y)
> MOSQUITTO_MAKE_OPTS += WITH_SHARED_LIBRARIES=no
> else
> MOSQUITTO_MAKE_OPTS += WITH_SHARED_LIBRARIES=yes
> endif
> 

Thanks for sharing !

Best regards,
Titouan
Peter Korsgaard Aug. 1, 2019, 5:25 p.m. UTC | #4
>>>>> "Titouan" == Titouan Christophe <titouan.christophe@railnova.eu> writes:

 > Hello Peter,
 > Thank you for reviewing this series !

 > On 8/1/19 12:41 PM, Peter Korsgaard wrote:
 >>>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:
 >> 
 >> Hi,
 >> 
 >> >> +ifeq ($(BR2_STATIC_LIBS),y)
 >> >> +MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=yes WITH_SHARED_LIBRARIES=no
 >> >> +else
 >> >> +MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no WITH_SHARED_LIBRARIES=yes
 >> >> +endif
 >> 
 >> > We actually have 3 variants:
 >> 
 >> > - static only (BR2_STATIC_LIBS)
 >> > - shared only (BR2_SHARED_LIBS)
 >> > - shared and static (BR2_SHARED_STATIC_LIBS)
 >> 
 >> > So I reworked it to take that into consideration and committed, thanks.
 >> 
 >> Sorry, I will have to take that back. I did a test compilation with a
 >> completely static (BR2_STATIC_LIBS) setup, which failed:

 > Could you share the defconfig you have used to obtain that result ?

It was afaik just the default arm variant with BR2_STATIC_LIBS enabled.

 > When I run test-pkg with BR2_PACKAGE_MOSQUITTO=y (and
 > BR2_PACKAGE_MOSQUITTO_BROKER defaults to y), I see:
 > """br-arm-full-static [5/6]: OK""".

Hmm, very odd. With current git HEAD and this patch 1/3 applied and the
following config:

BR2_arm=y
BR2_STATIC_LIBS=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-static-2019.02-rc1.tar.bz2"
BR2_TOOLCHAIN_EXTERNAL_GCC_7=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_4=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_PACKAGE_MOSQUITTO=y

(E.G. support/config-fragments/autobuild/br-arm-full-static.config + mosquitto)

I get the expected failure:

In file included from security.c:25:0:
lib_load.h:23:11: fatal error: dlfcn.h: No such file or directory
 # include <dlfcn.h>
           ^~~~~~~~~
compilation terminated.
Makefile:189: recipe for target 'security.o' failed

br-arm-full-static-2019.02-rc1.tar.bz2 does not contain a dlfcn.h file,
so that makes sense.
Titouan Christophe Aug. 2, 2019, 1:18 p.m. UTC | #5
Hello,
On 8/1/19 7:25 PM, Peter Korsgaard wrote:
> 
> It was afaik just the default arm variant with BR2_STATIC_LIBS enabled.
> 
>   > When I run test-pkg with BR2_PACKAGE_MOSQUITTO=y (and
>   > BR2_PACKAGE_MOSQUITTO_BROKER defaults to y), I see:
>   > """br-arm-full-static [5/6]: OK""".
> 
> Hmm, very odd. With current git HEAD and this patch 1/3 applied and the
> following config:
> 
> BR2_arm=y
> BR2_STATIC_LIBS=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
> BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-static-2019.02-rc1.tar.bz2"
> BR2_TOOLCHAIN_EXTERNAL_GCC_7=y
> BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_4=y
> BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
> # BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
> BR2_TOOLCHAIN_EXTERNAL_CXX=y
> BR2_PACKAGE_MOSQUITTO=y
> 

Indeed, when doing a regular make (and not test-pkg) with this defconfig 
I can reproduce the aforementioned failure.

> (E.G. support/config-fragments/autobuild/br-arm-full-static.config + mosquitto)
> 
> I get the expected failure:
> 
> In file included from security.c:25:0:
> lib_load.h:23:11: fatal error: dlfcn.h: No such file or directory
>   # include <dlfcn.h>
>             ^~~~~~~~~
> compilation terminated.
> Makefile:189: recipe for target 'security.o' failed
> 
> br-arm-full-static-2019.02-rc1.tar.bz2 does not contain a dlfcn.h file,
> so that makes sense.
> 

Now, when looking at mosquitto, I understand that WITH_SHARED_LIBRARY=no 
implies that libmosquitto.so won't be created (and linked against).

However, the failure over here is due to some code in the broker which 
is intended to dynamically load modules at runtime, no matter if 
mosquitto is running inside a statically linked executable or from a dylib.

As such, I suggest then to only make the broker depend on 
BR2_SHARED_LIBS, while still allowing the client lib to be built 
statically, which in my opinion makes sense for embedded targets.
What do you think ?

Kind regards,

Titouan
Peter Korsgaard Aug. 2, 2019, 3:58 p.m. UTC | #6
>>>>> "Titouan" == Titouan Christophe <titouan.christophe@railnova.eu> writes:

Hi,

 > Indeed, when doing a regular make (and not test-pkg) with this
 > defconfig I can reproduce the aforementioned failure.

Funky, but OK.


 > Now, when looking at mosquitto, I understand that
 > WITH_SHARED_LIBRARY=no implies that libmosquitto.so won't be created
 > (and linked against).

 > However, the failure over here is due to some code in the broker which
 > is intended to dynamically load modules at runtime, no matter if
 > mosquitto is running inside a statically linked executable or from a
 > dylib.

 > As such, I suggest then to only make the broker depend on
 > BR2_SHARED_LIBS, while still allowing the client lib to be built
 > statically, which in my opinion makes sense for embedded targets.
 > What do you think ?

Yes, that could work. Will you send a patch for that?

Patch
diff mbox series

diff --git a/package/mosquitto/Config.in b/package/mosquitto/Config.in
index 11b6d7891b..7135e86e69 100644
--- a/package/mosquitto/Config.in
+++ b/package/mosquitto/Config.in
@@ -1,6 +1,5 @@ 
 config BR2_PACKAGE_MOSQUITTO
 	bool "mosquitto"
-	depends on !BR2_STATIC_LIBS # builds .so
 	help
 	  Mosquitto is an open source message broker that implements
 	  the MQ Telemetry Transport protocol versions 3.1 and
@@ -22,6 +21,3 @@  config BR2_PACKAGE_MOSQUITTO_BROKER
 
 comment "mosquitto broker needs a system with MMU"
 	depends on BR2_PACKAGE_MOSQUTTO && !BR2_USE_MMU
-
-comment "mosquitto needs a toolchain w/ dynamic library"
-	depends on BR2_STATIC_LIBS
diff --git a/package/mosquitto/mosquitto.mk b/package/mosquitto/mosquitto.mk
index 51c0abd0ba..ed72af754a 100644
--- a/package/mosquitto/mosquitto.mk
+++ b/package/mosquitto/mosquitto.mk
@@ -17,6 +17,12 @@  MOSQUITTO_MAKE_OPTS = \
 	WITH_WRAP=no \
 	WITH_DOCS=no
 
+ifeq ($(BR2_STATIC_LIBS),y)
+MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=yes WITH_SHARED_LIBRARIES=no
+else
+MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no WITH_SHARED_LIBRARIES=yes
+endif
+
 # adns uses getaddrinfo_a
 ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),y)
 MOSQUITTO_MAKE_OPTS += WITH_ADNS=yes