Request for Review: perl-Tk

[Steven Pritchard]

On Tue, Jun 14, 2005 at 10:51:58PM +0200, Andreas Bierfert wrote:
> Would you be willing to approve this as is or some more opinions/changes?

Just a few more thoughts...

> BuildRequires:  perl >= 3:5.8.3

I had a comment "# Versions before this have known Unicode issues."
before that line.  I'd suggest keeping that comment so people remember
why we're requiring a specific version of perl.

It might not be a bad thing to Require that version of perl as well.

> BuildRequires:  libjpeg

Did you mean libjpeg or libjpeg-devel here?

> BuildRequires:  libpng-devel
> BuildRequires:  zlib-devel

libpng-devel requires zlib-devel, so zlib-devel can be dropped.

> BuildRequires:  fontconfig-devel

Required by xorg-x11-devel, so that can be dropped.

(I remember now that I hadn't submitted the package yet because I
hadn't double-checked all the dependencies.  :-)

> %{__perl} -pi -e 's|/usr/local/bin/perl\|/bin/perl|%{__perl}|' * */* */*/*

Are you sure that will work?  "|" looks over-used, plus "*" includes
directories.  How about this?

  find . -type f -exec %{__perl} -pi -e \
    's,^(#!)(/usr/local)?/bin/perl\b,$1%{__perl}, if ($. == 1)' {} \;

(Untested, I'll have to try building with that...)

> CFLAGS="$RPM_OPT_FLAGS" %{__perl} Makefile.PL INSTALLDIRS=vendor XFT=1

I had this:

  %{__perl} Makefile.PL INSTALLDIRS=vendor X11LIB=/usr/X11R6/%{_lib} XFT=1

Setting CFLAGS is redundant.  The options perl was compiled with will
be used automatically.  Anything else might not work properly.

The X11LIB thing is necessary to build on x86_64.

I also had this:

  %{__perl} -pi -e 's/^\tLD_RUN_PATH=[^\s]+\s*/\t/' Makefile

At some point that was in the Extras spec template.  I haven't looked
at it lately though...  Perhaps someone else can comment on why that
was necessary.

> make %{?_smp_mflags} OPTIMIZE="$RPM_OPT_FLAGS"

Again, OPTIMIZE isn't necessary.

Look at the spec template (and/or my spec file) for the proper way to
"make install".  (It's actually "make pure_install", IIRC.)

In the %files section, I have %{perl_vendorarch} instead of
%{perl_vendorlib}.  I can't remember what the difference is right
now...

And just think, Ville and Jose are the picky ones...  ;-)

Steve
-- 
Steven Pritchard - K&S Pritchard Enterprises, Inc.
Email: steve at kspei.com             http://www.kspei.com/
Phone: (618)398-3000               Mobile: (618)567-7320