Bug 303520

Summary:	giflib 5 compatibility
Product:	[Applications] konqueror	Reporter:	kde
Component:	khtml	Assignee:	Konqueror Developers <konq-bugs>
Status:	RESOLVED FIXED
Severity:	normal	CC:	adaptee, cfeck, daniel, neundorf, rakuco
Priority:	NOR
Version:	Git
Target Milestone:	---
Platform:	MacPorts
OS:	macOS
Latest Commit:	http://commits.kde.org/kdelibs/96af9ef4f8bd16816834008b73e7b29694136552	Version Fixed In:	4.10.2
Attachments:	Support giflib-5.x Differences between kde and upstream FindGIF.cmake Support giflib-5.x for kdelibs-4.10.0

Description kde 2012-07-14 10:04:08 UTC

kdelibs 4.8.3 does not seem to complete configuration when giflib 5.0.0 is installed.

Reproducible: Always

Steps to Reproduce:
1. install giflib 5.0.0
2. configure kdelibs 4.8.3

Actual Results:  
-----------------------------------------------------------------------------
-- The following REQUIRED packages could NOT be located on your system.
-- You must install these packages before continuing.
-----------------------------------------------------------------------------
   * giflib  <http://sourceforge.net/projects/giflib>
     GIF image format support
     Required by khtml.


Expected Results:  
successful configuration

Comment 1 Niels Rogalla 2012-12-12 08:58:15 UTC

Created attachment 75794 [details]
Support giflib-5.x

Added patch which let kdelibs configure, compile and install. Used cmake version was 2.8.10.2. I did not test the final result yet.

Comment 2 Christoph Feck 2012-12-12 14:29:41 UTC

Not sure if we can delete FindGIF.cmake, because I do not know if the minimum required cmake version already ships it.

Comment 3 Niels Rogalla 2012-12-13 12:06:26 UTC

Created attachment 75812 [details]
Differences between kde and upstream FindGIF.cmake

Makes sense, but FindGIF.cmake seems to be alreasy deleted in trunk. Maybe a sync with FindGIF.cmake of cmake-2.8.10 can fix the configure issues?

Comment 4 Raphael Kubo da Costa 2013-01-15 22:39:32 UTC

FWIW, kdelibs commit 903ce6b2ee9bc09cabfb5ca36d82d50db986c7a6 removed our copy of FindGIF.cmake since we have started depending on CMake 2.8.9 for KDE 4.10.

Since there won't be a KDE 4.9.6, I guess we can close this as fixed.

Comment 5 Christoph Feck 2013-01-16 01:35:15 UTC

From what I see in the patch at comment #1, supporting version 5 not only requires an updated FindGIF.cmake, but also code changes in khtml. Can someone check if this is really fixed?

Comment 6 Raphael Kubo da Costa 2013-01-16 16:03:17 UTC

D'oh, sorry, you are right, the build still fails with giflib 5 here.

Niels, would you mind sending this patch through ReviewBoard?

A few comments while here:

+#if GIFLIB_MAJOR >= 5
+    static unsigned int decode16Bit(unsigned char* signedLoc)
+#else
     static unsigned int decode16Bit(char* signedLoc)
+#endif

The GifByteType typedef is already an unsigned char for me here with giflib 4.1.6; are you sure the build really doesn't work without this change? Otherwise it could go in separately.

+#if GIFLIB_MAJOR >= 5
+	int ErrorCode;
+        GifFileType* file = DGifOpen(this, gifReaderBridge, &ErrorCode);
+#else
         GifFileType* file = DGifOpen(this, gifReaderBridge);
+#endif

ErrorCode is probably not needed since we don't do anything with it; just pass NULL/0 in the last parameter.

Comment 7 Niels Rogalla 2013-02-11 13:16:31 UTC

Created attachment 77129 [details]
Support giflib-5.x for kdelibs-4.10.0

Sorry for my late response.
Yes, the GifByteType typedef change to an unsigned char is required, or the build against giflib-5.x fails (5.0.4 at time of writing).

-- Error --
kdelibs-4.10.0/khtml/imload/decoders/gifloader.cpp:427:75:
error: invalid conversion from 'GifByteType* {aka unsigned char*}' to 'char*' [-fpermissive]
--

As you suggested, i have removed the ErrorCode variable and just pass NULL as last argument.

Added new patch against kdelibs-4.10.0.