[Buildroot] [PATCH 1/1] package/mpd: fix static build with tremor and vorbis

[Buildroot] [PATCH 1/1] package/mpd: fix static build with tremor and vorbis

[Fabrice Fontaine]

mpd can't be built statically with tremor and vorbis, build fails on:
block.c:(.text+0x11c): multiple definition of `vorbis_block_init'
/usr/lfs/v0/rc-buildroot-test/scripts/instance-2/output/host/bin/../sparc-buildroot-linux-uclibc/sysroot/usr/lib/libvorbisidec.a(block.o):block.c:(.text+0x0): first defined here
/usr/lfs/v0/rc-buildroot-test/scripts/instance-2/output/host/opt/ext-toolchain/bin/../lib/gcc/sparc-buildroot-linux-uclibc/7.4.0/../../../../sparc-buildroot-linux-uclibc/bin/ld: Disabling relaxation: it will not work with multiple definitions
/usr/lfs/v0/rc-buildroot-test/scripts/instance-2/output/host/bin/../sparc-buildroot-linux-uclibc/sysroot/usr/lib/libvorbis.a(block.o): In function `_vorbis_block_alloc':

Fix this by enabling vorbis or tremor depending on BR2_SOFT_FLOAT value
as suggested by J?rg in https://patchwork.ozlabs.org/patch/1085899

It should be noted that upstream removed the possibility to enable
tremor and vorbis at the same time since version 0.21.6 and
https://github.com/MusicPlayerDaemon/MPD/commit/c18cd941aaa4c16ac713bc4f1e16ada5281d689e

Fixes:
 - http://autobuild.buildroot.org/results/60c721a82ffd668bebf02d80bca83780d6cdb2f2

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/mpd/Config.in | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/package/mpd/Config.in b/package/mpd/Config.in
index d3844391a2..039c2402c2 100644
--- a/package/mpd/Config.in
+++ b/package/mpd/Config.in
@@ -9,7 +9,6 @@ menuconfig BR2_PACKAGE_MPD
 	depends on !BR2_TOOLCHAIN_HAS_GCC_BUG_64735 # exception_ptr
 	select BR2_PACKAGE_BOOST
 	select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE
-	select BR2_PACKAGE_MPD_TREMOR if !(BR2_PACKAGE_MPD_MAD || BR2_PACKAGE_MPD_MPG123 || BR2_PACKAGE_MPD_VORBIS || BR2_PACKAGE_MPD_WAVPACK || BR2_PACKAGE_MPD_FLAC || BR2_PACKAGE_MPD_MUSEPACK || BR2_PACKAGE_MPD_FFMPEG)
 	help
 	  MPD is a flexible, powerful, server-side application
 	  for playing music. Through plugins and libraries
@@ -132,18 +131,20 @@ config BR2_PACKAGE_MPD_OPUS
 	  Select this if you want to play back OPUS encoded files.
 
 config BR2_PACKAGE_MPD_TREMOR
-	bool "tremor"
+	bool "tremor" if !BR2_SOFT_FLOAT
 	select BR2_PACKAGE_LIBOGG
 	select BR2_PACKAGE_TREMOR
+	default y if !BR2_SOFT_FLOAT
 	help
 	  Enable vorbis input support.
 	  Select this if you want to play back OGG files on softfloat
 	  targets.
 
 config BR2_PACKAGE_MPD_VORBIS
-	bool "vorbis"
+	bool "vorbis" if BR2_SOFT_FLOAT
 	select BR2_PACKAGE_LIBOGG
 	select BR2_PACKAGE_LIBVORBIS
+	default y if BR2_SOFT_FLOAT
 	help
 	  Enable vorbis input/streaming support.
 	  Select this if you want to play back OGG files on hardfloat
-- 
2.20.1

[Buildroot] [PATCH 1/1] package/mpd: fix static build with tremor and vorbis

[Thomas Petazzoni]

Hello Fabrice,

On Sat,  1 Jun 2019 14:42:31 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> diff --git a/package/mpd/Config.in b/package/mpd/Config.in
> index d3844391a2..039c2402c2 100644
> --- a/package/mpd/Config.in
> +++ b/package/mpd/Config.in
> @@ -9,7 +9,6 @@ menuconfig BR2_PACKAGE_MPD
>  	depends on !BR2_TOOLCHAIN_HAS_GCC_BUG_64735 # exception_ptr
>  	select BR2_PACKAGE_BOOST
>  	select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE
> -	select BR2_PACKAGE_MPD_TREMOR if !(BR2_PACKAGE_MPD_MAD || BR2_PACKAGE_MPD_MPG123 || BR2_PACKAGE_MPD_VORBIS || BR2_PACKAGE_MPD_WAVPACK || BR2_PACKAGE_MPD_FLAC || BR2_PACKAGE_MPD_MUSEPACK || BR2_PACKAGE_MPD_FFMPEG)

