[Buildroot] [PATCH] package/sdl_mixer: disable libmad support for static build only

[Buildroot] [PATCH] package/sdl_mixer: disable libmad support for static build only

[Romain Naour]

Following the discussion on the ML [1], all package using SDL_mixer will
unlikely forget to link with -lmad.

[...]sysroot/usr/lib/libSDL_mixer.a(music_mad.o): In function `read_next_frame':
music_mad.c:(.text+0xbc): undefined reference to `mad_stream_buffer'
music_mad.c:(.text+0xd0): undefined reference to `mad_frame_decode'
music_mad.c:(.text+0x134): undefined reference to `mad_timer_add'

Until SDL_mixer and other package using it are not fixed, disable libmad support
for static build only.

Fixes:
http://autobuild.buildroot.net/results/e09/e09ea3a64d396bbc855acf7c07fcbea28fb2395d

[1] http://lists.busybox.net/pipermail/buildroot/2016-February/153442.html

Signed-off-by: Romain Naour <romain.naour@gmail.com>
Cc: Rodrigo Rebello <rprebello@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/sdl_mixer/sdl_mixer.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/sdl_mixer/sdl_mixer.mk b/package/sdl_mixer/sdl_mixer.mk
index a602b6e..3547799 100644
--- a/package/sdl_mixer/sdl_mixer.mk
+++ b/package/sdl_mixer/sdl_mixer.mk
@@ -20,7 +20,8 @@ SDL_MIXER_CONF_OPTS = \
 	--disable-music-mp3 \
 	--disable-music-flac # configure script fails when cross compiling
 
-ifeq ($(BR2_PACKAGE_LIBMAD),y)
+# disable libmad support for static build only.
+ifeq ($(BR2_PACKAGE_LIBMAD)x$(BR2_STATIC_LIBS),yx)
 SDL_MIXER_CONF_OPTS += --enable-music-mp3-mad-gpl
 SDL_MIXER_DEPENDENCIES += libmad
 else
-- 
2.5.0

[Buildroot] [PATCH] package/sdl_mixer: disable libmad support for static build only

[Thomas Petazzoni]

Romain,

On Sun, 21 Feb 2016 17:55:08 +0100, Romain Naour wrote:
> Following the discussion on the ML [1], all package using SDL_mixer will
> unlikely forget to link with -lmad.
> 
> [...]sysroot/usr/lib/libSDL_mixer.a(music_mad.o): In function `read_next_frame':
> music_mad.c:(.text+0xbc): undefined reference to `mad_stream_buffer'
> music_mad.c:(.text+0xd0): undefined reference to `mad_frame_decode'
> music_mad.c:(.text+0x134): undefined reference to `mad_timer_add'
> 
> Until SDL_mixer and other package using it are not fixed, disable libmad support
> for static build only.
> 
> Fixes:
> http://autobuild.buildroot.net/results/e09/e09ea3a64d396bbc855acf7c07fcbea28fb2395d
> 
> [1] http://lists.busybox.net/pipermail/buildroot/2016-February/153442.html

I am not sure whether this sort of patch is the good option. Yes, it
avoids the autobuilder failure, but it doesn't really fix the problem
itself. We could say it's a good temporary solution for 2016.02, but on
the other hand, once this patch gets merged, we will most likely forget
that this problem exists.

So I'm hesitating between:

 1/ Applying your patch, and just forget about this issue, since we
    probably don't care much anyway.

 2/ Applying your patch, and asking you to fill in a bug report in our
    bug tracker so that we don't forget about the issue.

 3/ Not applying your patch, so that we keep the issue in our
    autobuilder failures until someone fixes it.

Peter, what do you think?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] package/sdl_mixer: disable libmad support for static build only

[Rodrigo Rebello]

Thomas, Romain,

