Bug 486372 – media-libs/imlib uses PrintGifError which was changed to GifErrorString in giflib-4.2

• Status: RESOLVED DUPLICATE of bug 538976
• Product: Gentoo Linux

[Description Aaron Pelton 2013-09-28 20:29:43 UTC]

patch attached to resolve this.

[Comment 1 Aaron Pelton 2013-09-28 20:30:57 UTC]

Created attachment 359698 [details, diff]
PrintGifError to GifErrorString patch

[Comment 2 Jeroen Roovers (RETIRED) 2013-09-30 13:08:51 UTC]

*** Bug 486374 has been marked as a duplicate of this bug. ***

[Comment 3 Jeroen Roovers (RETIRED) 2013-10-14 12:29:37 UTC]

*** Bug 488010 has been marked as a duplicate of this bug. ***

[Comment 4 Franz Graf 2013-10-29 08:12:21 UTC]

(In reply to Aaron Pelton from comment #1)
> Created attachment 359698 [details, diff] [details, diff]
> PrintGifError to GifErrorString patch

I confirm the bug. Thanks for the patch. It is working, but is not applied automatically when put to e.g. /etc/portage/patches/media-libs/imlib-1.9.15/. epatch_user should be added to function src_prepare() of ebuild. Version bump requested.

[Comment 5 Martin von Gagern 2013-11-02 22:27:55 UTC]

Does GifErrorString actually print that string, the way PrintGifError does? The docs at http://giflib.sourceforge.net/gif_lib.html#idp139024 only mention a returned string. They also state the return type as int, which doesn't make too much sense.

GDAL for example does some more work to actually print that string, see http://trac.osgeo.org/gdal/browser/trunk/gdal/frmts/gif/gifdataset.cpp?rev=24608#L491.

[Comment 6 Aaron Pelton 2013-11-05 21:48:35 UTC]

(In reply to Martin von Gagern from comment #5)
> Does GifErrorString actually print that string, the way PrintGifError does?
> The docs at http://giflib.sourceforge.net/gif_lib.html#idp139024 only
> mention a returned string. They also state the return type as int, which
> doesn't make too much sense.
> 
> GDAL for example does some more work to actually print that string, see
> http://trac.osgeo.org/gdal/browser/trunk/gdal/frmts/gif/gifdataset.
> cpp?rev=24608#L491.

I would agree, the documentation is probably in error given the second link. I was working from a different sample patch (https://abf.rosalinux.ru/openmandriva/imlib/commit/b74ec4832c680c1bbe084cc94562f812fb05c7c5) which did what I proposed so I'm sure you're correct the second link work needs to be done. TBH, I didn't originally look very deeply nor at all the implications.

I'm willing to make the changes to the patch, but it's basically a one-liner from what I see so anybody could do it in 30 seconds. I confirmed PrintGifError just fprintf(stderr... the error string.

Further I see that v5 of giflib will require the error code as an arg. Not sure how soon v5 would be in the pipeline?

[Comment 7 Aaron Pelton 2014-01-20 16:22:50 UTC]

Created attachment 368256 [details, diff]
adjusted patch

added the missing fprintf

[Comment 8 Aaron Pelton 2014-01-20 16:28:46 UTC]

ignore adjusted patch...needs more work

[Comment 9 Aaron Pelton 2014-01-20 17:49:15 UTC]

Created attachment 368274 [details, diff]
improved patch which handles giflib 4/5 based on gdal code

[Comment 10 Alex Turbov 2014-01-22 07:13:23 UTC]

(In reply to Aaron Pelton from comment #9)
> Created attachment 368274 [details, diff] [details, diff]
> improved patch which handles giflib 4/5 based on gdal code

works fine.

[Comment 11 Greg Turner 2014-01-25 20:38:07 UTC]

Created attachment 368718 [details, diff]
imlib-1.9.15-myPrintGifError.patch

I seem to have written my own pseudo-reasonable patch for this as well, here it is.

Although clearly something should go into gentoo-x86 for this tout-suite, it's poor form to just stick a patch into Gentoo, for a real upstream bug, like this, without at least verifying that upstream trunk has their own solution.  Has anyone looked into that?

[Comment 12 Greg Turner 2014-01-26 04:48:49 UTC]

also see bug 499268

[Comment 13 Aaron Pelton 2014-01-27 14:06:02 UTC]

(In reply to Greg Turner from comment #11)
> Created attachment 368718 [details, diff] [details, diff]
> imlib-1.9.15-myPrintGifError.patch
>
> Although clearly something should go into gentoo-x86 for this tout-suite,
> it's poor form to just stick a patch into Gentoo, for a real upstream bug,
> like this, without at least verifying that upstream trunk has their own
> solution.  Has anyone looked into that?

I've poked around a little and don't find it listed (as imlib) in current gnome source (imlib 1.9 directory last updated 2009 and under z_archived) and I only have one package installed that depends on it.

So examining Kuickshow (the one) I find https://bugs.kde.org/show_bug.cgi?id=194875 which is a "migrate to qtlibsomething" from 2009 so they seem to be aware imlib was deprecated but continue using it for speed. I don't have an account for the kde bugtracker so I haven't reported it (seems redundant) but I can see an argument for an update.

IMO, because the patches are on what appears to be unchanging source, it certainly seems like a "faster" solution to pick one until the imlib dependency goes away which might be a while as the migration may not be trivial in any case.

[Comment 14 C. Wijtmans 2014-01-27 16:41:18 UTC]

i think this package should be masked.

[Comment 15 Aaron Pelton 2014-01-27 17:44:02 UTC]

(In reply to C.J. Wijtmans from comment #14)
> i think this package should be masked.

That sounds less than ideal as there appear to be a lot of ebuilds in packages that depend on imlib.

(imlib2 doesn't appear to be a dropin replacement)

[Comment 16 PaX Team 2014-01-29 17:42:08 UTC]

(In reply to Greg Turner from comment #11)
> Created attachment 368718 [details, diff] [details, diff]
> imlib-1.9.15-myPrintGifError.patch
> 
> I seem to have written my own pseudo-reasonable patch for this as well, here
> it is.

i think there's a typo in there: myPrintGitError2 -> myPrintGifError2 and PrintGitError -> GifErrorString

[Comment 17 C. Wijtmans 2014-01-30 12:09:20 UTC]

imlib-1.9.15-myPrintGifError.patch > imlib-1.9.15-myPrintGitError.patch

[Comment 18 Mario Bachmann 2014-03-22 14:36:36 UTC]

Same problem at the end of the compile phase of kuickshow. 

The patch imlib-1.9.15-myPrintGifError.patch for imlib solved it. 
The patch should go to the portage tree, I think.

[Comment 19 Greg Turner 2014-03-23 14:03:52 UTC]

(In reply to PaX Team from comment #16)
> (In reply to Greg Turner from comment #11)
> > Created attachment 368718 [details, diff] [details, diff] [details, diff]
> > imlib-1.9.15-myPrintGifError.patch
> > 
> > I seem to have written my own pseudo-reasonable patch for this as well, here
> > it is.
> 
> i think there's a typo in there: myPrintGitError2 -> myPrintGifError2 and
> PrintGitError -> GifErrorString

ack, that is indeed a typo.

[Comment 20 Greg Turner 2014-03-23 14:06:30 UTC]

Created attachment 373312 [details, diff]
imlib-1.9.15-myPrintGifError-r2.patch

Fix silly typo.

[Comment 21 Greg Turner 2014-03-23 14:15:21 UTC]

Created attachment 373314 [details, diff]
imlib-1.9.15-myPrintGifError-r3.patch

bleh... pretty sure there was a thinko in there too, as hinted at above (subtly -- almost missed it).

Corrected, I think, but untested.

Can someone who has slept more recently than I take a look and confirm my belief that this -r3 patch has one less bug than the -r2 version?

-gmt

[Comment 22 PaX Team 2014-03-23 17:49:01 UTC]

(In reply to Greg Turner from comment #21)
> Can someone who has slept more recently than I take a look and confirm my
> belief that this -r3 patch has one less bug than the -r2 version?

actually i think -r2 is closer to the truth save for the PrintGifError/GifErrorString rename i also needed.

[Comment 23 Greg Turner 2014-03-24 09:52:44 UTC]

(In reply to PaX Team from comment #22)
> (In reply to Greg Turner from comment #21)
> > Can someone who has slept more recently than I take a look and confirm my
> > belief that this -r3 patch has one less bug than the -r2 version?
> 
> actually i think -r2 is closer to the truth save for the
> PrintGifError/GifErrorString rename i also needed.

OK, got some sleep :)(In reply to PaX Team from comment #22)
> (In reply to Greg Turner from comment #21)
> > Can someone who has slept more recently than I take a look and confirm my
> > belief that this -r3 patch has one less bug than the -r2 version?
> 
> actually i think -r2 is closer to the truth save for the
> PrintGifError/GifErrorString rename i also needed.

I got some sleep finally :)

IIRC my original intent with MyPrintGifError/MyPrintGifError2 was to avoid any concerns about namespace conflicts, say, if these two units were linked into the same library.

As implemented, that's a non-issue: they're just macros, so they are not going to generate symbols.  Whether it's there as vestigial cruft, laziness, or a brainfart, it's not helping, so "MyPrintGifError2" should just be MyPrintGifError, unless I'm missing something.

Second, I think I now see what you are getting at (sleep really helps!) wrt PrintGifError/GifErrorString... I'll try again -- this time, with some testing -- and post an -r4 patch here.

[Comment 24 Greg Turner 2014-03-25 03:55:47 UTC]

Created attachment 373472 [details, diff]
imlib-1.9.15-myPrintGifError-r4.patch

Another attempt to clean up the mess in my patch.  Tested against giflib-4.2.3 and giflib-5.0.5.

[Comment 25 Greg Turner 2014-03-25 04:04:13 UTC]

Also gave this a try vs. giflib 4.1.6-r2; it compiles against that as well; hopefully all is well with it now.

[Comment 26 Michael Palimaka (kensington) 2014-04-01 10:50:25 UTC]

*** Bug 506178 has been marked as a duplicate of this bug. ***

[Comment 27 Jeroen Roovers (RETIRED) 2014-07-02 00:02:06 UTC]

*** Bug 514524 has been marked as a duplicate of this bug. ***

[Comment 28 Stefan Briesenick (RETIRED) 2014-08-25 12:16:43 UTC]

ping

[Comment 29 Maciej S. Szmigiero 2014-08-31 16:57:03 UTC]

I can confirm that the problem still occurs and patch from comment 24 fixes it.

[Comment 30 Michael Palimaka (kensington) 2015-05-06 13:29:27 UTC]

*** Bug 544350 has been marked as a duplicate of this bug. ***

[Comment 31 Jeroen Roovers (RETIRED) 2016-02-06 16:07:13 UTC]

*** Bug 538242 has been marked as a duplicate of this bug. ***

[Comment 32 Tijs Van Buggenhout 2016-02-11 12:35:23 UTC]

Created attachment 425194 [details, diff]
Build fixes for giflib5

/var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c: In function '_LoadGIF':
/var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:478:9: error: too few arguments to function 'DGifOpenFileHandle'
   gif = DGifOpenFileHandle(fd);
         ^
In file included from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/Imlib_private.h:43:0,
                 from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:4:
/usr/include/gif_lib.h:180:14: note: declared here
 GifFileType *DGifOpenFileHandle(int GifFileHandle, int *Error);
              ^
/var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:505:8: error: too few arguments to function 'DGifCloseFile'
        DGifCloseFile(gif);
        ^
In file included from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/Imlib_private.h:43:0,
                 from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:4:
/usr/include/gif_lib.h:183:9: note: declared here
     int DGifCloseFile(GifFileType * GifFile, int *ErrorCode);
         ^
/var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:511:8: error: too few arguments to function 'DGifCloseFile'
        DGifCloseFile(gif);
        ^
In file included from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/Imlib_private.h:43:0,
                 from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:4:
/usr/include/gif_lib.h:183:9: note: declared here
     int DGifCloseFile(GifFileType * GifFile, int *ErrorCode);
         ^
/var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:522:5: error: too few arguments to function 'DGifCloseFile'
     DGifCloseFile(gif);
     ^
In file included from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/Imlib_private.h:43:0,
                 from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:4:
/usr/include/gif_lib.h:183:9: note: declared here
     int DGifCloseFile(GifFileType * GifFile, int *ErrorCode);
         ^
/var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:611:3: error: too few arguments to function 'DGifCloseFile'
   DGifCloseFile(gif);
   ^
In file included from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/Imlib_private.h:43:0,
                 from /var/tmp/portage/media-libs/imlib-1.9.15-r4/work/imlib-1.9.15/Imlib/load.c:4:
/usr/include/gif_lib.h:183:9: note: declared here
     int DGifCloseFile(GifFileType * GifFile, int *ErrorCode);
         ^

[Comment 33 Pacho Ramos 2016-10-09 10:51:54 UTC]

we will fix it for 5.1 directly

*** This bug has been marked as a duplicate of bug 538976 ***