Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 208636 - Review Request: perl-PPI-Tester - A wxPerl-based interactive PPI debugger/tester
Summary: Review Request: perl-PPI-Tester - A wxPerl-based interactive PPI debugger/tester
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 208009
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-09-29 19:48 UTC by Jose Pedro Oliveira
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-10-06 10:51:45 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Jose Pedro Oliveira 2006-09-29 19:48:33 UTC
Spec URL:
ftp://perl.di.uminho.pt/pub/fedora/perl-PPI-Tester.spec

SRPM URL:
ftp://perl.di.uminho.pt/pub/fedora/perl-PPI-Tester-0.06-1.src.rpm

Description:
This package implements a wxWindows desktop application which provides
the ability to interactively test the PPI perl parser.

Comment 1 Jason Tibbitts 2006-10-03 04:35:34 UTC
This fails to build for me.

First the "perl Build.PL" bit hangs; from the looks of an strace it's waiting
for user input.  Prepending "echo|" gets things going:

+ /usr/bin/perl Build.PL installdirs=vendor
Warning: prerequisite ExtUtils::AutoInstall 0.49 not found.
Checking if your kit is complete...
Looks good
Warning: Guessing NAME [PPI-Tester] from current directory name.
Writing Makefile for PPI-Tester

And then there's no "Build" script to run.  (A Makefile is created instead, for
whatever reason.)  I added BR: perl(ExtUtils::AutoInstall) and the build makes
it to completion.  It took me quite some time to figure that all out, though.

The only rpmlint complaint is over the src.rpm:
   W: perl-PPI-Tester strange-permission PPI-Tester-0.06.tar.gz 0444
which admittedly is a bit weird but isn't a blocker (and won't really matter
once it's in CVS anyway).

Oddly, I could not run this within the mock chroot, because it can't find any
fonts.  I wonder if there's a missing runtime requirement somewhere.  (If so
it's almost certainly not here, but in one of the dependencies.)

The %check tests are successful, but aren't terribly happy:
t/01_compile....Error: Unable to initialize gtk, is DISPLAY set properly?
Use of uninitialized value in concatenation (.) or string at
/usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi/Wx.pm line 179.
Use of uninitialized value in concatenation (.) or string at
/usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi/Wx.pm line 180.
ok

Unfortunately I can't test this because there's no perl(Wx) package for FC5 yet
and because of the above font problem; I'll try to get a rawhide machine going
tomorrow but if I don't have a chance to do so then I'll leave it up to you to
test it.

Review:
* source files match upstream:
   9fcad41d6b2e33cfdf4025eb29644fae  PPI-Tester-0.06.tar.gz
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
X BuildRequires are proper (needs perl(ExtUtils::AutoInstall))
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* rpmlint is silent.
* final provides and requires are sane:
   perl(PPI::Tester) = 0.06
   perl(PPI::Tester::App)
   perl(PPI::Tester::Window)
   perl-PPI-Tester = 0.06-1.fc6
  =
   /usr/bin/perl
   perl(:MODULE_COMPAT_5.8.8)
   perl(FindBin)
   perl(PPI::Dumper)
   perl(PPI::Lexer)
   perl(PPI::Tester)
   perl(Wx)
   perl(Wx::Event)
   perl(base)
   perl(constant)
   perl(lib)
   perl(strict)
   perl(vars)
* %check is present and all tests pass:
   All tests successful.
   Files=1, Tests=2,  0 wallclock secs ( 0.14 cusr +  0.05 csys =  0.19 CPU)
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

Comment 2 Jason Tibbitts 2006-10-04 01:44:51 UTC
The FC5 perl-Wx hit my mirror so I was able to build this on a release I have;
it runs fine and is actually pretty neat.  Clicking the "Load" button causes the
application to exit with the following error:

Error while autoloading 'Wx::wxHIDE_READONLY' at
/usr/lib/perl5/vendor_perl/5.8.8/PPI/Tester.pm line 195

According to the documentation, it's currently unimplemented so this isn't a
problem.

Now, the only remaining issue: is it worth having a .desktop file.  Honestly
this is a test application for a Perl module and I don't think it warrants
appearing in the desktop menus even if it does open a window and have clickable
buttons and such.  Of course, you can add one if you like; I just don't think
the lack of one is a blocker.

So I'll go ahead and approve, but of course you'll have to fix the missing BR:
in order to actually build this.

APPROVED

Comment 3 Jose Pedro Oliveira 2006-10-04 19:00:25 UTC
Imported release 2 with the following changes:
- added the missing build requirement
- switched the perl building procedures to Makefile.PL
- the testsuit only runs if the switch "--with testsuite" is used
  (avoids the warnings related to envionment variable DISPLAY).

I also agree with you regarding the .desktop file. 

This new released has already been built for development.

Thanks for the review.

jpo

Comment 4 Jose Pedro Oliveira 2006-10-06 10:51:45 UTC
Also built for FC-5.


Note You need to log in before you can comment on or make changes to this bug.