Bug #36560 for Audio-TagLib: patch: warnings, test and portability fixes

Hi. The attached patch implements a number of fixes I needed to aply to get Audio::TagLib 1.43 to install (semi-) cleanly on perl 5.8.8 (and 5.10.0) on a vanilla Ubuntu 8.04 system: 1) It cleans up Makefile.PL to bail out in the approved way if library dependencies aren't met (currently it's failing rather than bailing). 2) It adds some portability fixes to Makefile.PL: i) use perl (which we know is available :-) instead of sed to postprocess TagLib.c ii) don't hardwire OS dependencies. It's conceivable that other Unices (and possibly Strawberry Perl) can compile the module OK. It's easier to get feedback on (and fixes for) untested systems by... letting them become tested systems :-) 3) Removed some warnings generated by g++ 4.2.3. Fix the do_open/do_close namespace conflict workaround (the old version was invalid). Fix some "deprecated conversion from string constant to 'char *'" warnings - the rest will have to wait until xsubpp and perl's definition of newXSproto are fixed: http://lists.alioth.debian.org/pipermail/pkg-perl-maintainers/2008-January/009337.html 4) Two of the MPEG tests were failing. According to Image::ExifTool the sample mp3 has a channel mode of Joint Stereo and has the original bit set, so I changed t/TagLib_MPEG_Properties.t to reflect that. I used the latest version of ppport.h (from Devel::PPPort 3.14). The only other issue I encountered was a warning that ppport.h redefines PERL_UNUSED_DECL, but I think that should be fixed in ppport.h. Hope this is useful. chocolateboy P.S. This page is invaluable for portability tips: http://cpantest.grango.org/wiki/CPANAuthorNotes Devel-CheckLib could be used to cleanly check for iconv and Probe-Perl could replace the $^X in Makefile.PL with a path that works on VMS.