Why was this select here in the first place ? Does it build fine with
no sub-option enabled ?

>  	help
>  	  MPD is a flexible, powerful, server-side application
>  	  for playing music. Through plugins and libraries
> @@ -132,18 +131,20 @@ config BR2_PACKAGE_MPD_OPUS
>  	  Select this if you want to play back OPUS encoded files.
>  
>  config BR2_PACKAGE_MPD_TREMOR
> -	bool "tremor"
> +	bool "tremor" if !BR2_SOFT_FLOAT
>  	select BR2_PACKAGE_LIBOGG
>  	select BR2_PACKAGE_TREMOR
> +	default y if !BR2_SOFT_FLOAT
>  	help
>  	  Enable vorbis input support.
>  	  Select this if you want to play back OGG files on softfloat
>  	  targets.

So for soft float targets, we should use tremor, but you make it depend
on !BR2_SOFT_FLOAT.

>  config BR2_PACKAGE_MPD_VORBIS
> -	bool "vorbis"
> +	bool "vorbis" if BR2_SOFT_FLOAT
>  	select BR2_PACKAGE_LIBOGG
>  	select BR2_PACKAGE_LIBVORBIS
> +	default y if BR2_SOFT_FLOAT
>  	help
>  	  Enable vorbis input/streaming support.
>  	  Select this if you want to play back OGG files on hardfloat

And for hard float targets, we should use vorbis, but you make it
available only on soft-float targets.

This seems weird.

Also, I am not sure why you are not just adding a depends on
BR2_SOFT_FLOAT in one case, and a "depends on !BR2_SOFT_FLOAT" in the
other case.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/mpd: fix static build with tremor and vorbis

[Fabrice Fontaine]

Dear Thomas,

Le sam. 1 juin 2019 ? 15:03, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a ?crit :
>
> Hello Fabrice,
>
> On Sat,  1 Jun 2019 14:42:31 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > diff --git a/package/mpd/Config.in b/package/mpd/Config.in
> > index d3844391a2..039c2402c2 100644
> > --- a/package/mpd/Config.in
> > +++ b/package/mpd/Config.in
> > @@ -9,7 +9,6 @@ menuconfig BR2_PACKAGE_MPD
> >       depends on !BR2_TOOLCHAIN_HAS_GCC_BUG_64735 # exception_ptr
> >       select BR2_PACKAGE_BOOST
> >       select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE
> > -     select BR2_PACKAGE_MPD_TREMOR if !(BR2_PACKAGE_MPD_MAD || BR2_PACKAGE_MPD_MPG123 || BR2_PACKAGE_MPD_VORBIS || BR2_PACKAGE_MPD_WAVPACK || BR2_PACKAGE_MPD_FLAC || BR2_PACKAGE_MPD_MUSEPACK || BR2_PACKAGE_MPD_FFMPEG)
>
> Why was this select here in the first place ? Does it build fine with
> no sub-option enabled ?
It builds fine without it. I don't know why this select was added.
>
> >       help
> >         MPD is a flexible, powerful, server-side application
> >         for playing music. Through plugins and libraries
> > @@ -132,18 +131,20 @@ config BR2_PACKAGE_MPD_OPUS
> >         Select this if you want to play back OPUS encoded files.
> >
> >  config BR2_PACKAGE_MPD_TREMOR
> > -     bool "tremor"
> > +     bool "tremor" if !BR2_SOFT_FLOAT
> >       select BR2_PACKAGE_LIBOGG
> >       select BR2_PACKAGE_TREMOR
> > +     default y if !BR2_SOFT_FLOAT
> >       help
> >         Enable vorbis input support.
> >         Select this if you want to play back OGG files on softfloat
> >         targets.
>
> So for soft float targets, we should use tremor, but you make it depend
> on !BR2_SOFT_FLOAT.
Indeed, this is a mistake.
>
> >  config BR2_PACKAGE_MPD_VORBIS
> > -     bool "vorbis"
> > +     bool "vorbis" if BR2_SOFT_FLOAT
> >       select BR2_PACKAGE_LIBOGG
> >       select BR2_PACKAGE_LIBVORBIS
> > +     default y if BR2_SOFT_FLOAT
> >       help
> >         Enable vorbis input/streaming support.
> >         Select this if you want to play back OGG files on hardfloat
>
> And for hard float targets, we should use vorbis, but you make it
> available only on soft-float targets.
>
> This seems weird.
>
> Also, I am not sure why you are not just adding a depends on
> BR2_SOFT_FLOAT in one case, and a "depends on !BR2_SOFT_FLOAT" in the
> other case.
OK, I'll just add a depends on. I'll change the current behavior but I
think this is acceptable.
>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice

[Buildroot] [PATCH 1/1] package/mpd: fix static build with tremor and vorbis

[Thomas Petazzoni]

Hello,

