Bug #39831 for CPAN-Site: CPAN::Site::Index::package_inventory() has problem with Text-PDF-0.29a.tar.gz

* Matisse Enzer via RT (bug-CPAN-Site@rt.cpan.org) [081211 21:55]: Show quoted text
You change if( m/^ (?:use\s+version\s*;\s*)? (?:our)? \s* \$ (?: \w+\:\:)* VERSION \s* \= \s* (.*)/x ) { register into if( m/^ (?:use\s+version\s*;\s*)? (?:our)? \s* \$ (?: \w+\:\:)* VERSION \s* \= \s* (.*)/x ) { $version = eval "my \$v = $1"; } else { print "VERSION not found in $file\n" if $debug; $version = 0; } register Doesn't that mean that you call register all the time? For each processed line? Ever ran your program with $debug on? Actually, CPANs package-list sees version '0' as different from version 'undef'. So, the default is incorrect. You changed: if( m/^\s* package \s* ((\w+\:\:)*\w+) \s* ;/x ) { $package = $1; print "package=$package\n" if $debug; next; } into if( m/^\s* package \s* ((\w+\:\:)*\w+) \s* ;/x ) { $package = $1 || 'unknown'; print "package=$package\n" if $debug; } "unknown" can never happen. "next" is a minor optimization. So no need to take those changes. I have changed original block into if( m/^\s* package \s* ((?:\w+\:\:)*\w+) \s* ;/x ) { $package = $1; warn "package=$package\n" if $debug; register $package, undef, $dist; next; } which first registers the package with an <undef> version. It solves the same problem as you encountered, but with much less run-time. The register() method needed some improvement as well. Attached is the new version to be... ok? -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net

I've looked at your changes and they seem fine to me. Thank you for respond to this issue, and I apologize for missing your response at first. As one more tiny suggestion I've attached a patch for Index.pm that factors out two complex regular expressions from inspect_entry() into compiled regular expression, for example: my $PACKAGE_REGEX = qr/^\s* package \s* ((?:\w+\:\:)*\w+) \s* ;/ox; The only reasons for this change are to make the code a little more readable, possibly faster, and maybe in future add tests specifically for those regular expression (one would probably need to declare them with 'our' in that case.) As far as I am concerned you could mark this ticket "fixed in 0.22"

Index: /menzer/CPAN-Site/lib/CPAN/Site/Index.pm =================================================================== --- /menzer/CPAN-Site/lib/CPAN/Site/Index.pm (revision 3524) +++ /menzer/CPAN-Site/lib/CPAN/Site/Index.pm (working copy) @@ -135,6 +135,9 @@ || $subdir eq 'lib'; # inside lib } +my $PACKAGE_REGEX = qr/^\s* package \s* ((?:\w+\:\:)*\w+) \s* ;/ox; +my $VERSION_REGEX = qr/^ (?:use\s+version\s*;\s*)? + (?:our)? \s* \$ (?: \w+\:\:)* VERSION \s* \= \s* (.*)/ox; sub inspect_entry { my $fn = $File::Find::name; return if ! -f $fn || $fn !~ $tar_gz; @@ -207,7 +210,7 @@ local $_ = $1; $file && $file =~ m/\.pm$/ or next; - if( m/^\s* package \s* ((?:\w+\:\:)*\w+) \s* ;/x ) + if( $_ =~ $PACKAGE_REGEX ) { $package = $1; warn "package=$package\n" if $debug; register $package, undef, $dist; @@ -212,8 +215,7 @@ warn "package=$package\n" if $debug; register $package, undef, $dist; } - elsif( m/^ (?:use\s+version\s*;\s*)? - (?:our)? \s* \$ (?: \w+\:\:)* VERSION \s* \= \s* (.*)/x ) + elsif( $_ =~ $VERSION_REGEX ) { local $VERSION; # destroyed by eval $version = eval "my \$v = $1"; $version = $version->numify if ref $version;