diff -uwbr orig-Audio-TagLib-1.43/Makefile.PL Audio-TagLib-1.43/Makefile.PL --- orig-Audio-TagLib-1.43/Makefile.PL 2007-01-17 02:49:27.000000000 +0000 +++ Audio-TagLib-1.43/Makefile.PL 2008-06-08 06:16:08.000000000 +0100 @@ -1,36 +1,44 @@ use 5.008003; use ExtUtils::MakeMaker; -BEGIN { - # a simple work around to perform the neccessary pre-check - # instead of overloading subs of MakeMaker or other wrapper - print STDERR "ONLY support TagLib version 1.4.*\n"; - require Carp; - # FIXME - # openned for FreeBSD, OS X (darwin) and Cygwin - Carp::croak("$^O is not supported currently") - unless $^O eq 'linux' or $^O eq 'freebsd' or $^O eq 'darwin' or - $^O eq 'cygwin'; - Carp::croak("Please install taglib C++ package first") unless - system("taglib-config --version") == 0; - our $libver = qx(taglib-config --version); - chomp($libver); - Carp::croak("Please install taglib ver 1.4.*") unless - $libver =~ m/^1\.4/io; - our $libs = qx(taglib-config --libs); - our $inc = ' -I/usr/include -I./include -I. '. qx(taglib-config --cflags); +sub bail($) { + my $reason = shift; + warn $reason, $/; + exit(0); } -# See lib/ExtUtils/MakeMaker.pm for details of how to influence -# the contents of the Makefile that is written. +# a simple workaround to perform the necessary pre-checks +# instead of overloading MakeMaker subs or another wrapper + +warn 'This module ONLY supports TagLib version 1.4.*', $/; + +bail('Please install the taglib C++ library first') + unless system('taglib-config --version') == 0; + +chomp(my $libver = qx{taglib-config --version}); + +bail('Please install taglib ver 1.4.*') + unless $libver =~ m/^1\.4/; + +chomp(my $libs = qx{taglib-config --libs}); +chomp(my $cflags = qx{taglib-config --cflags}); + +my $inc = " -I/usr/include -I./include -I. $cflags"; + WriteMakefile( NAME => 'Audio::TagLib', VERSION_FROM => 'lib/Audio/TagLib.pm', # finds $VERSION LICENSE => 'perl', PREREQ_PM => {}, # e.g., Module::Name => 1.1 - ($] >= 5.005 ? ## Add these new keywords supported since 5.005 - (ABSTRACT_FROM => 'lib/Audio/TagLib.pm', # retrieve abstract from module - AUTHOR => 'Dongxu, Ma <dongxu@cpan.org>') : ()), + ( + $] >= 5.005 # Add these new keywords supported since 5.005 + ? + ( + ABSTRACT_FROM => 'lib/Audio/TagLib.pm', # retrieve abstract from module + AUTHOR => 'Dongxu, Ma <dongxu@cpan.org>' + ) + : () + ), CC => 'g++', LD => 'g++', LDDLFLAGS => $^O eq 'darwin' ? "-dynamiclib $libs" : "-shared ". $libs, @@ -46,9 +54,9 @@ sub xs_c { my $cmd = shift->SUPER::xs_c(@_); - $cmd .= << 'END'; - sed -i -e 's/newXSproto("TagLib/newXSproto("Audio::TagLib/g' $*.c - sed -i -e 's/XS(boot_TagLib)/XS(boot_Audio__TagLib)/g' $*.c + $cmd .= sprintf(<<'END', $^X); + %1$s -pi -e 's/newXSproto\("TagLib/newXSproto("Audio::TagLib/g' $*.c + %1$s -pi -e 's/XS\(boot_TagLib\)/XS(boot_Audio__TagLib)/g' $*.c END return $cmd; } diff -uwbr orig-Audio-TagLib-1.43/t/TagLib_MPEG_Properties.t Audio-TagLib-1.43/t/TagLib_MPEG_Properties.t --- orig-Audio-TagLib-1.43/t/TagLib_MPEG_Properties.t 2007-01-17 02:44:49.000000000 +0000 +++ Audio-TagLib-1.43/t/TagLib_MPEG_Properties.t 2008-06-08 05:20:42.000000000 +0100 @@ -36,9 +36,9 @@ diag("method version() failed"); cmp_ok($i->layer(), "==", 3) or diag("method layer() failed"); -is($i->channelMode(), "Stereo") or +is($i->channelMode(), "JointStereo") or diag("method channelMode() failed"); ok(not $i->isCopyrighted()) or diag("method isCopyrighted() failed"); -ok(not $i->isOriginal()) or +ok($i->isOriginal()) or diag("method isOriginal() failed"); \ No newline at end of file diff -uwbr orig-Audio-TagLib-1.43/xs/apeitem.xs Audio-TagLib-1.43/xs/apeitem.xs --- orig-Audio-TagLib-1.43/xs/apeitem.xs 2007-01-17 02:44:37.000000000 +0000 +++ Audio-TagLib-1.43/xs/apeitem.xs 2008-06-08 06:14:40.000000000 +0100 @@ -1,24 +1,22 @@ ################################################################ # -# do_open & do_close referred in iostream.h +# do_open & do_close are referred to in iostream.h # included by apeitem.h -# undefine the macro temply here -# and redefine later +# undefine the macro temporarily here +# then redefine after the header has been included # ################################################################ -#ifdef do_open(a,b,c,d,e,f,g) -#undef do_open(a,b,c,d,e,f,g) +#ifdef do_open #undef do_open #undef do_close -#undef do_close(a,b) #endif #include "apeitem.h" -#ifndef do_open -#define do_open Perl_do_open +#ifdef do_open +#undef do_open +#undef do_close +#endif #define do_open(a,b,c,d,e,f,g) Perl_do_open(aTHX_ a,b,c,d,e,f,g) -#define do_close Perl_do_close #define do_close(a,b) Perl_do_close(aTHX_ a,b) -#endif MODULE = TagLib PACKAGE = TagLib::APE::Item PROTOTYPES: ENABLE diff -uwbr orig-Audio-TagLib-1.43/xs/fileref.xs Audio-TagLib-1.43/xs/fileref.xs --- orig-Audio-TagLib-1.43/xs/fileref.xs 2007-01-17 02:44:37.000000000 +0000 +++ Audio-TagLib-1.43/xs/fileref.xs 2008-06-08 03:46:34.000000000 +0100 @@ -14,7 +14,7 @@ PROTOTYPE: ;$$$ PREINIT: char * fileName; - char * style = "Average"; + const char * style = "Average"; TagLib::File * file; TagLib::FileRef * ref; INIT: diff -uwbr orig-Audio-TagLib-1.43/xs/tstring.xs Audio-TagLib-1.43/xs/tstring.xs --- orig-Audio-TagLib-1.43/xs/tstring.xs 2007-01-17 02:44:37.000000000 +0000 +++ Audio-TagLib-1.43/xs/tstring.xs 2008-06-08 03:43:07.000000000 +0100 @@ -14,7 +14,7 @@ TagLib::String::new(...) PROTOTYPE: ;$$ PREINIT: - char *encode, *fromcode; + const char *encode, *fromcode; enum TagLib::String::Type t; bool is_copy_from_string = TRUE; iconv_t codec;