[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 at bootlin.com> a écrit :
>
> Hello,
>
> On Sat, 1 Jun 2019 15:40:17 +0200
> Fabrice Fontaine <fontaine.fabrice at 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 at 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 at zacarias.com.ar>
>     Signed-off-by: Peter Korsgaard <jacmet at 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