- handle the presence of the default broken giflib better,

          by giving them some more prose about the problem, and skipping
          all but the first test that fails with the broken giflib

diff --git a/Changes b/Changes
index aa71ae4e8b69f1374fb709fd7d6fd9148a286232..30775a28f09fcfd3c27b5926b7ddab518d43a0ff 100644 (file)
--- a/Changes
+++ b/Changes
@@ -614,6 +614,9 @@ Revision history for Perl extension Imager.
           correctly (Ticket #365)
         - make channel pointer to i_gsamp() const int * so we can pass
           const arrays
+        - handle the presence of the default broken giflib better,
+          by giving them some more prose about the problem, and skipping
+          all but the first test that fails with the broken giflib
 
 =================================================================
 

diff --git a/README b/README
index d4ac76c0a47f09b680174e5ec6581bd96a314a63..1676309e9531582b6e8c77d614256a6161f76b75 100644 (file)
--- a/README
+++ b/README
@@ -84,6 +84,12 @@ Which simply sets the environment variables for the extra libraries
 to include the X libraries (which we do not use at all, but must
 included since libgif has been linked with it).
 
+Otherwise you could just build giflib without any X11 dependencies:
+
+  # must be a clean tree
+  cd giflib-4.1.0
+  ./configure --without-x ...
+
 Also note that libgif has a few bugs: You can run something like
 
 $ perl -Iblib/lib -Iblib/arch t/t105gif.t
@@ -129,6 +135,11 @@ needs for specific features.  The libraries it uses are:
   gif: http://www.arrakeen.demon.co.uk/giflib/  
 or at: http://prtr-13.ucsc.edu/~badger/software/libungif/index.shtml
 
+The default giflib is buggy in rarely used code, you can find a patch
+to fix the worst of the problems at:
+
+ http://www.develop-help.com/imager/giflib.patch
+
  tiff: http://www.libtiff.org/
 
    t1: ftp://ftp.neuroinformatik.ruhr-uni-bochum.de/pub/software/t1lib/

diff --git a/t/t105gif.t b/t/t105gif.t
index 7365b19407ee4d0af9aee910742290f0f06d5477..ab4304b3fab1242a9d0dade7877c5843914551d5 100644 (file)
--- a/t/t105gif.t
+++ b/t/t105gif.t
@@ -4,6 +4,8 @@ $|=1;
 print "1..60\n";
 use Imager qw(:all);
 
+my $buggy_giflib_file = "buggy_giflib.txt";
+
 sub ok ($$$);
 
 init_log("testout/t105gif.log",1);
@@ -170,6 +172,7 @@ if (!i_has_format("gif")) {
     print "ok 13\n";
 
     my $can_write_callback = 0;
+    unlink $buggy_giflib_file;
     if ($gifver >= 4.0) {
       ++$can_write_callback;
       unless (fork) {
@@ -192,23 +195,26 @@ if (!i_has_format("gif")) {
       }
       if (wait > 0 && $?) {
         $can_write_callback = 0;
-       print "not ok 14 # you probably need to patch giflib\n";
-       print <<EOS;
-#--- egif_lib.c        2000/12/11 07:33:12     1.1
-#+++ egif_lib.c        2000/12/11 07:33:48
-#@@ -167,6 +167,12 @@
-#         _GifError = E_GIF_ERR_NOT_ENOUGH_MEM;
-#         return NULL;
-#     }
-#+    if ((Private->HashTable = _InitHashTable()) == NULL) {
-#+        free(GifFile);
-#+        free(Private);
-#+        _GifError = E_GIF_ERR_NOT_ENOUGH_MEM;
-#+        return NULL;
-#+    }
-#
-#     GifFile->Private = (VoidPtr) Private;
-#     Private->FileHandle = 0;
+       print "not ok 14 # see $buggy_giflib_file\n";
+       print STDERR "\nprobable buggy giflib - skipping tests that depend on a good giflib\n";
+       print STDERR "see $buggy_giflib_file for more information\n";
+       open FLAG, "> $buggy_giflib_file" or die;
+       print FLAG <<EOS;
+This file is created by t105gif.t when test 14 fails.
+
+This failure usually indicates you\'re using the original version
+of giflib 4.1.0, which has a few bugs that Imager tickles.
+
+You can apply the patch from:
+
+http://www.develop-help.com/imager/giflib.patch
+
+or you can just install Imager as is, if you only need to write GIFs to 
+files or file descriptors (such as sockets).
+
+In previous versions of Imager only this test was careful about catching 
+the error, we now skip any tests that crashed or failed when the buggy 
+giflib was present.
 EOS
       }
     }
@@ -320,136 +326,142 @@ EOS
     # image has no colour map
     read_failure('testimg/nocmap.gif', 22);
 
-    # image has a local colour map
-    open FH, "< testimg/loccmap.gif"
-      or die "Cannot open testimg/loccmap.gif: $!";
-    binmode FH;
-    if (i_readgif(fileno(FH))) {
-      print "ok 23\n";
-    }
-    else {
-      print "not ok 23 # failed to read image with only a local colour map";
-    }
-    close FH;
+    unless (-e $buggy_giflib_file) {
+      # image has a local colour map
+      open FH, "< testimg/loccmap.gif"
+       or die "Cannot open testimg/loccmap.gif: $!";
+      binmode FH;
+      if (i_readgif(fileno(FH))) {
+       print "ok 23\n";
+      }
+      else {
+       print "not ok 23 # failed to read image with only a local colour map";
+      }
+      close FH;
 
-    # image has global and local colour maps
-    open FH, "< testimg/screen2.gif"
-      or die "Cannot open testimg/screen2.gif: $!";
-    binmode FH;
-    my $ims = i_readgif(fileno(FH));
-    if ($ims) {
-      print "ok 24\n";
-    }
-    else {
-      print "not ok 24 # ",Imager::_error_as_msg(),"\n";
-    }
-    close FH;
-    open FH, "< testimg/expected.gif"
-      or die "Cannot open testimg/expected.gif: $!";
-    binmode FH;
-    my $ime = i_readgif(fileno(FH));
-    close FH;
-    if ($ime) {
-      print "ok 25\n";
-    }
-    else {
-      print "not ok 25 # ",Imager::_error_as_msg(),"\n";
-    }
-    if ($ims && $ime) {
-      if (i_img_diff($ime, $ims)) {
-       print "not ok 26 # mismatch ",i_img_diff($ime, $ims),"\n";
-       # save the bad one
-       open FH, "> testout/t105_screen2.gif"
-         or die "Cannot create testout/t105_screen.gif: $!";
-       binmode FH;
-       i_writegifmc($ims, fileno(FH), 7)
-         or print "# could not save t105_screen.gif\n";
-       close FH;
+      # image has global and local colour maps
+      open FH, "< testimg/screen2.gif"
+       or die "Cannot open testimg/screen2.gif: $!";
+      binmode FH;
+      my $ims = i_readgif(fileno(FH));
+      if ($ims) {
+       print "ok 24\n";
       }
       else {
-       print "ok 26\n";
+       print "not ok 24 # ",Imager::_error_as_msg(),"\n";
+      }
+      close FH;
+      open FH, "< testimg/expected.gif"
+       or die "Cannot open testimg/expected.gif: $!";
+      binmode FH;
+      my $ime = i_readgif(fileno(FH));
+      close FH;
+      if ($ime) {
+       print "ok 25\n";
+      }
+      else {
+       print "not ok 25 # ",Imager::_error_as_msg(),"\n";
+      }
+      if ($ims && $ime) {
+       if (i_img_diff($ime, $ims)) {
+         print "not ok 26 # mismatch ",i_img_diff($ime, $ims),"\n";
+         # save the bad one
+         open FH, "> testout/t105_screen2.gif"
+           or die "Cannot create testout/t105_screen.gif: $!";
+         binmode FH;
+         i_writegifmc($ims, fileno(FH), 7)
+           or print "# could not save t105_screen.gif\n";
+         close FH;
+       }
+       else {
+         print "ok 26\n";
+       }
+      }
+      else {
+       print "ok 26 # skipped\n";
       }
-    }
-    else {
-      print "ok 26 # skipped\n";
-    }
 
-    # test reading a multi-image file into multiple images
-    open FH, "< testimg/screen2.gif"
-      or die "Cannot open testimg/screen2.gif: $!";
-    binmode FH;
-    @imgs = Imager::i_readgif_multi(fileno(FH))
-      or print "not ";
-    print "ok 27\n";
-    close FH;
-    @imgs == 2 or print "not ";
-    print "ok 28\n";
-    for my $img (@imgs) {
-      unless (Imager::i_img_type($img) == 1) {
-        print "not ";
-        last;
+      # test reading a multi-image file into multiple images
+      open FH, "< testimg/screen2.gif"
+       or die "Cannot open testimg/screen2.gif: $!";
+      binmode FH;
+      @imgs = Imager::i_readgif_multi(fileno(FH))
+       or print "not ";
+      print "ok 27\n";
+      close FH;
+      @imgs == 2 or print "not ";
+      print "ok 28\n";
+      for my $img (@imgs) {
+       unless (Imager::i_img_type($img) == 1) {
+         print "not ";
+         last;
+       }
+      }
+      print "ok 29\n";
+      Imager::i_colorcount($imgs[0]) == 4 or print "not ";
+      print "ok 30\n";
+      Imager::i_colorcount($imgs[1]) == 2 or print "not ";
+      print "ok 31\n";
+      Imager::i_tags_find($imgs[0], "gif_left", 0) or print "not ";
+      print "ok 32\n";
+      my @tags = map {[ Imager::i_tags_get($imgs[1], $_) ]} 0..Imager::i_tags_count($imgs[1])-1;
+      my ($left) = grep $_->[0] eq 'gif_left', @tags;
+      $left && $left->[1] == 3 or print "not ";
+      print "ok 33\n";
+      
+      # screen3.gif was saved with 
+      open FH, "< testimg/screen3.gif"
+       or die "Cannot open testimg/screen3.gif: $!";
+      binmode FH;
+      @imgs = Imager::i_readgif_multi(fileno(FH))
+       or print "not ";
+      print "ok 34\n";
+      close FH;
+      eval {
+       require 'Data/Dumper.pm';
+       Data::Dumper->import();
+      };
+      unless ($@) {
+       # build a big map of all tags for all images
+       @tags = 
+         map { 
+           my $im = $_; 
+           [ 
+            map { join ",", map { defined() ? $_ : "undef" } Imager::i_tags_get($im, $_) } 
+            0..Imager::i_tags_count($_)-1 
+           ] 
+         } @imgs;
+       my $dump = Dumper(\@tags);
+       $dump =~ s/^/# /mg;
+       print "# tags from gif\n", $dump;
       }
+      
+      # at this point @imgs should contain only paletted images
+      ok(35, Imager::i_img_type($imgs[0]) == 1, "imgs[0] not paletted");
+      ok(36, Imager::i_img_type($imgs[1]) == 1, "imgs[1] not paletted");
+      
+      # see how we go saving it
+      open FH, ">testout/t105_pal.gif" or die $!;
+      binmode FH;
+      ok(37, i_writegif_gen(fileno(FH), { make_colors=>'addi',
+                                         translate=>'closest',
+                                         transp=>'ordered',
+                                       }, @imgs), "write from paletted");
+      close FH;
+      
+      # make sure nothing bad happened
+      open FH, "< testout/t105_pal.gif" or die $!;
+      binmode FH;
+      ok(38, (my @imgs2 = Imager::i_readgif_multi(fileno(FH))) == 2,
+        "re-reading saved paletted images");
+      ok(39, i_img_diff($imgs[0], $imgs2[0]) == 0, "imgs[0] mismatch");
+      ok(40, i_img_diff($imgs[1], $imgs2[1]) == 0, "imgs[1] mismatch");
     }
-    print "ok 29\n";
-    Imager::i_colorcount($imgs[0]) == 4 or print "not ";
-    print "ok 30\n";
-    Imager::i_colorcount($imgs[1]) == 2 or print "not ";
-    print "ok 31\n";
-    Imager::i_tags_find($imgs[0], "gif_left", 0) or print "not ";
-    print "ok 32\n";
-    my @tags = map {[ Imager::i_tags_get($imgs[1], $_) ]} 0..Imager::i_tags_count($imgs[1])-1;
-    my ($left) = grep $_->[0] eq 'gif_left', @tags;
-    $left && $left->[1] == 3 or print "not ";
-    print "ok 33\n";
-
-    # screen3.gif was saved with 
-    open FH, "< testimg/screen3.gif"
-      or die "Cannot open testimg/screen3.gif: $!";
-    binmode FH;
-    @imgs = Imager::i_readgif_multi(fileno(FH))
-      or print "not ";
-    print "ok 34\n";
-    close FH;
-    eval {
-      require 'Data/Dumper.pm';
-      Data::Dumper->import();
-    };
-    unless ($@) {
-      # build a big map of all tags for all images
-      @tags = 
-       map { 
-         my $im = $_; 
-         [ 
-          map { join ",", map { defined() ? $_ : "undef" } Imager::i_tags_get($im, $_) } 
-          0..Imager::i_tags_count($_)-1 
-         ] 
-       } @imgs;
-      my $dump = Dumper(\@tags);
-      $dump =~ s/^/# /mg;
-      print "# tags from gif\n", $dump;
+    else {
+      for (23..40) {
+       print "ok $_ # skip see $buggy_giflib_file\n";
+      }
     }
-
-    # at this point @imgs should contain only paletted images
-    ok(35, Imager::i_img_type($imgs[0]) == 1, "imgs[0] not paletted");
-    ok(36, Imager::i_img_type($imgs[1]) == 1, "imgs[1] not paletted");
-
-    # see how we go saving it
-    open FH, ">testout/t105_pal.gif" or die $!;
-    binmode FH;
-    ok(37, i_writegif_gen(fileno(FH), { make_colors=>'addi',
-                                        translate=>'closest',
-                                        transp=>'ordered',
-                                      }, @imgs), "write from paletted");
-    close FH;
-    
-    # make sure nothing bad happened
-    open FH, "< testout/t105_pal.gif" or die $!;
-    binmode FH;
-    ok(38, (my @imgs2 = Imager::i_readgif_multi(fileno(FH))) == 2,
-       "re-reading saved paletted images");
-    ok(39, i_img_diff($imgs[0], $imgs2[0]) == 0, "imgs[0] mismatch");
-    ok(40, i_img_diff($imgs[1], $imgs2[1]) == 0, "imgs[1] mismatch");
-
     # test that the OO interface warns when we supply old options
     {
       my @warns;

diff --git a/t/t50basicoo.t b/t/t50basicoo.t
index 66f4537e35f15c9c2ce30987347e2a9d61f327e1..6de70b76dcd1eff7d9628bb806904f93a682faff 100644 (file)
--- a/t/t50basicoo.t
+++ b/t/t50basicoo.t
@@ -9,6 +9,8 @@ use strict;
 use Imager;
 use IO::Seekable;
 
+my $buggy_giflib_file = "buggy_giflib.txt";
+
 Imager::init("log"=>"testout/t50basicoo.log");
 
 # single image/file types
@@ -204,7 +206,8 @@ for my $type (@types) {
      "write to FH after writing $type");
   ok($fh->close, "closing FH after writing $type");
 
-  if ($type ne 'gif' || Imager::i_giflib_version() >= 4) {
+  if ($type ne 'gif' || 
+      (Imager::i_giflib_version() >= 4 && !-e $buggy_giflib_file)) {
     if (ok(open(DATA, "< $file"), "opening data source")) {
       binmode DATA;
       my $data = do { local $/; <DATA> };
@@ -297,7 +300,12 @@ for my $type (@types) {
     }
   }
   else {
-    skip("giflib < 4 doesn't support callbacks", 8);
+    if (-e $buggy_giflib_file) {
+      skip("see $buggy_giflib_file", 8);
+    }
+    else {
+      skip("giflib < 4 doesn't support callbacks", 8);
+    }
   }
 }
 

diff --git a/t/t70newgif.t b/t/t70newgif.t
index 6bc1343f238e3277bfe47c88fd32aaac42f40d17..fef823d33d8b8b570be52db48c8cabca724ce9cb 100644 (file)
--- a/t/t70newgif.t
+++ b/t/t70newgif.t
@@ -11,6 +11,8 @@
 BEGIN { $| = 1; print "1..24\n"; }
 END {print "not ok 1\n" unless $loaded;}
 
+my $buggy_giflib_file = "buggy_giflib.txt";
+
 use Imager qw(:all :handy);
 $loaded=1;
 
@@ -92,45 +94,59 @@ if (i_has_format("gif")) {
   print "ok 13\n";
   Imager->errstr =~ /fh option not open/ or print "not ";
   print "ok 14 # ",Imager->errstr,"\n";
-  @imgs = Imager->read_multi(type=>'gif', file=>'testimg/screen2.gif');
-  @imgs == 2 or print "not ";
-  print "ok 15\n";
-  grep(!UNIVERSAL::isa($_, 'Imager'), @imgs) and print "not ";
-  print "ok 16\n";
-  grep($_->type eq 'direct', @imgs) and print "not ";
-  print "ok 17\n";
-  (my @left = $imgs[0]->tags(name=>'gif_left')) == 1 or print "not ";
-  print "ok 18\n";
-  my $left = $imgs[1]->tags(name=>'gif_left') or print "not ";
-  print "ok 19\n";
-  $left == 3 or print "not ";
-  print "ok 20\n";
-  if (Imager::i_giflib_version() >= 4.0) {
-    open FH, "< testimg/screen2.gif" 
-      or die "Cannot open testimg/screen2.gif: $!";
-    binmode FH;
-    my $cb = 
-      sub {
-        my $tmp;
-        read(FH, $tmp, $_[0]) and $tmp
-      };
-    @imgs = Imager->read_multi(type=>'gif',
-                               callback => $cb) or print "not ";
-    print "ok 21\n";
-    close FH;
+  unless (-e $buggy_giflib_file) {
+    @imgs = Imager->read_multi(type=>'gif', file=>'testimg/screen2.gif');
     @imgs == 2 or print "not ";
-    print "ok 22\n";
-
-    open FH, "< testimg/screen2.gif" 
-      or die "Cannot open testimg/screen2.gif: $!";
-    binmode FH;
-    my $data = do { local $/; <FH>; };
-    close FH;
-    @imgs = Imager->read_multi(type=>'gif',
-                              data=>$data) or print "not ";
-    print "ok 23\n";
-    @imgs = 2 or print "not ";
-    print "ok 24\n";
+    print "ok 15\n";
+    grep(!UNIVERSAL::isa($_, 'Imager'), @imgs) and print "not ";
+    print "ok 16\n";
+    grep($_->type eq 'direct', @imgs) and print "not ";
+    print "ok 17\n";
+    (my @left = $imgs[0]->tags(name=>'gif_left')) == 1 or print "not ";
+    print "ok 18\n";
+    my $left = $imgs[1]->tags(name=>'gif_left') or print "not ";
+    print "ok 19\n";
+    $left == 3 or print "not ";
+    print "ok 20\n";
+  }
+  else {
+    for (15 .. 20) {
+      print "ok $_ # skip see $buggy_giflib_file\n";
+    }
+  }
+  if (Imager::i_giflib_version() >= 4.0) {
+    unless (-e $buggy_giflib_file) {
+      open FH, "< testimg/screen2.gif" 
+       or die "Cannot open testimg/screen2.gif: $!";
+      binmode FH;
+      my $cb = 
+       sub {
+         my $tmp;
+         read(FH, $tmp, $_[0]) and $tmp
+       };
+      @imgs = Imager->read_multi(type=>'gif',
+                                callback => $cb) or print "not ";
+      print "ok 21\n";
+      close FH;
+      @imgs == 2 or print "not ";
+      print "ok 22\n";
+      
+      open FH, "< testimg/screen2.gif" 
+       or die "Cannot open testimg/screen2.gif: $!";
+      binmode FH;
+      my $data = do { local $/; <FH>; };
+      close FH;
+      @imgs = Imager->read_multi(type=>'gif',
+                                data=>$data) or print "not ";
+      print "ok 23\n";
+      @imgs = 2 or print "not ";
+      print "ok 24\n";
+    }
+    else {
+      for (21..24) {
+       print "ok $_ # skip see $buggy_giflib_file\n";
+      }
+    }
   }
   else {
     for (21..24) {