Skip Menu |

This queue is for tickets about the CPAN-Site CPAN distribution.

Report information
The Basics
Id: 41935
Status: resolved
Priority: 0/
Queue: CPAN-Site

People
Owner: Nobody in particular
Requestors: matisse [...] spamcop.net
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.23
Fixed in: (no value)



Subject: CPAN::Site::Index::inspect_entry() will register some packages found outside the normal locations
CPAN::Site::Index::inspect_entry() will register some packages found outside the normal locations. For example, it will register packages found in a distributions "inc/" directory. Example: PPI-1.203 contains: PPI-1.203/inc/File/Remove.pm and inspect_entry() will register that package as belonging to PPI-1.203. I think the problem is that inspect_entry() has the following code, which only checks package_on_usual_location() in one case, but not in the other: #---------------- Index.pm 0.23 205-225 -------------------------- while($out_buf =~ s/^([^\n]*)\n//) { local $_ = $1; $file && $file =~ m/\.pm$/ or next; if( m/^\s* package \s* ((?:\w+\:\:)*\w+) \s* ;/x ) { $package = $1; warn "package=$package\n" if $debug; register $package, undef, $dist; } elsif( m/^ (?:use\s+version\s*;\s*)? (?:our)? \s* \$ (?: \w+\:\:)* VERSION \s* \= \s* (.*)/x ) { local $VERSION; # destroyed by eval $version = eval "my \$v = $1"; $version = $version->numify if ref $version; warn "version=$version\n" if $debug; register $package, $version, $dist if $package && package_on_usual_location $file; } } #------------------------------------end snip----------------- I am creating a unit test for this and will post it and a fix here as soon as I can, but because I need to get permission from the legal department for the post-back this may not be until some time in January.
Subject: Re: [rt.cpan.org #41935] CPAN::Site::Index::inspect_entry() will register some packages found outside the normal locations
Date: Mon, 29 Dec 2008 10:04:34 +0100
To: Matisse Enzer via RT <bug-CPAN-Site [...] rt.cpan.org>
From: Mark Overmeer <mark [...] overmeer.net>
* Matisse Enzer via RT (bug-CPAN-Site@rt.cpan.org) [081224 19:36]: Show quoted text
> Wed Dec 24 14:36:58 2008: Request 41935 was acted upon. > Transaction: Ticket created by MATISSE > Queue: CPAN-Site > Subject: CPAN::Site::Index::inspect_entry() will register some packages > > > CPAN::Site::Index::inspect_entry() will register some packages found > outside the normal locations. For example, it will register packages > found in a distributions "inc/" directory.
Great :( For me, CPAN::Site has quite a low priority, so I fix fast and break easy. Of course, adding many regression tests would make it into a much better module... but the concept is quite flawed in the first place. CPAN6 is the future. I think the better fix is to move the "usual_location" test to before any attempts for regexes. $out_buf .= $in_buf; while($out_buf =~ s/^([^\n]*)\n//) { local $_ = $1; - $file && $file =~ m/\.pm$/ or next; + $file && $file =~ m/\.pm$/ && package_on_usual_location $file + or next; This is the fix I will release now. But I think it should go much higher up... $out_buf .= $in_buf; unless($file && $file =~ m/\.pm$/ && package_on_usual_location $file) { $out_buf =~ s/.*\n//; # purge whole lines next; } while($out_buf =~ s/^([^\n]*)\n//) { local $_ = $1; - $file && $file =~ m/\.pm$/ or next; Or even further up. See $inspect_this_file in the attached pre-release. But the only real solution is to use Archive::Tar for the whole procedure. -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
Download CPAN-Site-0.24.tar.gz
application/x-gunzip 65.3k

Message body not shown because it is not plain text.

On Mon Dec 29 04:04:55 2008, Mark@Overmeer.net wrote: Show quoted text
> ... Of course, adding many regression tests would make it into a > much better module...
I have created a regression test for this issue, and as soon as I get clearance from the legal folks at work I will upload it (I have agreed to get permission first for all open-source releases I make.) Show quoted text
>... but the concept is quite flawed in the first > place. CPAN6 is the future.
That is probably true but, CPAN::Site is what we have now. Show quoted text
> I think the better fix is to move the "usual_location" test to before > any attempts for regexes.
... Show quoted text
> This is the fix I will release now. > But I think it should go much higher up...
... Show quoted text
> Or even further up. See $inspect_this_file > in the attached pre-release.
I agree that checking higher up is better. I ran my new regression tests on the pre-release version and it passed. Suggestions: Change the variable name from: $inspect_this_file to a more "boolean-sounding" name: $file_is_candidate_for_registration and also change line 212 from "next;" to "next BLOCK;" Show quoted text
> But the only real solution is to use Archive::Tar for the whole > procedure.
That is also more or less true - I think that is the basis of the ticket brian d foy opened: https://rt.cpan.org/Ticket/Display.html?id=40301 I have used CPAN::Site at three different companies now to help maintain private CPAN repositories where we want two important things: 1. Have only specific versions of public modules available, not the whole CPAN. We occasionally add/upgrade modules. 2. Add our own locally created modules to the repository. It's possible that CPAN6 will make this much easier, but that is far in the future. I wonder if there is enough interest from enough people/companies to create a solid framework for creating and maintaining private CPAN repos behind a company firewall. Different companies have different needs. The Java world has a dependency manager called Ivy (http://www.tls.cena.fr/products/ivy/) which supports "chaining" repositories. Anyway, the fix for this ticket seems good to me and I will send you my regression tests as soon as I get clearance. Thanks for the work of maintaining this distribution - I know how much of a pain in the ass it can be :-)
Subject: Re: [rt.cpan.org #41935] CPAN::Site::Index::inspect_entry() will register some packages found outside the normal locations
Date: Tue, 30 Dec 2008 11:38:04 +0100
To: Matisse Enzer via RT <bug-CPAN-Site [...] rt.cpan.org>
From: Mark Overmeer <mark [...] overmeer.net>
* Matisse Enzer via RT (bug-CPAN-Site@rt.cpan.org) [081229 19:28]: Show quoted text
> Queue: CPAN-Site > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=41935 >
> > place. CPAN6 is the future.
> That is probably true but, CPAN::Site is what we have now.
True. Show quoted text
> Change the variable name from: $inspect_this_file > to a more "boolean-sounding" name: > $file_is_candidate_for_registration
I cannot see any other value than a boolean related to that name. Show quoted text
> and also change line 212 from "next;" to "next BLOCK;"
Yes, not need but more readible. Show quoted text
> It's possible that CPAN6 will make this much easier, but that is far in > the future.
Hopefully not too far... but I have so much to do before it works: all the building bricks are not present either :( Show quoted text
> (http://www.tls.cena.fr/products/ivy/) which supports "chaining" > repositories.
CPAN6 has chaining, sub-setting and composition of archives. (one archive can be mirrored in multiple repositories) Show quoted text
> Anyway, the fix for this ticket seems good to me and I will send you my > regression tests as soon as I get clearance.
ok. Show quoted text
> Thanks for the work of maintaining this distribution - I know how much > of a pain in the ass it can be :-)
I always like it when people contribute. IMO, you should only publish modules if you maintain them... there are so many unmaintained modules on CPAN, its a horror. You're contributions are very helpful to other people. I have just release 0.24. The regression-tests can be added later. -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
I am attaching a new version of the unit tests for CPAN::Site::Index and a small tarball that is used by a new test. The file CPAN-Site-Index.t replaces t/CPAN-Site-Index.t The tarball should be added as: t/test_data/Distro-With-Packages-Outside-lib.tar.gz

Message body not shown because it is not plain text.

#!/usr/bin/perl use warnings; use strict; # CPAN-Site-Index.t - unit tests for CPAN::Site::Index #------------------------------------------------------------------------------- use strict; use warnings; use FindBin qw($Bin); use Test::More tests => 53; use_ok('CPAN::Site::Index') || BAIL_OUT('Failed to compiled module CPAN::Site::Index'); test_inspect_entry_for_distro_with_strange_data(); test_inspect_entry_for_distro_with_packages_that_should_not_be_registered(); exit; #------------------------------------------------------------------------------- sub test_inspect_entry_for_distro_with_strange_data { my $distro_to_test = 'Text-PDF-0.29a.tar.gz'; my %want_packages = ( 'Text::PDF::Array' => undef, 'Text::PDF::Bool' => undef, 'Text::PDF::Dict' => undef, 'Text::PDF::File' => '0.27', 'Text::PDF::Filter' => undef, 'Text::PDF::ASCII85Decode' => undef, 'Text::PDF::RunLengthDecode' => undef, 'Text::PDF::ASCIIHexDecode' => undef, 'Text::PDF::FlateDecode' => undef, 'Text::PDF::LZWDecode' => undef, 'Text::PDF::Name' => undef, 'Text::PDF::Null' => undef, 'Text::PDF::Number' => undef, 'Text::PDF::Objind' => undef, 'Text::PDF::Page' => undef, 'Text::PDF::Pages' => undef, 'Text::PDF::SFont' => undef, 'Text::PDF::String' => undef, 'Text::PDF::TTFont' => undef, 'Text::PDF::TTIOString' => undef, 'Text::PDF::TTFont0' => undef, 'Text::PDF::Utils' => undef, 'Text::PDF' => '0.29', ); _test_inspect_entry_for_distro( $distro_to_test, \%want_packages ); } sub test_inspect_entry_for_distro_with_packages_that_should_not_be_registered { my $distro_to_test = 'Distro-With-Packages-Outside-lib.tar.gz'; my %want_packages = ( 'TopOfDistro' => '0.01', 'InsideLib' => '0.01', ); _test_inspect_entry_for_distro( $distro_to_test, \%want_packages ); } sub _test_inspect_entry_for_distro { my $distro = shift; my $want_packages = shift; # inspect_entry() relies upon a global variable $topdir which # we is declared with 'our' in Index.pm so we can set it here for testing. $CPAN::Site::Index::topdir = "$Bin/test_data"; # inspect_entry is called in Index.pm using File::Find # find { wanted => \&inspect_entry, no_chdir => 1 }, $topdir; # so we set two variables that File::Find normally sets: $File::Find::name = "$CPAN::Site::Index::topdir/$distro"; $File::Find::dir = $CPAN::Site::Index::topdir; note("Checking $File::Find::name in $File::Find::dir"); $CPAN::Site::Index::findpkgs = {}; CPAN::Site::Index::inspect_entry(); my @missing_pkgs = (); foreach my $want_pkg ( sort keys %{$want_packages} ) { my $have_package = exists $CPAN::Site::Index::findpkgs->{$want_pkg}; ok( $have_package, "Found package '$want_pkg' in tarball." ) || push @missing_pkgs, $want_pkg; SKIP: { skip( "Didn't find '$want_pkg', no point in testing VERSION", 1 ) unless $have_package; my $have_version = $CPAN::Site::Index::findpkgs->{$want_pkg}->[0]; my $want_version = $want_packages->{$want_pkg}; is( $have_version, $want_version, "Got expected version of $want_pkg" ); } } if (@missing_pkgs) { diag( "Missing packages: @missing_pkgs\n\n", 'Packages found: ', explain($CPAN::Site::Index::findpkgs) ); } my @unexpected_packages = (); foreach my $got_package ( sort keys %{$CPAN::Site::Index::findpkgs} ) { if ( not exists $want_packages->{$got_package} ) { push @unexpected_packages, $got_package; } } is( scalar @unexpected_packages, 0, "No unexpected packages found in $distro" ) || diag( "Got unexpected packages in distro '$distro':\n\t", join( "\n\t", @unexpected_packages ), ); #diag Test::More::explain($CPAN::Site::Index::findpkgs); }
Subject: Re: [rt.cpan.org #41935] CPAN::Site::Index::inspect_entry() will register some packages found outside the normal locations
Date: Wed, 21 Jan 2009 10:01:01 +0100
To: Matisse Enzer via RT <bug-CPAN-Site [...] rt.cpan.org>
From: Mark Overmeer <mark [...] overmeer.net>
* Matisse Enzer via RT (bug-CPAN-Site@rt.cpan.org) [090107 15:49]: Show quoted text
> Queue: CPAN-Site > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=41935 > > > I am attaching a new version of the unit tests for CPAN::Site::Index and > a small tarball that is used by a new test. > > The file CPAN-Site-Index.t replaces t/CPAN-Site-Index.t > > The tarball should be added as: > t/test_data/Distro-With-Packages-Outside-lib.tar.gz
I would have preferred this second test in a seperate script. But... have released it anyway. Released as 0.25 -- Thank you very much for your effort! MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
fixed
Subject: Re: [rt.cpan.org #41935] CPAN::Site::Index::inspect_entry() will register some packages found outside the normal locations
Date: Tue, 3 Feb 2009 13:13:57 +0100
To: Matisse Enzer via RT <bug-CPAN-Site [...] rt.cpan.org>
From: Mark Overmeer <solutions [...] overmeer.net>
* Matisse Enzer via RT (bug-CPAN-Site@rt.cpan.org) [090107 15:49]: Show quoted text
> Queue: CPAN-Site > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=41935 > > > I am attaching a new version of the unit tests for CPAN::Site::Index and > a small tarball that is used by a new test.
CPAN-testers report this problem: (on Windows) http://nntp.x.perl.org/group/perl.cpan.testers/3161212 You may have an idea how to fix this. -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
Subject: Re: [rt.cpan.org #41935] CPAN::Site::Index::inspect_entry() will register some packages found outside the normal locations
Date: Tue, 3 Feb 2009 09:09:33 -0800
To: bug-CPAN-Site [...] rt.cpan.org
From: Matisse Enzer <matisse [...] matisse.net>
On Feb 3, 2009, at 4:37 AM, Mark Overmeer via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=41935 > > > * Matisse Enzer via RT (bug-CPAN-Site@rt.cpan.org) [090107 15:49]:
>> Queue: CPAN-Site >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=41935 > >> >> I am attaching a new version of the unit tests for >> CPAN::Site::Index and >> a small tarball that is used by a new test.
> > CPAN-testers report this problem: (on Windows) > http://nntp.x.perl.org/group/perl.cpan.testers/3161212 > You may have an idea how to fix this.
I don't have a Windows system to test on, but, the first thing I'd like to understand is why the "No such file or directory" message: t/CPAN-Site-Index....'S:/bin/dev/perl/cpan/build/CPAN-Site-0.25- kISQ7E/t/test_data/Text-PDF-0.29a.tar.gz'.gz: No such file or directory For one thing, ".gz" appears twice in that message. That seems to be just a problem with the error message itself - notice the single quoted path: 'S:/bin/dev/perl/cpan/build/CPAN-Site-0.25-kISQ7E/t/test_data/Text- PDF-0.29a.tar.gz' Maybe Windows is adding an extra .gz to the path? -Matisse
Report on old code; problem possible disappeared.