diff mbox

[01/21] libftdi: C++ bindings need boost

Message ID 1453676887-31236-2-git-send-email-thomas.petazzoni@free-electrons.com
State Accepted
Headers show

Commit Message

Thomas Petazzoni Jan. 24, 2016, 11:07 p.m. UTC
According to libftdi configure.in:

"""
dnl libftdi C++ wrapper. Needs boost.
[...]
        if test "x$HAVE_BOOST" != "xyes"; then
            AC_MSG_ERROR(Sorry, we need the boost library for the C++ wrapper)
        fi
"""

And indeed, if you enable BR2_PACKAGE_LIBTFDI_CPP but don't have Boost
enabled, the libfdipp library is not built. To fix this, this commit
changes BR2_PACKAGE_LIBTFDI_CPP to select Boost.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/libftdi/Config.in  | 9 ++++++++-
 package/libftdi/libftdi.mk | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN Jan. 25, 2016, 5:04 p.m. UTC | #1
Thomas, All,

On 2016-01-25 00:07 +0100, Thomas Petazzoni spake thusly:
> According to libftdi configure.in:
> 
> """
> dnl libftdi C++ wrapper. Needs boost.
> [...]
>         if test "x$HAVE_BOOST" != "xyes"; then
>             AC_MSG_ERROR(Sorry, we need the boost library for the C++ wrapper)
>         fi
> """
> 
> And indeed, if you enable BR2_PACKAGE_LIBTFDI_CPP but don't have Boost
> enabled, the libfdipp library is not built. To fix this, this commit
> changes BR2_PACKAGE_LIBTFDI_CPP to select Boost.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Still, a few comments below...

> ---
>  package/libftdi/Config.in  | 9 ++++++++-
>  package/libftdi/libftdi.mk | 1 +
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/package/libftdi/Config.in b/package/libftdi/Config.in
> index 76b30b1..c0ab0f2 100644
> --- a/package/libftdi/Config.in
> +++ b/package/libftdi/Config.in
> @@ -12,11 +12,18 @@ config BR2_PACKAGE_LIBFTDI
>  if BR2_PACKAGE_LIBFTDI
>  
>  config BR2_PACKAGE_LIBTFDI_CPP
> -	depends on BR2_INSTALL_LIBSTDCPP
>  	bool "C++ bindings"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_PACKAGE_BOOST_ARCH_SUPPORTS
> +	depends on BR2_USE_WCHAR
> +	select BR2_PACKAGE_BOOST

boost also has a threads dependency, but that is covered by the main
symbol, because of libusb.

I wonder if we should duplicate the dependency here, with another
comment, like:

    depends on BR2_TOOLCHAIN_HAS_THREADS # Boost

Or at least state it in a comment:

    # Boost depends on threads, but that's already accounted for
    # in the main symbol because of libusb.

>  	help
>  	  C++ bindings for libftdi
>  
> +comment "libftdi C++ bindings need a toolchain w/ wchar, C++"
> +	depends on BR2_PACKAGE_BOOST_ARCH_SUPPORTS
> +	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP

Ditto the threads dependency.

I'm not sure if we really want to do that, hence my reved-by tag.

Regards,
Yann E. MORIN.

>  endif # BR2_PACKAGE_LIBFTDI
>  
>  comment "libftdi needs a toolchain w/ threads"
> diff --git a/package/libftdi/libftdi.mk b/package/libftdi/libftdi.mk
> index 8370b0a..28f20e3 100644
> --- a/package/libftdi/libftdi.mk
> +++ b/package/libftdi/libftdi.mk
> @@ -15,6 +15,7 @@ LIBFDTI_CONF_OPTS = --without-examples
>  
>  # configure detect it automaticaly so we need to force it
>  ifeq ($(BR2_PACKAGE_LIBTFDI_CPP),y)
> +LIBFTDI_DEPENDENCIES += boost
>  LIBFDTI_CONF_OPTS += --enable-libftdipp
>  else
>  LIBFDTI_CONF_OPTS += --disable-libftdipp
> -- 
> 2.6.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni Jan. 25, 2016, 8:04 p.m. UTC | #2
Hello,

On Mon, 25 Jan 2016 18:04:38 +0100, Yann E. MORIN wrote:

> >  config BR2_PACKAGE_LIBTFDI_CPP
> > -	depends on BR2_INSTALL_LIBSTDCPP
> >  	bool "C++ bindings"
> > +	depends on BR2_INSTALL_LIBSTDCPP
> > +	depends on BR2_PACKAGE_BOOST_ARCH_SUPPORTS
> > +	depends on BR2_USE_WCHAR
> > +	select BR2_PACKAGE_BOOST
> 
> boost also has a threads dependency, but that is covered by the main
> symbol, because of libusb.
> 
> I wonder if we should duplicate the dependency here, with another
> comment, like:
> 
>     depends on BR2_TOOLCHAIN_HAS_THREADS # Boost
> 
> Or at least state it in a comment:
> 
>     # Boost depends on threads, but that's already accounted for
>     # in the main symbol because of libusb.

We don't have clear rules about what to do in such situations. We
sometimes say to duplicate the dependencies, but sometimes it looks
really really annoying to duplicate all those dependencies when you
*know* that the top-level package can anyway not be enabled in this or
that situation.

Thomas
diff mbox

Patch

diff --git a/package/libftdi/Config.in b/package/libftdi/Config.in
index 76b30b1..c0ab0f2 100644
--- a/package/libftdi/Config.in
+++ b/package/libftdi/Config.in
@@ -12,11 +12,18 @@  config BR2_PACKAGE_LIBFTDI
 if BR2_PACKAGE_LIBFTDI
 
 config BR2_PACKAGE_LIBTFDI_CPP
-	depends on BR2_INSTALL_LIBSTDCPP
 	bool "C++ bindings"
+	depends on BR2_INSTALL_LIBSTDCPP
+	depends on BR2_PACKAGE_BOOST_ARCH_SUPPORTS
+	depends on BR2_USE_WCHAR
+	select BR2_PACKAGE_BOOST
 	help
 	  C++ bindings for libftdi
 
+comment "libftdi C++ bindings need a toolchain w/ wchar, C++"
+	depends on BR2_PACKAGE_BOOST_ARCH_SUPPORTS
+	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP
+
 endif # BR2_PACKAGE_LIBFTDI
 
 comment "libftdi needs a toolchain w/ threads"
diff --git a/package/libftdi/libftdi.mk b/package/libftdi/libftdi.mk
index 8370b0a..28f20e3 100644
--- a/package/libftdi/libftdi.mk
+++ b/package/libftdi/libftdi.mk
@@ -15,6 +15,7 @@  LIBFDTI_CONF_OPTS = --without-examples
 
 # configure detect it automaticaly so we need to force it
 ifeq ($(BR2_PACKAGE_LIBTFDI_CPP),y)
+LIBFTDI_DEPENDENCIES += boost
 LIBFDTI_CONF_OPTS += --enable-libftdipp
 else
 LIBFDTI_CONF_OPTS += --disable-libftdipp