2016-02-21 14:18 GMT-03:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Romain,
>
> On Sun, 21 Feb 2016 17:55:08 +0100, Romain Naour wrote:
>> Following the discussion on the ML [1], all package using SDL_mixer will
>> unlikely forget to link with -lmad.
>>
>> [...]sysroot/usr/lib/libSDL_mixer.a(music_mad.o): In function `read_next_frame':
>> music_mad.c:(.text+0xbc): undefined reference to `mad_stream_buffer'
>> music_mad.c:(.text+0xd0): undefined reference to `mad_frame_decode'
>> music_mad.c:(.text+0x134): undefined reference to `mad_timer_add'
>>
>> Until SDL_mixer and other package using it are not fixed, disable libmad support
>> for static build only.
>>
>> Fixes:
>> http://autobuild.buildroot.net/results/e09/e09ea3a64d396bbc855acf7c07fcbea28fb2395d
>>
>> [1] http://lists.busybox.net/pipermail/buildroot/2016-February/153442.html
>
> I am not sure whether this sort of patch is the good option. Yes, it
> avoids the autobuilder failure, but it doesn't really fix the problem
> itself. We could say it's a good temporary solution for 2016.02, but on
> the other hand, once this patch gets merged, we will most likely forget
> that this problem exists.
>
> So I'm hesitating between:
>
>  1/ Applying your patch, and just forget about this issue, since we
>     probably don't care much anyway.
>
>  2/ Applying your patch, and asking you to fill in a bug report in our
>     bug tracker so that we don't forget about the issue.
>
>  3/ Not applying your patch, so that we keep the issue in our
>     autobuilder failures until someone fixes it.
>
> Peter, what do you think?
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

I don't think this workaround is necessary at all. Adding a
'Libs.private' section to SDL_mixer.pc and fixing packages that depend
on SDL_mixer so that they use pkg-config doesn't seem too complicated.
Currently only chocolate-doom, lbreakout2, ltris and prboom depend on
it.

I'll work on a patch for chocolate-doom now and handle the other 3 later.

Regards,
Rodrigo

[Buildroot] [PATCH] package/sdl_mixer: disable libmad support for static build only

[Thomas Petazzoni]

Hello,

On Sun, 21 Feb 2016 17:06:13 -0300, Rodrigo Rebello wrote:

> I don't think this workaround is necessary at all. Adding a
> 'Libs.private' section to SDL_mixer.pc and fixing packages that depend
> on SDL_mixer so that they use pkg-config doesn't seem too complicated.
> Currently only chocolate-doom, lbreakout2, ltris and prboom depend on
> it.

This is obviously the best option. Unfortunately, many of those
packages are not maintained, so we can't upstream the patches you're
talking about...

> I'll work on a patch for chocolate-doom now and handle the other 3 later.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] package/sdl_mixer: disable libmad support for static build only

[Rodrigo Rebello]

Thomas,

2016-02-21 17:13 GMT-03:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
[snip]
> This is obviously the best option. Unfortunately, many of those
> packages are not maintained, so we can't upstream the patches you're
> talking about...
>

That's a good point. Well, at least chocolate-doom is being actively
maintained, so we'll be able to upstream a patch in that case.

As for the other 3, do you think keeping patches for them in buildroot
could turn out to be a hassle in the long run?

Rodrigo

[Buildroot] [PATCH] package/sdl_mixer: disable libmad support for static build only

[Thomas Petazzoni]

Dear Rodrigo Rebello,

On Sun, 21 Feb 2016 17:37:28 -0300, Rodrigo Rebello wrote:

> That's a good point. Well, at least chocolate-doom is being actively
> maintained, so we'll be able to upstream a patch in that case.
> 
> As for the other 3, do you think keeping patches for them in buildroot
> could turn out to be a hassle in the long run?

Having patches that are not upstream is a hassle in general.

But since those packages are basically unmaintained upstream as far as
I remember, it means upstream won't do any new release, and therefore
we won't have to update/change the patches. Having patches is really a
big annoyance when the upstream project is still active and makes
releases, since we have to update our patches whenever we bump the
package.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] package/sdl_mixer: disable libmad support for static build only

[Romain Naour]

Hi Rodrigo, Thomas, All,

Le 21/02/2016 21:06, Rodrigo Rebello a ?crit :
> Thomas, Romain,
> 

[snip]

> 
> I don't think this workaround is necessary at all. Adding a
> 'Libs.private' section to SDL_mixer.pc and fixing packages that depend
> on SDL_mixer so that they use pkg-config doesn't seem too complicated.
> Currently only chocolate-doom, lbreakout2, ltris and prboom depend on
> it.
> 
> I'll work on a patch for chocolate-doom now and handle the other 3 later.

I'll will mark this patch rejected in patchwork.

Thanks for working on this.

Best regards,
Romain

> 
> Regards,
> Rodrigo
>