Bug 347722 – dev-python/gdmodule-0.56-r1 applies incorrect patch

• Status: RESOLVED FIXED
• Product: Gentoo Linux

[Description Christopher Schwan 2010-12-04 14:26:16 UTC]

Steven noticed that gdmodule-0.56-r1 is wrongly patch: http://archives.gentoo.org/gentoo-science/msg_745e77e63f260b06185bcb133df4ef5c.xml

-r1 introduced a patch because of bug #346751 . The important section looks like:

@@ -57,9 +57,6 @@ incdirs = dirtest([

 want_libs = [
     "gd",
-    "jpeg", "png", "gif", "z",
-    "X11", "Xpm",
-    "ttf", "freetype",
 ]

 libs = filetest(libdirs, want_libs)

which removes linking to to these libraries (-l... flags) as well as preprocessor definitions (-DHAVE_LIB...). This means that support for PNG, JPEG and so on is disabled which is wrong. I tried to solve that and noticed the following:

In gdmodule's c-source file, the following macros are used:

- HAVE_LIBTTF, if defined this in turn defines HAVE_LIBFREETYPE
- HAVE_LIBGIF
- HAVE_LIBPNG
- HAVE_LIBJPEG
- HAVE_LIBXPM

Using readelf I checked that basically only gd is linked. Since Setup.py implements some kind of automagic dependency (definitions and libraries are removed if they are not found), I suggest to patch the setup script (I will upload patch shortly).

[Comment 1 Christopher Schwan 2010-12-04 14:27:26 UTC]

Created attachment 256318 [details, diff]
patch proposal

[Comment 2 Justin Lecher (RETIRED) 2010-12-05 17:58:44 UTC]

Thanks for the patch. But this means, support for all and depend on all. Can we go with CPPFLAGS to trigger it finer?

[Comment 3 Christopher Schwan 2010-12-05 18:58:44 UTC]

(In reply to comment #2)
> Thanks for the patch. But this means, support for all and depend on all. Can we
> go with CPPFLAGS to trigger it finer?
> 

Yes! After applying the patch we can selectively disable features using a command like

    sed -i '/( "HAVE_LIBFREETYPE", None )/d' Setup.py

and USE-flags matching those of media-libs/gd. Note that in media-libs/gd gif support is enabled by default (see configure.ac:18 in gd's tarball) - we have to shift down the line with HAVE_LIBGIF in order to maintain correct syntax. I am going to upload a new patch and ebuild.

[Comment 4 Christopher Schwan 2010-12-05 18:59:55 UTC]

Created attachment 256416 [details]
new ebuild

[Comment 5 Christopher Schwan 2010-12-05 19:00:47 UTC]

Created attachment 256417 [details, diff]
new patch

[Comment 6 Justin Lecher (RETIRED) 2010-12-05 19:03:32 UTC]

I think something like

use png && append-cppflags -DHAVE_LIBPNG

would be more sufficient.

[Comment 7 Christopher Schwan 2010-12-05 19:20:52 UTC]

Why did I forget that ? But of course, that is much more simple - what about the new ebuild ?

[Comment 8 Christopher Schwan 2010-12-05 19:21:30 UTC]

Created attachment 256421 [details]
new ebuild

[Comment 9 Justin Lecher (RETIRED) 2010-12-06 06:55:52 UTC]

(In reply to comment #8)
> Created an attachment (id=256421) [details]
> new ebuild
> 

That looks good. What about gif?

[Comment 10 Christopher Schwan 2010-12-06 07:08:22 UTC]

(In reply to comment #9)
> (In reply to comment #8)
> > Created an attachment (id=256421) [details] [details]
> > new ebuild
> > 
> 
> That looks good. What about gif?
> 

Gif is enabled by default in gd (see my comment #3) - it also does not pull in any dependencies. Thus I think we should enable it by default, too.

[Comment 11 Christopher Schwan 2010-12-06 07:10:39 UTC]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Created an attachment (id=256421) [details] [details] [details]
> > > new ebuild
> > > 
> > 
> > That looks good. What about gif?
> > 
> 
> Gif is enabled by default in gd (see my comment #3) - it also does not pull in
> any dependencies. Thus I think we should enable it by default, too.
> 

I mean it really is enabled with the current patch!

[Comment 12 Christopher Schwan 2010-12-06 07:19:53 UTC]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > Created an attachment (id=256421) [details] [details] [details] [details]
> > > > new ebuild
> > > > 
> > > 
> > > That looks good. What about gif?
> > > 
> > 
> > Gif is enabled by default in gd (see my comment #3) - it also does not pull in
> > any dependencies. Thus I think we should enable it by default, too.
> > 
> 
> I mean it really is enabled with the current patch!
> 

Sorry - did not upload the current one. The following ebuild and patch should be final ...

[Comment 13 Christopher Schwan 2010-12-06 07:20:35 UTC]

Created attachment 256476 [details]
new ebuild

[Comment 14 Christopher Schwan 2010-12-06 07:21:03 UTC]

Created attachment 256478 [details, diff]
new patch

[Comment 15 Justin Lecher (RETIRED) 2010-12-06 09:35:25 UTC]

(In reply to comment #13)
> Created an attachment (id=256476) [details]
> new ebuild
> 

it is still lacking IUSE="gif". I will add that and commit it. THanks for your efforts.

[Comment 16 Justin Lecher (RETIRED) 2010-12-06 15:54:54 UTC]

+*gdmodule-0.56-r2 (06 Dec 2010)
+
+  06 Dec 2010; Justin Lecher <[e-mail]> -gdmodule-0.56-r1.ebuild,
+  +gdmodule-0.56-r2.ebuild, +files/gdmodule-0.56-fix-libs.patch:
+  Correct handling of image type support, thanks cschwan for providing the
+  patches, #347722
+