On Sat, 1 Jun 2019 15:40:17 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > > -     select BR2_PACKAGE_MPD_TREMOR if !(BR2_PACKAGE_MPD_MAD || BR2_PACKAGE_MPD_MPG123 || BR2_PACKAGE_MPD_VORBIS || BR2_PACKAGE_MPD_WAVPACK || BR2_PACKAGE_MPD_FLAC || BR2_PACKAGE_MPD_MUSEPACK || BR2_PACKAGE_MPD_FFMPEG)  
> >
> > Why was this select here in the first place ? Does it build fine with
> > no sub-option enabled ?  
> It builds fine without it. I don't know why this select was added.

Should be mentioned in the commit log, and or perhaps even be part of a
separate preparation patch.

The reason why this select was added is explained in commit
d428dddf680b067c13ba9a08bfd502c7286cffe7:

commit d428dddf680b067c13ba9a08bfd502c7286cffe7
Author: Gustavo Zacarias <gustavo@zacarias.com.ar>
Date:   Fri Feb 25 17:10:05 2011 -0300

    mpd: default to tremor input plugin
    
    MPD requires at least one input plugin to build.
    If there's none selected default to tremor (vorbis).
    
    Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
    Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>

Perhaps this is no longer true today.

> > Also, I am not sure why you are not just adding a depends on
> > BR2_SOFT_FLOAT in one case, and a "depends on !BR2_SOFT_FLOAT" in the
> > other case.  
> OK, I'll just add a depends on. I'll change the current behavior but I
> think this is acceptable.

Your idea was to try to remain backward compatible with
existing .config ? I'm not sure it's easy to achieve: we would want
tremor to be enabled if vorbis was enabled but we're on a soft-float
target. Except that you need to make vorbis depends on !BR2_SOFT_FLOAT,
so it will become disabled anyway.

I'd say, let's keep things simple, and use simple dependencies. BTW,
I'm not sure we should use BR2_SOFT_FLOAT. Perhaps, we should make the
MPD_TREMOR and MPD_VORBIS options mutually exclusive ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/mpd: fix static build with tremor and vorbis

[Fabrice Fontaine]

Le sam. 1 juin 2019 ? 15:47, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a ?crit :
>
> Hello,
>
> On Sat, 1 Jun 2019 15:40:17 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > > > -     select BR2_PACKAGE_MPD_TREMOR if !(BR2_PACKAGE_MPD_MAD || BR2_PACKAGE_MPD_MPG123 || BR2_PACKAGE_MPD_VORBIS || BR2_PACKAGE_MPD_WAVPACK || BR2_PACKAGE_MPD_FLAC || BR2_PACKAGE_MPD_MUSEPACK || BR2_PACKAGE_MPD_FFMPEG)
> > >
> > > Why was this select here in the first place ? Does it build fine with
> > > no sub-option enabled ?
> > It builds fine without it. I don't know why this select was added.
>
> Should be mentioned in the commit log, and or perhaps even be part of a
> separate preparation patch.
OK, I'll retest it again and split the patch.
>
> The reason why this select was added is explained in commit
> d428dddf680b067c13ba9a08bfd502c7286cffe7:
>
> commit d428dddf680b067c13ba9a08bfd502c7286cffe7
> Author: Gustavo Zacarias <gustavo@zacarias.com.ar>
> Date:   Fri Feb 25 17:10:05 2011 -0300
>
>     mpd: default to tremor input plugin
>
>     MPD requires at least one input plugin to build.
>     If there's none selected default to tremor (vorbis).
>
>     Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
>     Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>
> Perhaps this is no longer true today.
>
> > > Also, I am not sure why you are not just adding a depends on
> > > BR2_SOFT_FLOAT in one case, and a "depends on !BR2_SOFT_FLOAT" in the
> > > other case.
> > OK, I'll just add a depends on. I'll change the current behavior but I
> > think this is acceptable.
>
> Your idea was to try to remain backward compatible with
> existing .config ?
Yes, that was my idea.
> I'm not sure it's easy to achieve: we would want
> tremor to be enabled if vorbis was enabled but we're on a soft-float
> target. Except that you need to make vorbis depends on !BR2_SOFT_FLOAT,
> so it will become disabled anyway.
>
> I'd say, let's keep things simple, and use simple dependencies. BTW,
> I'm not sure we should use BR2_SOFT_FLOAT. Perhaps, we should make the
> MPD_TREMOR and MPD_VORBIS options mutually exclusive ?
My original idea (before seeing J?rg proposal) was to add a depends on
!BR2_STATIC_LIBS || !BR2_PACKAGE_VORBIS for tremor option and
!BR2_STATIC_LIBS || !BR2_PACKAGE_TREMOR for vorbis
I think that !BR2_STATIC_LIBS is not useful as upstream decided to
make both options exclusive so ok I'll just make them mutually
exclusive.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice